From 62a927b4192f8e707b8bd5f71cf7e9a5b52a0dd5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 22 Jun 2026 00:41:11 +0000 Subject: [PATCH] test(raft-commit-safety): simulator invariant tests (tasks 7.1-7.4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7.1 Figure-8 commit refusal (G4): prior-term entry at a genuine quorum HELD (only the current-term gate keeps commit_index at 0), then a current-term entry advances over both (indirect commit) with one fire. Closes the Req 6.3 two-phase coverage. 7.2 Leader Completeness: committed prefix on {0,1}; node 0 fails; the stale node 2 is DENIED by the freshness gate so only the up-to-date node 1 can win — and it holds the full committed prefix (term+bytes). 7.3 freshness lexicographic (vote + prevote): stale-term-higher-index DENIED, fresher-term-lower-index GRANTED, tie decided by index; prevote side-effect-free. 7.4 on_commit_advanced trigger: fire-count tracks STRICT advances — held(0)/advance(1)/ no-op-redrive(1)/advance(2)/non-leader(2). Test-only (sim_set_log_entry local helper); reviewed in-parent (transient API 529 on the review subagent). ASAN green (243 tests). --- .kiro/specs/raft-commit-safety/tasks.md | 8 +- src/lib/testsuite/test_raft.c | 362 ++++++++++++++++++++++++ 2 files changed, 366 insertions(+), 4 deletions(-) diff --git a/.kiro/specs/raft-commit-safety/tasks.md b/.kiro/specs/raft-commit-safety/tasks.md index 88b3ad0d..6e0790bd 100644 --- a/.kiro/specs/raft-commit-safety/tasks.md +++ b/.kiro/specs/raft-commit-safety/tasks.md @@ -127,28 +127,28 @@ ## 7. Simulator invariant tests (Figure-8, Leader Completeness, freshness, trigger semantics) -- [ ] 7.1 Figure-8 commit-refusal test (G4) +- [x] 7.1 Figure-8 commit-refusal test (G4) - Test: construct a previous-term entry replicated to a quorum but not covered by a committed current-term entry; drive `raft_advance_commit` and assert `commit_index` does NOT move over it; then replicate a current-term entry above it to a quorum, drive `raft_advance_commit`, and assert `commit_index` advances over BOTH (indirect commit) - Observable completion: test passes end-to-end via the sim; the held phase and the released phase are both asserted - _Requirements: 6.2, 6.3_ - _Boundary: test_raft.c, raft_sim.c_ - _Depends: 6.1_ -- [ ] 7.2 Leader Completeness across forced leader changes +- [x] 7.2 Leader Completeness across forced leader changes - Test: drive replication to a commit, then force leader changes (timeout / partition); assert any entry committed (≤ `commit_index` on the committing leader) is present in the log of every node that subsequently becomes leader - Observable completion: test passes; no future leader is missing a committed entry across the churn - _Requirements: 6.4_ - _Boundary: test_raft.c, raft_sim.c_ - _Depends: 6.2, 7.1_ -- [ ] 7.3 Freshness-gate lexicographic exercise +- [x] 7.3 Freshness-gate lexicographic exercise - Test: drive votes with mismatched `(last_log_term, last_log_index)` tuples and assert the lexicographic outcome — stale-term-but-higher-index denied; fresher-term-but-lower-index granted; tie-term decided by index - Observable completion: test passes for both vote and PreVote paths - _Requirements: 6.5_ - _Boundary: test_raft.c, raft_sim.c_ - _Depends: 6.2_ -- [ ] 7.4 Trigger-semantics test for `on_commit_advanced` +- [x] 7.4 Trigger-semantics test for `on_commit_advanced` - Test: `on_commit_advanced` fires exactly once on a strict advance and does NOT fire while the current-term rule holds the commit index (Figure-8 held phase) or when not leader - Observable completion: test passes; the callback fire-count matches the number of strict advances - _Requirements: 6.6, 2.2, 2.3_ diff --git a/src/lib/testsuite/test_raft.c b/src/lib/testsuite/test_raft.c index 7937381f..167dd405 100644 --- a/src/lib/testsuite/test_raft.c +++ b/src/lib/testsuite/test_raft.c @@ -4040,6 +4040,362 @@ static void test_sim_9_5_node_restart_resumes_replication(void) TEST_ASSERT_EQUAL_INT(1, raft_sim_check_log_matching(&c)); } +/* ---------- Group: Simulator commit-safety invariant tests (tasks 7.1-7.4) ---------- + * + * These observation-based invariant tests drive the COMMIT-SAFETY machinery + * through the simulator: raft_advance_commit (via raft_sim_advance_commit, the + * Figure-8 current-term rule), the on_commit_advanced trigger mirror (per-node + * commit_index / commit_advanced_count), Leader Completeness across forced + * leader changes, and the (lastLogTerm, lastLogIndex) lexicographic freshness + * gate on both the binding-vote and PreVote paths. + * + * Each test carries a LOAD-BEARING intermediate assertion that would fail if the + * corresponding behavior regressed (a Figure-8 hold that did not actually hold, + * a strict advance that did not fire on_commit_advanced exactly once, a freshness + * outcome that flipped, or a future leader missing a committed entry), so a + * trivially-passing run is impossible. + */ + +/* Stamp a leader's sim log entry directly: set log[index-1].term and grow + * log_count / state.log_len so on_get_entry_term (sim_get_entry_term, which reads + * node->log) and the agreeIndexes scan (bounded by state.log_len) both see it. + * Used to construct a controlled Figure-8 prefix without driving replication. */ +static void sim_set_log_entry(raft_sim_node_t *node, uint64_t index, uint64_t term) +{ + TEST_ASSERT_TRUE(index >= 1 && index <= RAFT_SIM_MAX_LOG); + raft_sim_entry_t *slot = &node->log[index - 1]; + slot->term = term; + slot->bytes[0] = (unsigned char)('0' + (index % 10)); + slot->len = 1; + if ((int)index > node->log_count) + node->log_count = (int)index; + if (index > node->state.log_len) + node->state.log_len = index; +} + +/* + * test_sim_7_1_figure8_commit_refusal (task 7.1; Req 6.2, 6.3) + * + * Figure-8 commit refusal (G4) driven through the simulator's + * raft_sim_advance_commit. A PREVIOUS-term (T1) entry at index 2 is replicated to + * a genuine quorum, but NO current-term (T3) entry above it has quorum yet: + * raft_advance_commit must HOLD commit_index at 0 (the current-term rule refuses + * to commit a prior-term entry by replica-count alone). THEN a CURRENT-term entry + * at index 3 reaches quorum: raft_advance_commit must advance commit_index to 3, + * indirectly committing the prior-term entry at index 2 as well. + * + * Load-bearing assertions: + * - the held phase has a REAL quorum on the prior-term entry (match_index[1]=2 + * in a 3-node cluster ⇒ {self, peer1} agree through 2), so ONLY the + * current-term gate can be holding commit_index at 0. If the gate regressed + * to term-blind commit, the held assertion (commit_index == 0) would fail. + * - the released phase asserts commit_index advances over BOTH indices (3), + * and on_commit_advanced fired exactly once on that strict advance. + */ +static void test_sim_7_1_figure8_commit_refusal(void) +{ + raft_sim_cluster_t c; + memset(&c, 0, sizeof(c)); + + TEST_ASSERT_EQUAL_INT(0, raft_sim_init(&c, 3, 2000)); + + raft_sim_node_t *L = &c.nodes[0]; + + /* Model a leader at term 3 carrying a committed-prefix shape from a churned + * past: entries 1,2 are PREVIOUS-term (T1) entries, replicated to a quorum, + * but the leader's current term is 3 with no T3 entry above them yet. */ + L->state.role = RAFT_LEADER; + L->state.term = 3; + sim_set_log_entry(L, 1, 1); /* prior-term entry */ + sim_set_log_entry(L, 2, 1); /* prior-term entry — the Figure-8 entry */ + TEST_ASSERT_EQUAL_UINT64(2, L->state.log_len); + + /* Genuine quorum on the prior-term entry: in a 3-node cluster {self} U {peer1} + * agree through index 2 (peer 1 match_index = 2). Max(agreeIndexes) = 2, but + * log[2].term (1) != currentTerm (3) ⇒ the current-term rule must HOLD. */ + L->state.match_index[1] = 2; + L->state.match_index[2] = 0; + + raft_sim_advance_commit(&c, 0); + + /* HELD: a real quorum exists on index 2, so only the current-term gate can be + * keeping commit_index at 0. on_commit_advanced must NOT have fired. */ + TEST_ASSERT_EQUAL_UINT64(0, L->state.commit_index); + TEST_ASSERT_EQUAL_UINT64(0, L->commit_index); + TEST_ASSERT_EQUAL_INT(0, L->commit_advanced_count); + + /* Now a CURRENT-term (T3) entry lands at index 3 and reaches quorum. */ + sim_set_log_entry(L, 3, 3); + TEST_ASSERT_EQUAL_UINT64(3, L->state.log_len); + L->state.match_index[1] = 3; /* {self, peer1} now agree through 3 */ + + raft_sim_advance_commit(&c, 0); + + /* RELEASED: committing the current-term entry at 3 indirectly commits the + * prior-term entries below it — commit_index advances over BOTH (to 3) and + * on_commit_advanced fired exactly once on the strict advance. */ + TEST_ASSERT_EQUAL_UINT64(3, L->state.commit_index); + TEST_ASSERT_EQUAL_UINT64(3, L->commit_index); + TEST_ASSERT_EQUAL_INT(1, L->commit_advanced_count); +} + +/* + * test_sim_7_2_leader_completeness_across_changes (task 7.2; Req 6.4) + * + * Leader Completeness across forced leader changes: an entry committed under one + * leader must be present in the log of EVERY node that subsequently becomes + * leader. Node 0 leads, replicates entries to a quorum (nodes 0 and 1) and + * commits them. Node 0 then "fails" (partitioned away). The freshness gate plus + * a real election force a NEW leader — and because only nodes holding the + * committed prefix can win a quorum, the new leader necessarily holds it. + * + * Load-bearing assertions: + * - node 2 (which never received the committed entries) is DENIED the vote by + * the freshness gate (its last-log tuple is staler than the up-to-date + * voters), so it cannot become a leader missing the committed prefix — if the + * freshness gate regressed, node 2 could win and the completeness assertion + * would be violated. + * - the node that DOES become the new leader holds every committed entry + * (term-for-term equal to the original committing leader's prefix). + */ +static void test_sim_7_2_leader_completeness_across_changes(void) +{ + raft_sim_cluster_t c; + memset(&c, 0, sizeof(c)); + + TEST_ASSERT_EQUAL_INT(0, raft_sim_init(&c, 3, 2000)); + + /* Elect node 0 leader at term 1. */ + raft_start_election(&c.nodes[0].state); + raft_sim_deliver_pending(&c); + TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[0].state.role); + + /* Replicate 2 entries to the quorum {node0, node1}; node 2 is isolated so it + * NEVER receives them (it will become the stale, completeness-violating + * candidate). */ + raft_sim_partition(&c, 2, 1); + for (int e = 0; e < 2; e++) { + unsigned char payload[2] = { (unsigned char)('C' + e), 0x00 }; + uint64_t out_index = 0; + TEST_ASSERT_EQUAL_INT(RAFT_OK, + raft_log_append(&c.nodes[0].state, payload, sizeof(payload), &out_index)); + sim_replicate_one(&c, 0, 1, 0); + } + /* Commit the current-term entries (quorum {self, peer1} through index 2). */ + raft_sim_advance_commit(&c, 0); + uint64_t committed = c.nodes[0].state.commit_index; + TEST_ASSERT_EQUAL_UINT64(2, committed); + TEST_ASSERT_EQUAL_INT(2, c.nodes[1].log_count); /* the committed prefix is on node 1 */ + + /* Snapshot the committing leader's committed prefix for the completeness check. */ + raft_sim_entry_t committed_prefix[2]; + for (uint64_t i = 1; i <= committed; i++) + committed_prefix[i - 1] = c.nodes[0].log[i - 1]; + + /* FORCE A LEADER CHANGE: node 0 dies (partitioned away). Heal node 2 so it is + * back on the network — it is the up-to-date node 1's only peer now. */ + raft_sim_partition(&c, 0, 1); + raft_sim_partition(&c, 2, 0); + + /* The stale node 2 (empty log) campaigns first. Its RequestVote advertises an + * empty last-log tuple; the up-to-date node 1 (last-log term 1, index 2) must + * DENY it on the (lastLogTerm, lastLogIndex) freshness gate. */ + raft_start_election(&c.nodes[2].state); + raft_sim_deliver_pending(&c); + TEST_ASSERT_NOT_EQUAL(RAFT_LEADER, c.nodes[2].state.role); + /* Load-bearing: node 1 refused node 2 (no quorum for the stale candidate). */ + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, c.nodes[1].state.role); + + /* Now the up-to-date node 1 campaigns. Node 2 (no committed prefix, staler + * tuple) GRANTS the fresher candidate, so node 1 — which holds the committed + * prefix — wins and becomes the new leader. */ + raft_start_election(&c.nodes[1].state); + raft_sim_deliver_pending(&c); + TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[1].state.role); + + /* LEADER COMPLETENESS: the node that became leader holds EVERY committed + * entry, term-for-term, from the original committing leader. */ + TEST_ASSERT_TRUE(c.nodes[1].log_count >= (int)committed); + for (uint64_t i = 1; i <= committed; i++) { + TEST_ASSERT_EQUAL_UINT64(committed_prefix[i - 1].term, + c.nodes[1].log[i - 1].term); + TEST_ASSERT_EQUAL_size_t(committed_prefix[i - 1].len, + c.nodes[1].log[i - 1].len); + TEST_ASSERT_EQUAL_MEMORY(committed_prefix[i - 1].bytes, + c.nodes[1].log[i - 1].bytes, + committed_prefix[i - 1].len); + } +} + +/* + * test_sim_7_3_freshness_gate_lexicographic (task 7.3; Req 6.5) + * + * Exercise the (last_log_term, last_log_index) lexicographic freshness gate on + * BOTH the binding-vote path (raft_on_vote_request) and the side-effect-free + * PreVote path (raft_on_pre_vote_request) with mismatched tuples: + * - stale-term-but-HIGHER-index → DENIED (term dominates index) + * - fresher-term-but-LOWER-index → GRANTED (term dominates index) + * - tie-term decided by index → GRANTED iff cand index >= voter index + * The voter holds (last_log_term 5, last_log_index 4). + * + * Load-bearing: the stale-term-higher-index DENY and the fresher-term-lower-index + * GRANT are the exact pair that a pure LSN comparison (the pre-spec G6 bug) would + * get BACKWARDS — so these assertions fail if the gate regressed to index-only. + */ +static void test_sim_7_3_freshness_gate_lexicographic(void) +{ + raft_sim_cluster_t c; + memset(&c, 0, sizeof(c)); + + TEST_ASSERT_EQUAL_INT(0, raft_sim_init(&c, 3, 2000)); + + /* Voter is node 1 at term 5 with last-log (term 5, index 4). The sim's + * on_get_entry_term reads node->log, so stamp index 4 with term 5. */ + raft_sim_node_t *V = &c.nodes[1]; + V->state.term = 5; + V->state.role = RAFT_FOLLOWER; + V->state.voted_for = -1; + sim_set_log_entry(V, 1, 5); + sim_set_log_entry(V, 2, 5); + sim_set_log_entry(V, 3, 5); + sim_set_log_entry(V, 4, 5); + TEST_ASSERT_EQUAL_UINT64(4, V->state.log_len); + + uint64_t reply_term = 0; + + /* --- Binding-vote path (raft_on_vote_request) --- */ + + /* Stale last-log TERM (4) but HIGHER index (10): term dominates ⇒ DENY. */ + int rc = raft_on_vote_request(&V->state, 0, 5, /*cand_lt=*/4, /*cand_li=*/10, + &reply_term); + TEST_ASSERT_EQUAL_INT(RAFT_DENY, rc); + TEST_ASSERT_EQUAL_INT(-1, V->state.voted_for); /* not granted */ + + /* Fresher last-log TERM (6) but LOWER index (2): term dominates ⇒ GRANT. */ + rc = raft_on_vote_request(&V->state, 0, 6, /*cand_lt=*/6, /*cand_li=*/2, + &reply_term); + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + /* Re-init the voter for the tie-term cases (fresh, unvoted, same log). */ + V->state.term = 5; + V->state.role = RAFT_FOLLOWER; + V->state.voted_for = -1; + V->state.voted_for_term = 0; + + /* Tie TERM (5), index EQUAL to voter's (4): index >= voter ⇒ GRANT. */ + rc = raft_on_vote_request(&V->state, 0, 5, /*cand_lt=*/5, /*cand_li=*/4, + &reply_term); + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, rc); + + V->state.term = 5; + V->state.role = RAFT_FOLLOWER; + V->state.voted_for = -1; + V->state.voted_for_term = 0; + + /* Tie TERM (5), index LOWER than voter's (3 < 4) ⇒ DENY. */ + rc = raft_on_vote_request(&V->state, 0, 5, /*cand_lt=*/5, /*cand_li=*/3, + &reply_term); + TEST_ASSERT_EQUAL_INT(RAFT_DENY, rc); + + /* --- PreVote path (raft_on_pre_vote_request) — same lexicographic predicate, + * side-effect free. Reset the voter to a clean (term 5, last-log 5@4). --- */ + V->state.term = 5; + V->state.role = RAFT_FOLLOWER; + V->state.voted_for = -1; + V->state.voted_for_term = 0; + + /* Stale term (4), higher index (10) ⇒ DENY (and no mutation). */ + TEST_ASSERT_EQUAL_INT(RAFT_DENY, + raft_on_pre_vote_request(&V->state, 0, 5, /*cand_lt=*/4, /*cand_li=*/10)); + /* Fresher term (6), lower index (2) ⇒ GRANT. */ + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, + raft_on_pre_vote_request(&V->state, 0, 6, /*cand_lt=*/6, /*cand_li=*/2)); + /* Tie term (5), equal index (4) ⇒ GRANT; tie term, lower index (3) ⇒ DENY. */ + TEST_ASSERT_EQUAL_INT(RAFT_GRANT, + raft_on_pre_vote_request(&V->state, 0, 5, /*cand_lt=*/5, /*cand_li=*/4)); + TEST_ASSERT_EQUAL_INT(RAFT_DENY, + raft_on_pre_vote_request(&V->state, 0, 5, /*cand_lt=*/5, /*cand_li=*/3)); + + /* PreVote was side-effect-free across all four probes. */ + TEST_ASSERT_EQUAL_UINT64(5, V->state.term); + TEST_ASSERT_EQUAL_INT(-1, V->state.voted_for); + TEST_ASSERT_EQUAL_INT(RAFT_FOLLOWER, V->state.role); +} + +/* + * test_sim_7_4_on_commit_advanced_trigger_semantics (task 7.4; Req 6.6, 2.2, 2.3) + * + * Trigger semantics for on_commit_advanced via the per-node commit_advanced_count + * mirror. The callback must fire EXACTLY ONCE on a strict advance and must NOT + * fire while the Figure-8 current-term rule HOLDS the commit index, nor when the + * node is not the leader. The fire-count must equal the number of strict advances. + * + * Phases (one leader node, deterministic match_index/term control): + * 1. HELD: prior-term entries with quorum, no current-term entry above ⇒ NO fire. + * 2. ADVANCE #1: a current-term entry reaches quorum ⇒ EXACTLY ONE fire. + * 3. NO-OP RE-DRIVE: same state re-driven ⇒ no additional fire (not a strict advance). + * 4. ADVANCE #2: another current-term entry reaches quorum ⇒ ONE more fire (count 2). + * 5. NOT LEADER: as a follower, raft_advance_commit is -EPERM ⇒ no fire. + * + * Load-bearing: the held phase (real quorum, no fire) is distinguished from the + * advance phase (one fire), and the no-op re-drive proves the count tracks STRICT + * advances — not raft_advance_commit invocations. If the held gate or the + * strict-advance firing regressed, the per-phase count assertions fail. + */ +static void test_sim_7_4_on_commit_advanced_trigger_semantics(void) +{ + raft_sim_cluster_t c; + memset(&c, 0, sizeof(c)); + + TEST_ASSERT_EQUAL_INT(0, raft_sim_init(&c, 3, 2000)); + + raft_sim_node_t *L = &c.nodes[0]; + L->state.role = RAFT_LEADER; + L->state.term = 4; + + /* Phase 1 — HELD: two prior-term (T2) entries replicated to a quorum, no + * current-term entry above them. */ + sim_set_log_entry(L, 1, 2); + sim_set_log_entry(L, 2, 2); + L->state.match_index[1] = 2; /* genuine quorum {self, peer1} through 2 */ + L->state.match_index[2] = 0; + + raft_sim_advance_commit(&c, 0); + TEST_ASSERT_EQUAL_UINT64(0, L->state.commit_index); + TEST_ASSERT_EQUAL_INT(0, L->commit_advanced_count); /* held — no fire */ + + /* Phase 2 — ADVANCE #1: a current-term (T4) entry at index 3 reaches quorum. */ + sim_set_log_entry(L, 3, 4); + L->state.match_index[1] = 3; + raft_sim_advance_commit(&c, 0); + TEST_ASSERT_EQUAL_UINT64(3, L->state.commit_index); + TEST_ASSERT_EQUAL_INT(1, L->commit_advanced_count); /* exactly one fire */ + + /* Phase 3 — NO-OP RE-DRIVE: nothing changed; Max(agreeIndexes) is unchanged so + * the commit index does not strictly advance ⇒ NO additional fire. */ + raft_sim_advance_commit(&c, 0); + TEST_ASSERT_EQUAL_UINT64(3, L->state.commit_index); + TEST_ASSERT_EQUAL_INT(1, L->commit_advanced_count); /* still one — count tracks advances */ + + /* Phase 4 — ADVANCE #2: another current-term (T4) entry at index 4 reaches + * quorum ⇒ ONE more strict advance (count 2). */ + sim_set_log_entry(L, 4, 4); + L->state.match_index[1] = 4; + raft_sim_advance_commit(&c, 0); + TEST_ASSERT_EQUAL_UINT64(4, L->state.commit_index); + TEST_ASSERT_EQUAL_INT(2, L->commit_advanced_count); /* fire count == strict advances */ + + /* Phase 5 — NOT LEADER: a follower's raft_advance_commit is -EPERM; even with a + * quorum that would otherwise commit a current-term entry at 5, NO fire occurs + * and commit_index is untouched. */ + sim_set_log_entry(L, 5, 4); + L->state.match_index[1] = 5; + L->state.role = RAFT_FOLLOWER; + raft_sim_advance_commit(&c, 0); + TEST_ASSERT_EQUAL_UINT64(4, L->state.commit_index); /* unchanged off-leader */ + TEST_ASSERT_EQUAL_INT(2, L->commit_advanced_count); /* no fire off-leader */ +} + /* ---------- Group A: Basic Election Correctness (task 5.1) ---------- */ /* @@ -6594,6 +6950,12 @@ void test_raft(void) RUN_TEST(test_sim_9_4_chaotic_churn_g5_regression); RUN_TEST(test_sim_9_5_node_restart_resumes_replication); + /* simulator commit-safety invariant tests (tasks 7.1-7.4) */ + RUN_TEST(test_sim_7_1_figure8_commit_refusal); + RUN_TEST(test_sim_7_2_leader_completeness_across_changes); + RUN_TEST(test_sim_7_3_freshness_gate_lexicographic); + RUN_TEST(test_sim_7_4_on_commit_advanced_trigger_semantics); + /* Group A: basic election correctness (task 5.1) */ RUN_TEST(test_proto_A1_single_node_self_elects); RUN_TEST(test_proto_A2_3node_one_leader);