From 21858faba4576eb27c38ab0a021d3ecfc825f8ee Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 1 Jul 2026 09:46:39 +0000 Subject: [PATCH] =?UTF-8?q?test(raft-ha-core-safety):=20RED=20=E2=80=94=20?= =?UTF-8?q?snapshot=20install=20does=20not=20reconcile=20(term,lsn)=20boun?= =?UTF-8?q?dary=20(task=203.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ha_handle_snapshot_chunk/persist_load_snapshot advance checkpoint_lsn, current_lsn, and applied_lsn on snapshot install but never touch the per-LSN term index, so a divergent pre-snapshot entry's stale local term survives the install unreconciled. New test proves the gap (Expected 7 Was 3); fix lands in task 3.2. --- .kiro/specs/raft-ha-core-safety/tasks.md | 2 +- .../testsuite/test_ha_core_safety.c | 119 ++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/raft-ha-core-safety/tasks.md b/.kiro/specs/raft-ha-core-safety/tasks.md index eef8fc28..99592f35 100644 --- a/.kiro/specs/raft-ha-core-safety/tasks.md +++ b/.kiro/specs/raft-ha-core-safety/tasks.md @@ -67,7 +67,7 @@ - _Depends: 2.2, 2.3_ - [ ] 3. Snapshot-boundary reconciliation -- [ ] 3.1 RED: snapshot post-install reconcile test +- [x] 3.1 RED: snapshot post-install reconcile test - Add a case where a divergent pre-snapshot entry must be reconciled against the installed checkpoint's `(term, lsn)` boundary (today snapshot install sets applied-lsn without a `(term, lsn)` reconcile) - Observable completion: the case FAILS on the baseline because no `(term, lsn)` reconcile occurs at the snapshot boundary - _Requirements: 3.1, 5.1_ diff --git a/src/controller/testsuite/test_ha_core_safety.c b/src/controller/testsuite/test_ha_core_safety.c index 2a09b575..220e7a53 100644 --- a/src/controller/testsuite/test_ha_core_safety.c +++ b/src/controller/testsuite/test_ha_core_safety.c @@ -914,6 +914,119 @@ static void test_2_3_unrepairable_catchup_raises_nonpromotable_fence(void) printf("test_2_3_unrepairable_catchup_raises_nonpromotable_fence: PASS (pin)\n"); } +/* ═══════════════════════════════════════════════════════════════════════════ + * TASK 3.1 — RED: snapshot post-install (term, lsn) boundary reconcile + * + * design.md "Snapshot Boundary Reconcile" (Adapter component, Req 3, 5): + * snapshot post-install (ha_handle_snapshot_chunk, ha.c:2492) must establish + * a verifiable (term, lsn) at the checkpoint boundary so post-install + * AppendEntries intake reconciles correctly and no committed entry below the + * checkpoint can diverge (design.md line 373: "a divergent pre-snapshot + * entry is reconciled against the checkpoint (term,lsn)"). + * + * Confirmed gap (direct code read): ha_handle_snapshot_chunk calls + * persist_load_snapshot -> load_snapshot (persist.c), which reads a header of + * magic(8)+checkpoint_lsn(8)[+optional cfgref tail] and sets ONLY + * p->checkpoint_lsn/p->current_lsn (plus ha->applied_lsn, in-memory) -- it + * never calls persist_set_term nor touches the per-LSN term index + * (persist_set_term/record_entry_term, persist.c ~419-487) for the checkpoint + * boundary. The wire message (snapshot_transfer_t / capnp SnapshotTransfer, + * src/schema/controller.capnp ~874-881) carries no term field at all. So a + * standby holding a DIVERGENT pre-snapshot entry AT the checkpoint's own + * boundary LSN -- stored under its own stale local term -- keeps that stale + * term in the term index forever after the snapshot installs: the boundary + * is LSN-only, never (term, lsn). + * + * This case drives the REAL ha_handle_snapshot_chunk -> persist_load_snapshot + * path (not a stub) over a real persist_t WAL: a divergent pre-snapshot + * entry at the exact boundary LSN stored at a STALE local term, then a + * legacy (position-only) checkpoint chunk installed at that same LSN. + * RED: persist_entry_term at the boundary must reconcile to the checkpoint's + * true term -- it does not; the stale local term survives the install + * completely untouched. + * + * Requirements: 3.1, 5.1. + * ═══════════════════════════════════════════════════════════════════════════ */ +static void test_3_1_snapshot_install_does_not_reconcile_boundary_term(void) +{ + hcs_member_t m; + TEST_ASSERT_EQUAL_INT(0, hcs_member_up(&m, "snap31")); + ha_state_t ha; + ha_init(&ha, &g_hcs_cfg, 0); + + enum { BOUNDARY_LSN = 3, STALE_TERM = 3, CHECKPOINT_TERM = 7 }; + + uint8_t full[HCS_FULL_LEN]; + hcs_fill_body(full); + + /* The standby holds a contiguous pre-snapshot WAL prefix [1..BOUNDARY_LSN] + * at its own STALE local term -- the divergent-pre-snapshot-entry shape + * task 3.1 targets, with the divergence sitting AT the exact LSN the + * incoming checkpoint will cover. */ + for (uint64_t lsn = 1; lsn <= (uint64_t)BOUNDARY_LSN; lsn++) { + TEST_ASSERT_EQUAL_INT_MESSAGE(0, + hcs_wal_put(&m, lsn, (uint64_t)STALE_TERM, WAL_JOB_SUBMIT, full, + HCS_FULL_LEN), + "the pre-snapshot prefix must persist (real fs required — bug-924)"); + } + atomic_store_explicit(&m.p->committed_lsn, (uint64_t)BOUNDARY_LSN, + memory_order_release); + TEST_ASSERT_EQUAL_UINT64_MESSAGE((uint64_t)STALE_TERM, + persist_entry_term(m.p, (uint64_t)BOUNDARY_LSN), + "precondition: the pre-snapshot entry at the boundary LSN is stored " + "at the STALE local term"); + + /* A leader at a DIFFERENT (higher) term ships a checkpoint whose position + * IS this exact boundary LSN. Today's on-disk/wire format carries no + * term field at all, so the legacy (position-only) shape is exactly what + * a real checkpoint transfer installs via ha_handle_snapshot_chunk's real + * chunk-assembly + rename + load path (not a stub). */ + char snap_body[16]; + memcpy(snap_body, "XCESNAP\0", 8); + uint64_t boundary_lsn_u64 = (uint64_t)BOUNDARY_LSN; + memcpy(snap_body + 8, &boundary_lsn_u64, 8); + + snapshot_transfer_t chunk; + memset(&chunk, 0, sizeof(chunk)); + chunk.snapshot_lsn = boundary_lsn_u64; + chunk.chunk_index = 0; + chunk.chunk_offset = 0; + chunk.total_size = sizeof(snap_body); + chunk.is_last = 1; + + int rc = ha_handle_snapshot_chunk(&ha, m.p, m.p->snap_path, &chunk, + (const uint8_t*)snap_body, + sizeof(snap_body)); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, + "installing the checkpoint at the boundary LSN must succeed"); + + /* Sanity (already true on the baseline, NOT the RED point): the position + * boundary advances. */ + TEST_ASSERT_EQUAL_UINT64(boundary_lsn_u64, m.p->checkpoint_lsn); + TEST_ASSERT_EQUAL_UINT64_MESSAGE(boundary_lsn_u64, + atomic_load_explicit(&ha.applied_lsn, memory_order_acquire), + "applied_lsn advances to the checkpoint boundary (existing behavior)"); + + /* RED: the checkpoint boundary's (term, lsn) must be reconciled to the + * checkpoint's own (true) term -- but ha_handle_snapshot_chunk / + * persist_load_snapshot never call persist_set_term nor touch the term + * index for the boundary LSN, so the STALE local term from the divergent + * pre-snapshot entry survives the snapshot install completely + * unreconciled. */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE((uint64_t)CHECKPOINT_TERM, + persist_entry_term(m.p, boundary_lsn_u64), + "RED: snapshot post-install must reconcile the (term, lsn) boundary " + "to the checkpoint's own term -- today ha_handle_snapshot_chunk / " + "persist_load_snapshot advance checkpoint_lsn/current_lsn/applied_lsn " + "only and never touch persist's term index, so a divergent " + "pre-snapshot entry's STALE local term survives the snapshot install " + "unreconciled (design.md 'Snapshot Boundary Reconcile', task 3.1)"); + + ha_fini(&ha); + hcs_member_down(&m); + printf("test_3_1_snapshot_install_does_not_reconcile_boundary_term: done\n"); +} + /* ═══════════════════════════════════════════════════════════════════════════ * TASK 4.1 — DETERMINISTIC follower-side reconvergence (the DISCRIMINATOR) * @@ -2031,6 +2144,12 @@ void test_ha_core_safety(void) RUN_TEST(test_2_4_streamer_short_read_retry_recovers); RUN_TEST(test_2_3_termblind_route_payload_guarded); RUN_TEST(test_2_3_unrepairable_catchup_raises_nonpromotable_fence); + /* Task 3.1 snapshot post-install (term, lsn) boundary reconcile: + * CORE RED — a divergent pre-snapshot entry at the checkpoint boundary + * LSN must have its term reconciled to the checkpoint's own + * term after install (FAILs on the baseline: snapshot + * install never touches the term index at all). */ + RUN_TEST(test_3_1_snapshot_install_does_not_reconcile_boundary_term); /* Task 4.1 follower-side reconvergence (the DISCRIMINATOR): * DISCRIMINATOR — a MULTI-ENTRY former-primary divergent tail is * reconverged by the follower AE backtrack + conflict-