Commit ab96a768 authored by Yossi Gottlieb's avatar Yossi Gottlieb Committed by Willem
Browse files

Ignore RequestVote when leader is established. (#58)

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 aa3586ff
...@@ -531,11 +531,6 @@ int raft_already_voted(raft_server_t* me_) ...@@ -531,11 +531,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;
...@@ -585,6 +580,13 @@ int raft_recv_requestvote(raft_server_t* me_, ...@@ -585,6 +580,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);
......
...@@ -1013,6 +1013,46 @@ void TestRaft_server_recv_requestvote_dont_grant_vote_if_we_didnt_vote_for_this_ ...@@ -1013,6 +1013,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();
...@@ -3828,7 +3868,11 @@ void TestRaft_leader_recv_requestvote_responds_without_granting(CuTest * tc) ...@@ -3828,7 +3868,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,
...@@ -3863,3 +3907,4 @@ void TestRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(C ...@@ -3863,3 +3907,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