Skip to content

Commit

Permalink
raft: fix spurious split-vote
Browse files Browse the repository at this point in the history
Due to a typo raft candidate counted a vote for another node as a vote
for self in its split-vote detector. This could lead to spurious
split-vote detection in cases when another node wins elections with a bare
minimum of votes for it (exactly a quorum of votes).

Closes #8698

NO_DOC=bugfix

(cherry picked from commit 2afde5b)
  • Loading branch information
sergepetrenko committed May 29, 2023
1 parent 27c550c commit 3e0229f
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 3 deletions.
5 changes: 5 additions & 0 deletions changelogs/unreleased/gh-8698-spurious-split-vote.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
## bugfix/replication

* Fixed nodes configured with `election_mode = 'candidate'` spuriously detecting
a split-vote when another candidate should win with exactly a quorum of votes
for it (gh-8698).
2 changes: 1 addition & 1 deletion src/lib/raft/raft.c
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ raft_sm_schedule_new_vote(struct raft *raft, uint32_t candidate_id,
assert(!raft->votes[raft->self].did_vote);
raft->volatile_vote = candidate_id;
vclock_copy(&raft->candidate_vclock, candidate_vclock);
raft_add_vote(raft, raft->self, raft->self);
raft_add_vote(raft, raft->self, candidate_id);
raft_sm_pause_and_dump(raft);
/* Nothing visible is changed - no broadcast. */
}
Expand Down
31 changes: 30 additions & 1 deletion test/unit/raft.c
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ raft_test_bump_term_before_cfg()
static void
raft_test_split_vote(void)
{
raft_start_test(64);
raft_start_test(67);
struct raft_node node;
raft_node_create(&node);

Expand Down Expand Up @@ -2032,6 +2032,35 @@ raft_test_split_vote(void)

is(node.raft.timer.repeat, 0, "still waiting for yield");

/*
* gh-8698: a candidate might erroneously discover a split vote when
* simply voting for another node.
*/
raft_node_destroy(&node);
raft_node_create(&node);
raft_node_cfg_cluster_size(&node, 2);
raft_node_cfg_election_quorum(&node, 2);

is(raft_node_send_vote_request(
&node,
2 /* Term. */,
"{}" /* Vclock. */,
2 /* Source. */
), 0, "vote for 2");

ok(raft_node_check_full_state(
&node,
RAFT_STATE_FOLLOWER /* State. */,
0 /* Leader. */,
2 /* Term. */,
2 /* Vote. */,
2 /* Volatile term. */,
2 /* Volatile vote. */,
"{0: 2}" /* Vclock. */
), "term and vote are persisted");
ok(node.raft.timer.repeat >= node.raft.election_timeout,
"no split vote");

raft_node_destroy(&node);
raft_finish_test();
}
Expand Down
5 changes: 4 additions & 1 deletion test/unit/raft.result
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ ok 15 - subtests
ok 16 - subtests
*** raft_test_bump_term_before_cfg: done ***
*** raft_test_split_vote ***
1..64
1..67
ok 1 - elections with a new term
ok 2 - vote response for 1 from 2
ok 3 - vote response for 3 from 3
Expand Down Expand Up @@ -363,6 +363,9 @@ ok 16 - subtests
ok 62 - planned new election after yield
ok 63 - vote response for 3 from 3
ok 64 - still waiting for yield
ok 65 - vote for 2
ok 66 - term and vote are persisted
ok 67 - no split vote
ok 17 - subtests
*** raft_test_split_vote: done ***
*** raft_test_pre_vote ***
Expand Down

0 comments on commit 3e0229f

Please sign in to comment.