Skip to content
This repository was archived by the owner on Apr 8, 2024. It is now read-only.

Add tutorial on how to remove nodes from a cluster #159

Closed
wants to merge 2 commits into from

Conversation

marregui
Copy link
Contributor

@marregui marregui commented Mar 5, 2020

This tutorial is a roundabout exemplification of the use of the crate-node tool to detach and bootstrap nodes.

First a cluster is setup, by pointing the reader to existing tutorials, and by elaborating on how to build CrateDB from sources and test locally rather than on a production cluster. Then some data is loaded, with references to the ISS tutorial, which is awesome. But also, suggesting alternative ways. Finally the 3 node cluster is made to be a single node cluster.


.. _scaling-down-starting-vanilla-cluster:

Starting a Vanilla cluster
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "Vanilla" a technical term here, or just meant in the sense of "generic"? If the latter I'd write it lowercase "vanilla"

@marregui
Copy link
Contributor Author

Hola @mechanomi ,

I was going to work on this guide, however there is this blocked label. Should I get the guide to merge point, or are the contents not relevant anymore?

Thank you

@nomicode nomicode removed the blocked label Jun 19, 2020
@nomicode
Copy link
Contributor

nomicode commented Jun 22, 2020

@marregui the blocked label was to indicate to me that you are blocked on this. but I changed how we're organizing the project board so I removed it

will get back to you on this soon

@marregui marregui force-pushed the ma/downscaling branch 2 times, most recently from 0c58a8e to 0814d5d Compare July 2, 2020 10:36
@marregui
Copy link
Contributor Author

marregui commented Jul 2, 2020

Excellent thank you. I have pushed the latest version, so now it is down to PR reviewing amending.

@nomicode
Copy link
Contributor

@marregui heya. I've taken a look! I have some high-level comments


I am not comfortable with the idea of shipping scripts in a directory that the reader has to clone. for a how-to guide, the scripts should be embedded in the text

however, it is fine to link to pastebin versions of them for ease of use. our current way of doing this is to use a pastebin service that is most appropriate to the type of code being used. the critical thing here, though, is that the full code is there in the doc. the pastebin is doesn't have anything extra in it. this allows us to re-create pastebin links if we want to without risking losing any information

also: please don't use GitHub to this, or any other service that ties the paste to a specific username. we want to avoid the situation where an individual author disables their account on a third-party service some time in the future and takes down a bunch of pastes that we are using

so my first request is this: can you rewrite this so that you introduce each script as you go along. explain what it does, how it can be used, etc. fit it into the "story" you're telling in the how-to

for some of the scripts, I think you should consider dropping them entirely. for example, the one that installs CrateDB. a better solution here is to link to our existing installation guides or deployment guides and let the user figure this out for themselves. we should keep the how-to guide as tight and focused as possible

I also don't think it's necessary to include the complete configuration for each node. we should assume a default configuration and only give specific example configuration snippets when the user is required to make alterations to the default config


can you please restructure the how-to so that you don't introduce all the scripts at the beginning. the scripts, or config snippets, or code snippets should appear in the text at the point where they are needed. this helps keep the how-to readable and formats it more like a story (which is easier to read)


