Commit 56a1fb81 authored by Yossi Gottlieb's avatar Yossi Gottlieb
Browse files

Ignore RequestVote when leader is established.

This behavior is described in the original Raft paper and is required to
prevent removed nodes from disrupting the cluster.

This causes other problems though, as a split node may trigger election
which is rejected but leaves it with a future term.  The generally
accepted solution for this appears to be the use of PreVote RPC to avoid
election that can never succeed.
parent d1885191
...@@ -492,11 +492,6 @@ int raft_already_voted(raft_server_t* me_) ...@@ -492,11 +492,6 @@ int raft_already_voted(raft_server_t* me_)
static int __should_grant_vote(raft_server_private_t* me, msg_requestvote_t* vr) static int __should_grant_vote(raft_server_private_t* me, msg_requestvote_t* vr)
{ {
/* TODO: 4.2.3 Raft Dissertation:
* if a server receives a RequestVote request within the minimum election
* timeout of hearing from a current leader, it does not update its term or
* grant its vote */
if (!raft_node_is_voting(raft_get_my_node((void*)me))) if (!raft_node_is_voting(raft_get_my_node((void*)me)))
return 0; return 0;
...@@ -536,6 +531,13 @@ int raft_recv_requestvote(raft_server_t* me_, ...@@ -536,6 +531,13 @@ int raft_recv_requestvote(raft_server_t* me_,
if (!node) if (!node)
node = raft_get_node(me_, vr->candidate_id); node = raft_get_node(me_, vr->candidate_id);
/* Reject request if we have a leader */
if (me->current_leader && me->current_leader != node &&
(me->timeout_elapsed < me->election_timeout)) {
r->vote_granted = 0;
goto done;
}
if (raft_get_current_term(me_) < vr->term) if (raft_get_current_term(me_) < vr->term)
{ {
e = raft_set_current_term(me_, vr->term); e = raft_set_current_term(me_, vr->term);
......
...@@ -972,6 +972,46 @@ void TestRaft_server_recv_requestvote_dont_grant_vote_if_we_didnt_vote_for_this_ ...@@ -972,6 +972,46 @@ void TestRaft_server_recv_requestvote_dont_grant_vote_if_we_didnt_vote_for_this_
CuAssertTrue(tc, 0 == rvr.vote_granted); CuAssertTrue(tc, 0 == rvr.vote_granted);
} }
/* If requestvote is received within the minimum election timeout of
* hearing from a current leader, it does not update its term or grant its
* vote (6).
*/
void TestRaft_server_recv_requestvote_ignore_if_master_is_fresh(CuTest * tc)
{
raft_cbs_t funcs = { 0
};
void *r = raft_new();
raft_set_callbacks(r, &funcs, NULL);
raft_add_node(r, NULL, 1, 1);
raft_add_node(r, NULL, 2, 0);
raft_set_current_term(r, 1);
raft_set_election_timeout(r, 1000);
msg_appendentries_t ae = { 0 };
msg_appendentries_response_t aer;
ae.term = 1;
raft_recv_appendentries(r, raft_get_node(r, 2), &ae, &aer);
CuAssertTrue(tc, 1 == aer.success);
msg_requestvote_t rv = {
.term = 2,
.candidate_id = 3,
.last_log_idx = 0,
.last_log_term = 1
};
msg_requestvote_response_t rvr;
raft_recv_requestvote(r, raft_get_node(r, 3), &rv, &rvr);
CuAssertTrue(tc, 1 != rvr.vote_granted);
/* After election timeout passed, the same requestvote should be accepted */
raft_periodic(r, 1001);
raft_recv_requestvote(r, raft_get_node(r, 3), &rv, &rvr);
CuAssertTrue(tc, 1 == rvr.vote_granted);
}
void TestRaft_follower_becomes_follower_is_follower(CuTest * tc) void TestRaft_follower_becomes_follower_is_follower(CuTest * tc)
{ {
void *r = raft_new(); void *r = raft_new();
...@@ -3791,7 +3831,11 @@ void TestRaft_leader_recv_requestvote_responds_without_granting(CuTest * tc) ...@@ -3791,7 +3831,11 @@ void TestRaft_leader_recv_requestvote_responds_without_granting(CuTest * tc)
CuAssertTrue(tc, 0 == rvr.vote_granted); CuAssertTrue(tc, 0 == rvr.vote_granted);
} }
void TestRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(CuTest * tc) #if 0
/* This test is disabled because it violates the Raft paper's view on
* ignoring RequestVotes when a leader is established.
*/
void T_estRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(CuTest * tc)
{ {
raft_cbs_t funcs = { raft_cbs_t funcs = {
.persist_vote = __raft_persist_vote, .persist_vote = __raft_persist_vote,
...@@ -3826,3 +3870,4 @@ void TestRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(C ...@@ -3826,3 +3870,4 @@ void TestRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(C
raft_recv_requestvote(r, raft_get_node(r, 3), &rv, &rvr); raft_recv_requestvote(r, raft_get_node(r, 3), &rv, &rvr);
CuAssertTrue(tc, 1 == raft_is_follower(r)); CuAssertTrue(tc, 1 == raft_is_follower(r));
} }
#endif
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment