recv: Check remote term early on when handling RequestVote responses

When handling a RequestVote response, immediately check the term of the remote
peer, before doing anything else.

This avoids checking if we are in candidate state twice, once at the beginning
of the message handling and once after the term check, to handle the case where
we step down.

Also, decide whether to perform an actual term bump or just a check based on the
pre-vote flag of the RequestVote response being handled, and not based on
whether we are in pre-vote phase or not. That's because we always want to bump
our term if for any reason we discover an higher term (e.g. we receive a delayed
non pre-vote response containing a newer term while in the meantime we have
transitioned to pre-vote phase).

Signed-off-by: Free Ekanayaka <free@ekanayaka.io>

src/recv_request_vote_result.c

@@ -24,23 +24,11 @@ int recvRequestVoteResult(struct raft *r,
     assert(r != NULL);
     assert(id > 0);
 
-    votes_index = configurationIndexOfVoter(&r->configuration, id);
-    if (votes_index == r->configuration.n) {
-        infof("non-voting or unknown server -> reject");
-        return 0;
-    }
-
-    /* Ignore responses if we are not candidate anymore */
-    if (r->state != RAFT_CANDIDATE) {
-        assert(r->state == RAFT_LEADER || r->state == RAFT_FOLLOWER);
-        infof("local server is %s -> ignore", raft_state_name(r->state));
-        return 0;
-    }
-
-    /* If we're in the pre-vote phase, don't actually increment our term right
-     * now (we'll do it later, if we start the second phase), and also don't
-     * step down if the peer is just one term ahead (this is okay as in the
-     * request we sent our current term plus one). */
+    /* If this is a pre-vote result, don't actually increment our term right
+     * now, because the term included in this message is not necessarily the
+     * term the remote peer is at (pre-vote results contain the term that the
+     * peer would bump to if the request it receives was an actual request, and
+     * that term is typically our current term plus one). */
     if (r->candidate_state.in_pre_vote) {
         recvCheckMatchingTerms(r, result->term, &match);
     } else {
@@ -50,9 +38,16 @@ int recvRequestVoteResult(struct raft *r,
         }
     }
 
-    /* Converted to follower as a result of seeing a higher term. */
+    /* Ignore responses if we are not candidate anymore */
     if (r->state != RAFT_CANDIDATE) {
-        infof("no longer candidate -> ignore");
+        assert(r->state == RAFT_LEADER || r->state == RAFT_FOLLOWER);
+        infof("local server is %s -> ignore", raft_state_name(r->state));
+        return 0;
+    }
+
+    votes_index = configurationIndexOfVoter(&r->configuration, id);
+    if (votes_index == r->configuration.n) {
+        infof("non-voting or unknown server -> reject");
         return 0;
     }
 

test/integration/test_election.c

@@ -330,7 +330,7 @@ TEST_V1(election, RejectIfHigherTerm, setUp, tearDown, 0, NULL)
     CLUSTER_TRACE(
         "[ 120] 1 > recv request vote result from server 2\n"
         "           remote term is higher (3 vs 2) -> bump term, step down\n"
-        "           no longer candidate -> ignore\n");
+        "           local server is follower -> ignore\n");
 
     munit_assert_int(raft_state(CLUSTER_RAFT(1)), ==, RAFT_FOLLOWER);