for the sample data, I would suggest two alternative methods:

  1. either point the reader at our tutorials showing them how to generate mock time series data using ISS telemetry information

  2. show them how to use the cr8 tool (see https://crate.io/docs/crate/howtos/en/latest/performance/inserts/testing.html)


it's possible that this how-to becomes quite long with the changes I have requested. that's fine. as we continue to review and edit we can make the decision to split it up into multiple parts if that makes sense

@marregui
Copy link
Contributor Author

thank you!, sure, will do as you request.

curiosity, is pastebin this: https://pastebin.com/ ?

@nomicode
Copy link
Contributor

@marregui that's one pastebin. but you should look around. there are other options. we should pick the one that is most used by the community (whichever community the doc is targeting)

@marregui
Copy link
Contributor Author

Hi @mechanomi

I have deleted the scripts and inlined them where relevant. I have also revisited the prose to include hooks to other guides/documentation.

@marregui marregui changed the title Add guide to downscale a Vanilla cluster Add tutorial on how to remove nodes from a cluster Jul 24, 2020
@nomicode nomicode added docs A documentation issue content: new and removed feature labels Jul 24, 2020
@nomicode
Copy link
Contributor

@marregui can you let me know explicitly when you'd like me to take another look, please? I'm not sure from the comment you left. thanks!

@marregui
Copy link
Contributor Author

hi @mechanomi could you please have another look? thank you in advance.

@marregui
Copy link
Contributor Author

marregui commented Aug 4, 2020

hi, I will away next week. I was wondering about the chances of merging the PR this week? thank you in advance

@nomicode
Copy link
Contributor

nomicode commented Aug 7, 2020

@marregui heya. sorry, I've been sick this week. I hope to have a second review ready for you by the time you're back from vacation

@marregui
Copy link
Contributor Author

Hi @mechanomi I hope you are feeling better. I am back.

@nomicode
Copy link
Contributor

@marregui there appears to be a downscaling.rst and a downscaling.txt file added. I assume that one of them is the file you want to add and the other is an older version of the same file. can you remove the older version? or indicate to me which one is the newest version so I know which file to edit

thank you!

The idea is to give readers a low entry point to clustering, from which
they can expand their knowledge.

A Vanilla cluster is a three node cluster that runs on a single host.
In this guide we create the cluster, add data to it, and then remove
two nodes. Some scripts are required, which are available under the
'scripts' folder. The idea is that users download a zip containing
them, or checkout the repo to access them.
@marregui
Copy link
Contributor Author

@mechanomi apologies for this, I have amended it. The correct document is *.rst.

@nomicode
Copy link
Contributor

okay great!! thank you. this is looking much better now

since you started on this tutorial, the multi node cluster tutorial was updated quite significantly. check it out: https://crate.io/docs/crate/howtos/en/latest/clustering/multi-node-setup.html
can you update your tutorial so that it directs people there and uses the setup that is documented in that tutorial

if you need to suggest specific config changes, that's fine. but they should be config changes that can be applied to the cluster that the reader has set up following the previous tutorial

you can do away with the whole "Installing from sources" section. let's just provide the reader with one and only one way to do everything. and in this instance, we should direct them to the multi node cluster tutorial and work with the results of that. doing it this way will keep the tutorial easy to follow, easy to understand, and short

then, for thee "Adding some data" section, let's go a similar route. you have already linked to the "generate time series tutorial", which is great. but then the next section goes on to detail how to generate and work with a different type of mock data

for the same reasons as before, I think it's probably best if we take out the stuff to do with csv logs. that includes the script to handle the data as well as the tables/queries

that should simplify that whole section as it's mostly just deleting stuff

for the "Exploring the Data" section, could you rework this so that you're exploring the data generated by the "generate time series data" tutorial running on the multi node cluster set up by the previous tutorial? that way, the tutorial you are writing is building on both of those previous tutorials

this will include switching away from working with the logs table to working with the iss table

hopefully, all of the above is relatively straight forward as it mostly involves deleting and/or slightly adapting what you already have. the real meat of the tutorial is the last section where you demonstrate how to downscale the cluster. and hopefully, the changes I am suggesting don't substantially affect that bit

what do you think?

@marregui
Copy link
Contributor Author

Thank you for the feedback!. All sounds very reasonable, and I will proceed as you describe.

@mfussenegger mfussenegger requested a review from seut August 28, 2020 08:26
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of comments. I'd suggest that someone which is not involved works through this to validate that all is understandable and works.

Comment on lines +22 to +24
This setup provides a local cluster environment, when you only have one host, at the
cost of increased latency in writes, because when operating as a cluster, the nodes
must reach consensus on each write operation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence confuses me. What is the intention here? Expressing that there are no benefits of running a cluster of multiple nodes on just 1 node but only downsides?
If so I suggest to rephrase and using shorter sentences instead of one really long confusing one.

Suggestion:

This setup provides a local-only cluster environment for demonstration purpose only.

.. Note:

     Running multiple nodes, a full cluster, on 1 host only won't result in any benefits like resiliency or load 
     balancing over a single-node cluster. Instead performance will decrease due to internal node-to-node 
     communication like discovery, write-consensus, coordinator-election, etc.

cluster.name: vanilla
node.name: <NODE_NAME>
network.host: _local_
node.max_local_storage_nodes: 3
Copy link
Member

@seut seut Aug 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we'll probably backport this commit soon or later elastic/elasticsearch#42428 which will remove this setting, I'd suggest to not to use this setting here anymore but configure a dedicated path.data for each node instead.

-Cpath.repo=$path_snapshots \
-Cnode.name=<NODE_NAME> \
-Ctransport.tcp.port=<PORT>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth adding an example how to create the cluster using that script?
Do people understand that they have to provide a node name as the first argument?
Maybe one example is enough.


::

node_name=$1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the shebang should be added, people will probably copy-paste it and then it won't work. Aslo it clarifies which language this script is written in.
Additionally maybe use the related script RST highlighter?


::

import random
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what language is this script written in? adding a shebang would help, same arguments as above.


::

cluster.name: simple # don't really need to change this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the cluster name now different and why don't I need to change it?

Suggested change
cluster.name: simple # don't really need to change this
cluster.name: vanilla


cluster.name: simple # don't really need to change this
node.name: n1
stats.service.interval: 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, this setting is not relevant, so I'd suggest to drop it.

http.cors.enabled: true
http.cors.allow-origin: "*"

transport.tcp.port: <PORT>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it needed to define this setting? afaik the default works fine.
Otherwise the settings for discovery.seed_hosts and cluster.initial_master_nodes would have to be adjusted as well.

- 127.0.0.1:4302
- 127.0.0.1:4303

where ``<PORT>`` is one of ``[4301, 4302, 4303]``, and ``<NODE_NAME>`` one of ``[n1, n1, n3]``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said, no need for <PORT>.

http.cors.enabled: true
http.cors.allow-origin: "*"

transport.tcp.port: 4301
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as said, not needed.

@marregui
Copy link
Contributor Author

marregui commented Sep 7, 2020

Hi,

Thank you for the comments and advise. This tutorial was written a long time ago as an answer to a user's question in community. He was satisfied, he learned, and since then, a whole lot of content was produced and added without having any consideration towards the content in this tutorial. I think having to morph it entirely into something I never planned, specially after the fact (user is happy) seems wasteful to me. Thus I would like to discontinue work on this content and @mechanomi can decide whether to close, or amend/merge.

Thank you again.

@marregui marregui removed their assignment Sep 7, 2020
@marregui marregui closed this Oct 8, 2020
@nomicode nomicode deleted the ma/downscaling branch October 9, 2020 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docs A documentation issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants