Enable pre-vote by default

CHANGELOG.md

@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 [7.0.0-dev6]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.0-dev6
 
+### Added
+
+- PreVote optimistaion enabled. This requires that a follower checks that it could be elected before becoming a candidate. This optimisation improves the availablilty of Raft when there are omission faults like partial network partitions. (#7462)
+
 ### Changed
 
 - Start nodes now confirm that read-only ledger directories are empty on startup (#7355).

src/consensus/aft/impl/state.h

@@ -149,7 +149,7 @@ namespace aft
 
   struct State
   {
-    State(ccf::NodeId node_id_, bool pre_vote_enabled_ = false) :
+    State(ccf::NodeId node_id_, bool pre_vote_enabled_ = true) :
       node_id(std::move(node_id_)),
       pre_vote_enabled(pre_vote_enabled_)
     {}
@@ -191,7 +191,7 @@ namespace aft
     // that index itself is committed
     std::optional<ccf::SeqNo> retired_committed_idx = std::nullopt;
 
-    bool pre_vote_enabled = false;
+    bool pre_vote_enabled = true;
   };
   DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(State);
   DECLARE_JSON_REQUIRED_FIELDS(

src/consensus/aft/test/committable_suffix.cpp

@@ -177,6 +177,15 @@ DOCTEST_TEST_CASE("Retention of dead leader's commit")
   {
     rA.periodic(election_timeout);
 
+    // Dispatch RequestPreVotes
+    DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idA, channelsA->messages));
+
+    // Dispatch responses
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idB, channelsB->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idC, channelsC->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idD, channelsD->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idE, channelsE->messages));
+
     // Dispatch RequestVotes
     DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idA, channelsA->messages));
 
@@ -335,6 +344,14 @@ DOCTEST_TEST_CASE("Retention of dead leader's commit")
   {
     rB.periodic(election_timeout);
 
+    // Dispatch RequestPreVotes
+    DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idB, channelsB->messages));
+
+    // Dispatch responses
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idC, channelsC->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idD, channelsD->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idE, channelsE->messages));
+
     // Dispatch RequestVotes
     DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idB, channelsB->messages));
 
@@ -374,6 +391,14 @@ DOCTEST_TEST_CASE("Retention of dead leader's commit")
   {
     rC.periodic(election_timeout);
 
+    // Dispatch RequestPreVotes
+    DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idC, channelsC->messages));
+
+    // Dispatch responses
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idB, channelsB->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idD, channelsD->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idE, channelsE->messages));
+
     // Dispatch RequestVotes
     DOCTEST_REQUIRE(4 == dispatch_all(nodes, node_idC, channelsC->messages));
 
@@ -512,6 +537,10 @@ DOCTEST_TEST_CASE_TEMPLATE("Multi-term divergence", T, WorstCase, RandomCase)
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idB));
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idC));
 
+    DOCTEST_REQUIRE(2 == dispatch_all(nodes, node_idA));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idB));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idC));
+
     DOCTEST_REQUIRE(rA.is_primary());
     DOCTEST_REQUIRE(rA.get_view() == 1);
 
@@ -578,6 +607,18 @@ DOCTEST_TEST_CASE_TEMPLATE("Multi-term divergence", T, WorstCase, RandomCase)
 
     primary.periodic(election_timeout);
 
+    // RequestPreVote is only sent to Node C
+    keep_messages_for(node_idC, channels_primary->messages);
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, primary_id));
+
+    // Node C pre-votes in favour
+    DOCTEST_REQUIRE(
+      1 ==
+      dispatch_all_and_DOCTEST_CHECK<aft::RequestPreVoteResponse>(
+        nodes, node_idC, [](const aft::RequestPreVoteResponse& rvr) {
+          DOCTEST_REQUIRE(rvr.vote_granted == true);
+        }));
+
     // RequestVote is only sent to Node C
     keep_messages_for(node_idC, channels_primary->messages);
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, primary_id));
@@ -755,6 +796,10 @@ DOCTEST_TEST_CASE_TEMPLATE("Multi-term divergence", T, WorstCase, RandomCase)
       {
         channelsA->messages.clear();
         rA.periodic(election_timeout);
+
+        // Attempt pre-vote
+        dispatch_all(nodes, node_idA);
+        dispatch_all(nodes, node_idC);
       }
     }
     else
@@ -765,15 +810,25 @@ DOCTEST_TEST_CASE_TEMPLATE("Multi-term divergence", T, WorstCase, RandomCase)
       {
         channelsB->messages.clear();
         rB.periodic(election_timeout);
+
+        // Attempt pre-vote
+        dispatch_all(nodes, node_idB);
+        dispatch_all(nodes, node_idC);
       }
     }
 
     auto& rPrimary = aim_for_a_primary ? rA : rB;
     const auto id_primary = aim_for_a_primary ? node_idA : node_idB;
     auto& channelsPrimary = aim_for_a_primary ? channelsA : channelsB;
 
+    // PreVote
     dispatch_all(nodes, id_primary);
     dispatch_all(nodes, node_idC);
+    // Election
+    dispatch_all(nodes, id_primary);
+    dispatch_all(nodes, node_idC);
+
+    DOCTEST_REQUIRE(rPrimary.is_primary());
 
     {
       DOCTEST_INFO("Catch node C up");

src/consensus/aft/test/driver.h

@@ -117,7 +117,7 @@ class RaftDriver
     std::shared_ptr<TRaft> raft;
   };
 
-  bool pre_vote_enabled = false;
+  bool pre_vote_enabled = true;
   std::map<ccf::NodeId, NodeDriver> _nodes;
   std::set<std::pair<ccf::NodeId, ccf::NodeId>> _connections;
 

src/consensus/aft/test/main.cpp

@@ -146,6 +146,73 @@ DOCTEST_TEST_CASE(
 
   r0.start_ticking();
   r0.periodic(election_timeout * 2);
+  DOCTEST_REQUIRE(
+    r0c->count_messages_with_type(aft::RaftMsgType::raft_request_pre_vote) ==
+    3);
+
+  DOCTEST_INFO("Node 1 receives the request pre-vote");
+
+  auto rpv_raw =
+    r0c->pop_first(aft::RaftMsgType::raft_request_pre_vote, node_id1);
+  DOCTEST_REQUIRE(rpv_raw.has_value());
+  {
+    auto rpv = *(aft::RequestPreVote*)rpv_raw->data();
+    DOCTEST_REQUIRE(rpv.term == 0);
+    DOCTEST_REQUIRE(rpv.last_committable_idx == 0);
+    DOCTEST_REQUIRE(
+      rpv.term_of_last_committable_idx == aft::ViewHistory::InvalidView);
+  }
+
+  receive_message(r0, r1, *rpv_raw);
+
+  DOCTEST_INFO("Node 2 receives the request pre-vote");
+
+  rpv_raw = r0c->pop_first(aft::RaftMsgType::raft_request_pre_vote, node_id2);
+  DOCTEST_REQUIRE(rpv_raw.has_value());
+  {
+    auto rpv = *(aft::RequestPreVote*)rpv_raw->data();
+    DOCTEST_REQUIRE(rpv.term == 0);
+    DOCTEST_REQUIRE(rpv.last_committable_idx == 0);
+    DOCTEST_REQUIRE(
+      rpv.term_of_last_committable_idx == aft::ViewHistory::InvalidView);
+  }
+
+  receive_message(r0, r2, *rpv_raw);
+
+  DOCTEST_INFO("Node 1 pre-votes for Node 0");
+
+  DOCTEST_REQUIRE(
+    r1c->count_messages_with_type(
+      aft::RaftMsgType::raft_request_pre_vote_response) == 1);
+
+  auto rpvr_raw =
+    r1c->pop_first(aft::RaftMsgType::raft_request_pre_vote_response, node_id0);
+  DOCTEST_REQUIRE(rpvr_raw.has_value());
+  {
+    auto rvrc = *(aft::RequestPreVoteResponse*)rpvr_raw->data();
+    DOCTEST_REQUIRE(rvrc.term == 0);
+    DOCTEST_REQUIRE(rvrc.vote_granted);
+  }
+
+  receive_message(r1, r0, *rpvr_raw);
+
+  DOCTEST_INFO("Node 2 pre-votes for Node 0");
+
+  DOCTEST_REQUIRE(
+    r2c->count_messages_with_type(
+      aft::RaftMsgType::raft_request_pre_vote_response) == 1);
+
+  rpvr_raw =
+    r2c->pop_first(aft::RaftMsgType::raft_request_pre_vote_response, node_id0);
+  DOCTEST_REQUIRE(rpvr_raw.has_value());
+  {
+    auto rvrc = *(aft::RequestPreVoteResponse*)rpvr_raw->data();
+    DOCTEST_REQUIRE(rvrc.term == 0);
+    DOCTEST_REQUIRE(rvrc.vote_granted);
+  }
+
+  receive_message(r2, r0, *rpvr_raw);
+
   DOCTEST_REQUIRE(
     r0c->count_messages_with_type(aft::RaftMsgType::raft_request_vote) == 3);
 
@@ -322,6 +389,13 @@ DOCTEST_TEST_CASE(
   r0.start_ticking();
   r0.periodic(election_timeout * 2);
 
+  DOCTEST_INFO("Send request_pre_votes to other nodes");
+  DOCTEST_REQUIRE(2 == dispatch_all(nodes, node_id0, r0c->messages));
+
+  DOCTEST_INFO("Send request_pre_vote_reponses back");
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id2, r2c->messages));
+
   DOCTEST_INFO("Send request_votes to other nodes");
   DOCTEST_REQUIRE(2 == dispatch_all(nodes, node_id0, r0c->messages));
 
