Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster config fixes and removal of meta.peers config field #3638

Merged
merged 4 commits into from
Aug 12, 2015
Merged

Conversation

jwilder
Copy link
Contributor

@jwilder jwilder commented Aug 12, 2015

This PR fixes a panic and a regression with the -hostname flag as well as removes the [meta].peers config option to avoid confusion. The supported method for joining a node to a cluster is using the -join flag.

The env var overrides panic if we if peers was set in the config.
The type of that value is a []string and NumFields is not a valid
for that type.  We don't currently support this slice fields via
the env variable settings.  This particular one can be set with the
-join flag already so we just skip these fields for now.
Adding a new peer must happen via the -join flag.
@jwilder jwilder changed the title Clustering fixes Clustering config fixes Aug 12, 2015
@@ -31,7 +31,7 @@ type Config struct {
Dir string `toml:"dir"`
Hostname string `toml:"hostname"`
BindAddress string `toml:"bind-address"`
Peers []string `toml:"peers"`
Peers []string `toml:"-"`
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this mean? Ignore in older config files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It's ignored in old config files and not-settable via toml configs.

@jwilder jwilder changed the title Clustering config fixes Cluster config fixes and removal of meta.peers config field Aug 12, 2015
@otoolep
Copy link
Contributor

otoolep commented Aug 12, 2015

+1

jwilder added a commit that referenced this pull request Aug 12, 2015
Cluster config fixes and removal of meta.peers config field
@jwilder jwilder merged commit 2e15449 into master Aug 12, 2015
@jwilder jwilder deleted the jw-fixes branch August 12, 2015 19:43
@sebito91
Copy link
Contributor

Honestly, not sure this makes much sense. How are we supposed to control having a leader + follower set for when clustering is open to 100s or 1000s of nodes? Does this mean we'll always need to start one machine as the effective 'leader', then just have a -join for all other machines in the cluster? What if the leader fails and we need to restart that node, should they start back up with -join or without?

If we look to something like hdfs, this is handled via zookeeper and leader election is seamless to the process (ie. all nodes start up the same way with leader election handled behind the scenes). This change puts a lot of that work onto the designer of the cluster, and effectively creates a leader + follower structure that doesn't really work when you get out to scale.

@jwilder
Copy link
Contributor Author

jwilder commented Aug 21, 2015

When you start a cluster you can have them all start with the same -join flags. (e.g. set -join host1:8088,host2:8088,host3:8088 on all nodes). The first 3 nodes to join will take part in the raft cluster and will elect a leader amongst themselves. If the leader fails, a new leader will be elected from the remaining raft peers if a quorum is available.

Adding additional nodes just requires joining another node in an existing cluster. It does not need to join a member of the raft cluster. Any node is fine. Once a node has joined a cluster, the -join flag is not used anymore and it will re-use it's existing cluster state so it's fine to just set it once and leave it.

@jwilder
Copy link
Contributor Author

jwilder commented Aug 21, 2015

You can also create a cluster by incrementally joining new nodes to existing ones but you don't have to do it this way. For example, start one, then start new nodes passing -join node1:8088. Once you have more than 1 member in the cluster, new nodes can join any of those nodes to expand the cluster.

@beckettsean
Copy link
Contributor

Deleted prior comment, my understanding is actually incorrect.

@sebito91
Copy link
Contributor

@beckettsean, @jwilder: thanks for the feedback. Obviously the documentation is still under review and there are a few kinks to work out...completely understood. It makes a little more sense now that we can effectively daisy-chain members into the cluster as necessary (according to @jwilder).

The concern we had internally was that the -join logic was unclear...do we just specify the three nodes for raft consensus across all machines that would join the cluster? How would node 1000 work when specifying the same raft consensus set? What happens to the 1000 nodes in the cluster if the three in the raft consensus died?

We'll keep testing these features out and document any issues that come up.

@jwilder
Copy link
Contributor Author

jwilder commented Aug 24, 2015

@sebito91 You can specify any nodes to join. Using the same 3 nodes every time is fine even with larger clusters.

If a raft node dies and will never come back, it would need to be replaced. If it just goes offline, it'll join the raft group when it restarts. With 3 raft nodes in the cluster, the cluster can support one raft node offline before availability is affected. With 5, it can support 2 failed. To replace a failed raft node, it's a manual process right now which still needs to be documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants