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

raft: reset votes when becomePreCandidate #8346

Conversation

lishuai87
Copy link
Contributor

@lishuai87 lishuai87 commented Aug 1, 2017

raft: reset votes when becomePreCandidate. because the poll method wouldn't update grant state when key exists.

here is a case:

  1. cluster with three node [A, B, C], all term are 2, with leader A and all logs are replicated.
  2. node A down. B and C start election at the same time (split votes), both become pre-candidate.
  3. C grant B's pre-vote, and B grant C's pre-vote. So, both B and C become candidate, with term 3.
  4. C reject B's vote, and B reject C's vote. So, B and C are still candidate. B record votes like (B, true) (C, false), C record votes like (B, false) (C, true)
  5. B election-timeout first, start pre-vote. C grant B's pre-vote.
  6. when B check poll state,it's still (B, true) (C, false). So, election will continue, and no one will win.

@xiang90
Copy link
Contributor

xiang90 commented Aug 2, 2017

need test

@lishuai87
Copy link
Contributor Author

add a new test case, TestPreVoteWithSplitVote

@@ -611,6 +611,7 @@ func (r *raft) becomePreCandidate() {
// but doesn't change anything else. In particular it does not increase
// r.Term or change r.Vote.
r.step = stepCandidate
r.votes = make(map[uint64]bool)
Copy link
Contributor

Choose a reason for hiding this comment

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

check line612? it says vote should not be changed for some reason. i do not feel it is safe to change the vote decision for one term either.

/cc @bdarnell

Copy link
Contributor

Choose a reason for hiding this comment

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

probably if pre-candidate times out, it needs to increase term to handle the potential vote spilt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vote is what we voted for, it's in hard state. while votes is what other nodes grant/reject us, it's a memory state.
we can't change vote, but can change votes when a pre-vote happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. I misread.

@xiang90
Copy link
Contributor

xiang90 commented Aug 3, 2017

Lgtm

@xiang90 xiang90 merged commit 033c0cb into etcd-io:master Aug 3, 2017
irfansharif added a commit to irfansharif/cockroach that referenced this pull request Aug 7, 2017
@lishuai87 lishuai87 deleted the shawnsli/reset-votes-when-become-pre-candidate branch August 22, 2017 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants