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

Forbid removed node from being a Candidate #476

Merged
merged 9 commits into from Oct 7, 2021
Merged

Conversation

dhiaayachi
Copy link
Contributor

@dhiaayachi dhiaayachi commented Sep 22, 2021

When a follower is removed from the cluster it's removed from the configuration servers list but still run a follower loop (originally a follower node or ShutdownOnRemove set to false for Leader). When that node timeout (because it's not receiving heartbeat from the other nodes in the cluster), it transition to a Candidate state and try to trigger an election. 2 possible cases:

  • the cluster is stable and have a leader: the vote request will be rejected
  • the cluster is unstable and don't have a leader: the vote is granted and the removed node will be part of the voting nodes.

This uncover 2 issue:

  • a removed node should not transition to a Candidate state. This is fixed in this PR
  • a node that is not part of the cluster should not be allowed to vote: this a bit trickier to fix because the RequestVoteRequest only have the ServerAddress which do not represent a unique ID of the node, the one stored in the configuration server list. A test that reproduce the issue is in node not part of the cluster is not allowed to vote #477

@dhiaayachi dhiaayachi changed the title Forbid removed node from being a Candidate or vote Forbid removed node from being a Candidate Sep 29, 2021
Copy link
Member

@mkeeler mkeeler left a comment

Choose a reason for hiding this comment

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

LGTM with one minor comment about a warning log.

raft.go Outdated Show resolved Hide resolved
@dhiaayachi dhiaayachi merged commit e55a8bf into main Oct 7, 2021
@dhiaayachi dhiaayachi deleted the removed-node-vote branch October 7, 2021 00:31
dhiaayachi added a commit that referenced this pull request Oct 7, 2021
* modify `TestRaft_RemoveFollower` to check removed node state

* add test to verify that a removed node is not able to vote

* do not transition to `Candidate` state if not part of stable configuration

* add some comments to the tests

* do not return if not transitioning state

* add wait loop to test

* remove test related to removed node voting

* check `inConfig` instead of `hasVote`

* updating warn log message to be more accurate
dhiaayachi added a commit that referenced this pull request Oct 12, 2021
* modify `TestRaft_RemoveFollower` to check removed node state

* add test to verify that a removed node is not able to vote

* do not transition to `Candidate` state if not part of stable configuration

* add some comments to the tests

* do not return if not transitioning state

* add wait loop to test

* remove test related to removed node voting

* check `inConfig` instead of `hasVote`

* updating warn log message to be more accurate
mkeeler pushed a commit that referenced this pull request Nov 3, 2021
* modify `TestRaft_RemoveFollower` to check removed node state

* add test to verify that a removed node is not able to vote

* do not transition to `Candidate` state if not part of stable configuration

* add some comments to the tests

* do not return if not transitioning state

* add wait loop to test

* remove test related to removed node voting

* check `inConfig` instead of `hasVote`

* updating warn log message to be more accurate
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

2 participants