Re: [Tarantool-patches] [PATCH v4 09/12] raft: introduce raft_start/stop_candidate

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Good job on the patch!

You didn't cover the stop_candidate call with demote flag true in the
unit tests. I added my commit on top of this one. See it on the branch
and below. Squash if you agree. Otherwise lets discuss.

====================
diff --git a/test/unit/raft.c b/test/unit/raft.c
index a718ab3f4..29e48da13 100644
--- a/test/unit/raft.c
+++ b/test/unit/raft.c
@@ -1270,29 +1270,50 @@ raft_test_too_long_wal_write(void)
 static void
 raft_test_start_stop_candidate(void)
 {
-	raft_start_test(4);
+	raft_start_test(8);
 	struct raft_node node;
 	raft_node_create(&node);
 
 	raft_node_cfg_is_candidate(&node, false);
 	raft_node_cfg_election_quorum(&node, 1);
 
-	raft_start_candidate(&node.raft);
+	raft_node_start_candidate(&node);
 	raft_run_next_event();
 	is(node.raft.state, RAFT_STATE_LEADER, "became leader after "
-					       "start_candidate");
-	raft_stop_candidate(&node.raft, false);
+	   "start candidate");
+
+	raft_node_stop_candidate(&node);
 	raft_run_for(node.cfg_death_timeout);
 	is(node.raft.state, RAFT_STATE_LEADER, "remain leader after "
-					       "stop_candidate");
+	   "stop candidate");
+
+	raft_node_demote_candidate(&node);
+	is(node.raft.state, RAFT_STATE_FOLLOWER, "demote drops a non-candidate "
+	   "leader to a follower");
+
+	/*
+	 * Ensure the non-candidate leader is demoted when sees a new term, and
+	 * does not try election again.
+	 */
+	raft_node_start_candidate(&node);
+	raft_run_next_event();
+	raft_node_stop_candidate(&node);
+	is(node.raft.state, RAFT_STATE_LEADER, "non-candidate but still "
+	   "leader");
 
 	is(raft_node_send_vote_request(&node,
-		3 /* Term. */,
+		4 /* Term. */,
 		"{}" /* Vclock. */,
 		2 /* Source. */
 	), 0, "vote request from 2");
 	is(node.raft.state, RAFT_STATE_FOLLOWER, "demote once new election "
-						 "starts");
+	   "starts");
+
+	raft_run_for(node.cfg_election_timeout * 2);
+	is(node.raft.state, RAFT_STATE_FOLLOWER, "still follower");
+	is(node.raft.term, 4, "still the same term");
+
+	raft_node_destroy(&node);
 	raft_finish_test();
 }
 
diff --git a/test/unit/raft.result b/test/unit/raft.result
index f9a8f249b..3a3dc5dd2 100644
--- a/test/unit/raft.result
+++ b/test/unit/raft.result
@@ -221,11 +221,15 @@ ok 12 - subtests
 ok 13 - subtests
 	*** raft_test_too_long_wal_write: done ***
 	*** raft_test_start_stop_candidate ***
-    1..4
-    ok 1 - became leader after start_candidate
-    ok 2 - remain leader after stop_candidate
-    ok 3 - vote request from 2
-    ok 4 - demote once new election starts
+    1..8
+    ok 1 - became leader after start candidate
+    ok 2 - remain leader after stop candidate
+    ok 3 - demote drops a non-candidate leader to a follower
+    ok 4 - non-candidate but still leader
+    ok 5 - vote request from 2
+    ok 6 - demote once new election starts
+    ok 7 - still follower
+    ok 8 - still the same term
 ok 14 - subtests
 	*** raft_test_start_stop_candidate: done ***
 	*** main_f: done ***
diff --git a/test/unit/raft_test_utils.c b/test/unit/raft_test_utils.c
index b8735f373..452c05c81 100644
--- a/test/unit/raft_test_utils.c
+++ b/test/unit/raft_test_utils.c
@@ -387,6 +387,27 @@ raft_node_unblock(struct raft_node *node)
 	}
 }
 
+void
+raft_node_start_candidate(struct raft_node *node)
+{
+	assert(raft_node_is_started(node));
+	raft_start_candidate(&node->raft);
+}
+
+void
+raft_node_stop_candidate(struct raft_node *node)
+{
+	assert(raft_node_is_started(node));
+	raft_stop_candidate(&node->raft, false);
+}
+
+void
+raft_node_demote_candidate(struct raft_node *node)
+{
+	assert(raft_node_is_started(node));
+	raft_stop_candidate(&node->raft, true);
+}
+
 void
 raft_node_cfg_is_enabled(struct raft_node *node, bool value)
 {
diff --git a/test/unit/raft_test_utils.h b/test/unit/raft_test_utils.h
index bc3db0c2a..5f8618716 100644
--- a/test/unit/raft_test_utils.h
+++ b/test/unit/raft_test_utils.h
@@ -208,6 +208,23 @@ raft_node_block(struct raft_node *node);
 void
 raft_node_unblock(struct raft_node *node);
 
+/**
+ * Make the node candidate, and maybe start election if a leader is not known.
+ */
+void
+raft_node_start_candidate(struct raft_node *node);
+
+/**
+ * Make the node non-candidate for next elections, but if it is a leader right
+ * now, it will stay a leader.
+ */
+void
+raft_node_stop_candidate(struct raft_node *node);
+
+/** Stop the candidate and remove its leader role if present. */
+void
+raft_node_demote_candidate(struct raft_node *node);
+
 /** Configuration methods. */
 
 void