@@ -456,6 +530,10 @@ DOCTEST_TEST_CASE("Multiple nodes late join" * doctest::test_suite("multiple"))
   r0.start_ticking();
   r0.periodic(election_timeout * 2);
 
+  // Pre-vote
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
+  // Vote
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
@@ -567,6 +645,10 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple"))
 
   DOCTEST_INFO("Initial election");
   {
+    // Pre-vote
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
+    // Vote
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
 
@@ -825,6 +907,10 @@ DOCTEST_TEST_CASE("Exceed append entries limit")
   r0.start_ticking();
   r0.periodic(election_timeout * 2);
 
+  // Pre-vote
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
+  DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
+  // Vote
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
   DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
@@ -983,6 +1069,10 @@ DOCTEST_TEST_CASE(
 
   DOCTEST_INFO("Initial election");
   {
+    // Pre-vote
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
+    DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
+    // Vote
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id0, r0c->messages));
     DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_id1, r1c->messages));
 
@@ -997,15 +1087,17 @@ DOCTEST_TEST_CASE(
     "because it isn't ticking yet");
   r1.periodic(election_timeout * 2);
   DOCTEST_REQUIRE(
-    r1c->count_messages_with_type(aft::RaftMsgType::raft_request_vote) == 0);
+    r1c->count_messages_with_type(aft::RaftMsgType::raft_request_pre_vote) ==
+    0);
 
   r1.start_ticking();
   DOCTEST_INFO(
     "Node 1 is now ticking, exceeds its election timeout and so calls an "
     "election");
   r1.periodic(election_timeout * 2);
   DOCTEST_REQUIRE(
-    r1c->count_messages_with_type(aft::RaftMsgType::raft_request_vote) == 1);
+    r1c->count_messages_with_type(aft::RaftMsgType::raft_request_pre_vote) ==
+    1);
 }
 
 int main(int argc, char** argv)