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.

Signed-off-by: Li Wei <[email protected]>

src/raft_server.c

@@ -539,11 +539,6 @@ static int __should_grant_vote(raft_server_private_t* me, msg_requestvote_t* vr)
 {
     raft_node_t *my_node = raft_get_my_node((void*)me);
 
-    /* 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 (my_node && !raft_node_is_voting(my_node))
         return 0;
 
@@ -581,6 +576,13 @@ int raft_recv_requestvote(raft_server_t* me_,
     if (!node)
         node = raft_get_node(me_, vr->candidate_id);
 
+    /* Reject request if we have a leader */
+    if (me->leader_id != -1 && me->leader_id != raft_node_get_id(node) &&
+        me->timeout_elapsed < me->election_timeout) {
+        r->vote_granted = 0;
+        goto done;
+    }
+
     if (raft_get_current_term(me_) < vr->term)
     {
         e = raft_set_current_term(me_, vr->term);

tests/test_server.c

@@ -1076,6 +1076,46 @@ void TestRaft_server_recv_requestvote_dont_grant_vote_if_we_didnt_vote_for_this_
     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 *r = raft_new();
@@ -3909,7 +3949,11 @@ void TestRaft_leader_recv_requestvote_responds_without_granting(CuTest * tc)
     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 = {
         .persist_vote = __raft_persist_vote,
@@ -3944,6 +3988,7 @@ void TestRaft_leader_recv_requestvote_responds_with_granting_if_term_is_higher(C
     raft_recv_requestvote(r, raft_get_node(r, 3), &rv, &rvr);
     CuAssertTrue(tc, 1 == raft_is_follower(r));
 }
+#endif
 
 void TestRaft_leader_recv_appendentries_response_set_has_sufficient_logs_after_voting_committed(
     CuTest * tc)