tick: Don't convert to candidate while a snapshot is installing

Signed-off-by: Cole Miller <cole.miller@canonical.com>

Author: cole-miller

src/replication.c

@@ -1201,6 +1201,9 @@ static void installSnapshotCb(struct raft_io_snapshot_put *req, int status)
     struct raft_append_entries_result result;
     int rv;
 
+    /* We avoid converting to candidate state while installing a snapshot. */
+    assert(r->state == RAFT_FOLLOWER || r->state == RAFT_UNAVAILABLE);
+
     r->snapshot.put.data = NULL;
 
     result.term = r->current_term;
@@ -1246,9 +1249,6 @@ static void installSnapshotCb(struct raft_io_snapshot_put *req, int status)
     raft_configuration_close(&snapshot->configuration);
 
 respond:
-    /* TODO Investigate when and if a RAFT_FOLLOWER moves to RAFT_CANDIDATE
-     * during the installation of a snapshot.
-     * See https://github.com/canonical/raft/issues/343 */
     if (r->state == RAFT_FOLLOWER) {
         result.last_log_index = r->last_stored;
         sendAppendEntriesResult(r, &result);

src/tick.c

@@ -41,6 +41,11 @@ static int tickFollower(struct raft *r)
      *   current leader or granting vote to candidate, convert to candidate.
      */
     if (electionTimerExpired(r) && server->role == RAFT_VOTER) {
+        if (r->snapshot.put.data != NULL) {
+            tracef("installing snapshot -> don't convert to candidate");
+            electionResetTimer(r);
+            return 0;
+        }
         tracef("convert to candidate and start new election");
         rv = convertToCandidate(r, false /* disrupt leader */);
         if (rv != 0) {

test/integration/test_snapshot.c

@@ -717,3 +717,45 @@ TEST(snapshot, takeSnapshotFail, setUp, tearDown, 0, fsm_snapshot_async_params)
     /* No crash or leaks have occurred */
     return MUNIT_OK;
 }
+
+/* A follower doesn't convert to candidate state while it's installing a snapshot. */
+TEST(snapshot, snapshotBlocksCandidate, setUp, tearDown, 0, NULL)
+{
+    struct fixture *f = data;
+    (void)params;
+
+    /* Set very low threshold and trailing entries number */
+    SET_SNAPSHOT_THRESHOLD(3);
+    SET_SNAPSHOT_TRAILING(1);
+    SET_SNAPSHOT_TIMEOUT(200);
+
+    raft_set_election_timeout(CLUSTER_RAFT(2), 600);
+
+    /* Apply a few of entries, to force a snapshot to be taken. Drop all network
+     * traffic between servers 0 and 2 in order for AppendEntries RPCs to not be
+     * replicated */
+    fprintf(stderr, "saturating\n");
+    CLUSTER_SATURATE_BOTHWAYS(0, 2);
+    CLUSTER_MAKE_PROGRESS;
+    CLUSTER_MAKE_PROGRESS;
+    CLUSTER_MAKE_PROGRESS;
+
+    /* Reconnect both servers and set a high disk latency on server 2 */
+    CLUSTER_SET_DISK_LATENCY(2, 2000);
+    CLUSTER_DESATURATE_BOTHWAYS(0, 2);
+    fprintf(stderr, "desaturating\n");
+
+    /* Wait a while and check that the leader has sent a snapshot */
+    CLUSTER_STEP_UNTIL_ELAPSED(500);
+    munit_assert_int(CLUSTER_N_SEND(0, RAFT_IO_INSTALL_SNAPSHOT), ==, 1);
+    munit_assert_int(CLUSTER_N_RECV(2, RAFT_IO_INSTALL_SNAPSHOT), ==, 1);
+
+    /* Disconnect the servers again so that heartbeats, etc. won't arrive */
+    fprintf(stderr, "resaturating\n");
+    CLUSTER_SATURATE_BOTHWAYS(0, 2);
+    munit_assert_int(CLUSTER_STATE(2), ==, RAFT_FOLLOWER);
+    munit_assert_ptr(CLUSTER_RAFT(2)->snapshot.put.data, !=, NULL);
+    CLUSTER_STEP_UNTIL_ELAPSED(1500);
+    munit_assert_int(CLUSTER_STATE(2), ==, RAFT_FOLLOWER);
+    return MUNIT_OK;
+}