From ee2a7c4ef3af6a56da5da302a39657d1fb58dc1a Mon Sep 17 00:00:00 2001 From: Julien Maffre <42961061+jumaffre@users.noreply.github.com> Date: Wed, 26 Aug 2020 10:58:16 +0100 Subject: [PATCH] Fix Raft term history on backup (#1531) --- src/consensus/raft/raft.h | 15 ++++++++++++++- tests/e2e_logging.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/consensus/raft/raft.h b/src/consensus/raft/raft.h index 10c98e342b..45eaeb1107 100644 --- a/src/consensus/raft/raft.h +++ b/src/consensus/raft/raft.h @@ -546,6 +546,7 @@ namespace raft LOG_FAIL_FMT(err.what()); return; } + LOG_DEBUG_FMT( "Received pt: {} pi: {} t: {} i: {}", r.prev_term, @@ -618,7 +619,7 @@ namespace raft if (r.prev_idx < commit_idx) { LOG_DEBUG_FMT( - "Recv append entries to {} from {} but prev_idex ({}) < commit_idx " + "Recv append entries to {} from {} but prev_idx ({}) < commit_idx " "({})", local_id, r.from_node, @@ -684,22 +685,34 @@ namespace raft switch (deserialise_success) { case kv::DeserialiseSuccess::FAILED: + { throw std::logic_error( "Follower failed to apply log entry " + std::to_string(i)); break; + } case kv::DeserialiseSuccess::PASS_SIGNATURE: + { LOG_DEBUG_FMT("Deserialising signature at {}", i); committable_indices.push_back(i); + if (sig_term) + { term_history.update(commit_idx + 1, sig_term); + commit_if_possible(r.leader_commit_idx); + } break; + } case kv::DeserialiseSuccess::PASS: + { break; + } default: + { throw std::logic_error("Unknown DeserialiseSuccess value"); + } } } diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 45c9fbdcdb..f7fb8f9daa 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -445,6 +445,8 @@ def test_view_history(network, args): check = infra.checker.Checker() + previous_node = None + previous_tx_ids = "" for node in network.get_joined_nodes(): with node.client("user0") as c: r = c.get("/node/commit") @@ -491,6 +493,18 @@ def test_view_history(network, args): f"Node {node.node_id}: Incomplete or inconsistent view history" ) + # Compare view history between nodes + if previous_tx_ids: + # Some nodes may have a slightly longer view history so only compare the common prefix + min_tx_ids_len = min(len(previous_tx_ids), len(tx_ids_condensed)) + assert ( + tx_ids_condensed[:min_tx_ids_len] + == previous_tx_ids[:min_tx_ids_len] + ), f"Tx IDs don't match between node {node.node_id} and node {previous_node.node_id}: {tx_ids_condensed[:min_tx_ids_len]} and {previous_tx_ids[:min_tx_ids_len]}" + + previous_tx_ids = tx_ids_condensed + previous_node = node + return network