Changing interaction between Leave and RemovePeers #621
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This LGTM. Clever fix! |
armon
added a commit
that referenced
this pull request
Jan 22, 2015
Changing interaction between Leave and RemovePeers
duckhan
pushed a commit
to duckhan/consul
that referenced
this pull request
Oct 24, 2021
Skip ent tests on fork PRs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #434. Previously, a graceful leave would just send the leave intention via Serf (gossip). The leader would detect the leave eventually and use that information to do a RemovePeer and catalog deregister. If the current leader was the one leaving, there was a race between the deregistration and the shutdown itself. Sometimes, the shutdown would happen first leaving the leader in the peer set. This was recoverable if quorum was not lost (e.g. >=3 servers). Other times deregister would happen, meaning quorum would not be lost for 2 node clusters. For a single node cluster, this RemovePeer against ourself was a bug. It would prevent the node from ever being able to establish leadership again when using -bootstrap or -bootstrap-expect.
This change prevents a node from ever processing a Serf leave intention about itself. This only affects a leave on the leader node. Instead, Leave() now does an explicit RemovePeer if we are the leader. This prevents the race condition, but allows us to guard against a single node cluster. For non-leaders, we now wait to ensure the RemovePeer can be replicated within a sane time window, preventing a potential quorum loss for small clusters.