Don't send AppendEntries inline while processing a NACK, wait for the next tick (#3151)

src/consensus/aft/raft.h

@@ -2679,12 +2679,12 @@ namespace aft
 
       if (r.success != AppendEntriesResponseType::OK)
       {
-        // Failed due to log inconsistency. Reset sent_idx and try again.
+        // Failed due to log inconsistency. Reset sent_idx, and try again soon.
         LOG_DEBUG_FMT(
           "Recv append entries response to {} from {}: failed",
           state->my_node_id,
           from);
-        send_append_entries(from, node->second.match_idx + 1);
+        node->second.sent_idx = node->second.match_idx;
         return;
       }
 

src/consensus/aft/test/committable_suffix.cpp

@@ -442,6 +442,7 @@ DOCTEST_TEST_CASE("Retention of dead leader's commit")
     {
       // Only the first AppendEntries to B is kept, all other
       // AppendEntries are lost
+      rC.periodic(request_timeout);
       keep_first_for(node_idB, channelsC->messages);
       DOCTEST_REQUIRE(1 == dispatch_all(nodes, node_idC, channelsC->messages));
 

src/consensus/aft/test/main.cpp

@@ -712,6 +712,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple"))
     aer.success = aft::AppendEntriesResponseType::FAIL;
     const auto p = reinterpret_cast<uint8_t*>(&aer);
     receive_message(r1, r0, {p, p + sizeof(aer)});
+    r0.periodic(request_timeout);
     DOCTEST_REQUIRE(r0c->messages.size() == 1);
 
     // Only the third entry is deserialised
@@ -761,6 +762,7 @@ DOCTEST_TEST_CASE("Recv append entries logic" * doctest::test_suite("multiple"))
     aer.success = aft::AppendEntriesResponseType::FAIL;
     const auto p = reinterpret_cast<uint8_t*>(&aer);
     receive_message(r1, r0, {p, p + sizeof(aer)});
+    r0.periodic(request_timeout);
     DOCTEST_REQUIRE(r0c->messages.size() == 1);
 
     // Receive append entries (idx: 5, prev_idx: 3)
@@ -951,13 +953,11 @@ DOCTEST_TEST_CASE("Exceed append entries limit")
   auto aer = r2c->messages.front().second;
   r2c->messages.pop_front();
   receive_message(r2, r0, aer);
+  r0.periodic(request_timeout);
 
   DOCTEST_REQUIRE(r0c->messages.size() > num_small_entries_sent);
-  DOCTEST_REQUIRE(
-    r0c->messages.size() <= num_small_entries_sent + num_big_entries);
   auto sent_entries = dispatch_all(nodes, node_id0, r0c->messages);
   DOCTEST_REQUIRE(sent_entries > num_small_entries_sent);
-  DOCTEST_REQUIRE(sent_entries <= num_small_entries_sent + num_big_entries);
   DOCTEST_REQUIRE(r2.ledger->ledger.size() == individual_entries);
 }
 

tests/raft_scenarios/fancy_election

@@ -68,6 +68,7 @@ dispatch_one,1
 dispatch_one,2
 
 # Node 1 sends its committable suffix
+periodic_all,10
 dispatch_one,1
 
 # Node 2 ACKs all of that, advancing commit on Node 1

tests/raft_scenarios/suffix_collision

@@ -66,5 +66,8 @@ dispatch_all
 periodic_all,10
 dispatch_all
 
+periodic_all,10
+dispatch_all
+
 state_all
 assert_state_sync