diff --git a/.kiro/specs/ha-term-safety/tasks.md b/.kiro/specs/ha-term-safety/tasks.md index 9eb5ea5b..d239490b 100644 --- a/.kiro/specs/ha-term-safety/tasks.md +++ b/.kiro/specs/ha-term-safety/tasks.md @@ -106,7 +106,7 @@ ## 5. Verification: clean-failover preservation, tooling, and the live chaotic regression -- [ ] 5.1 Adapter callback unit/integration tests +- [x] 5.1 Adapter callback unit/integration tests - Cover: term stamp + read-back incl. legacy-0 mapping; higher-term heartbeat adoption via the intake route; truncate-then-persist of a divergent suffix with committed prefix untouched; `on_commit_advanced` releasing the barrier only on durable majority; config marker append + committed-config derive + divergent-marker truncation - Observable completion: the adapter test binary exercises every callback and the marker path and passes under ASAN - _Requirements: 1.1, 1.5, 1.8, 2.3, 3.1, 3.2, 4.1, 7.1, 7.6, 7.7_ @@ -196,3 +196,6 @@ - **bug (verify-bypass, caught round 1 Finding A):** handle_repl_config_push originally called load_primary_config (activating pushed bytes) BEFORE the hash gate → mismatched config went live. Fixed: gate is the SOLE activation point. - **PATTERN (Do-Not-Repeat):** for a marker-keyed out-of-band-bytes consumer, (1) verify-before-activate must be the SOLE activation path (no earlier load); (2) the consumer must be wired into ALL production reach paths — byte-arrival AND boot-recovery AND commit-advance — not just one (dead-code-in-production = incomplete task, same trap as 4.2); (3) a fence flag must have a real production READER or it's not a fence; (4) marker (WAL) and bytes (out-of-band) commit on INDEPENDENT transports → always handle push-before-commit with a commit-advance retry. async-I/O: schedule disk work off the iomgr ACK thread via delaymgr. - **CONSOLIDATED 5.5/cleanup list:** (a) `ha_cb_*_stub` rename + stale stub-header comment ha.c ~1326; (b) ha_convergence.c ~292-297 stale cluster_apply_ctrlhello_reply doc-comment; (c) ha.h rejoin_reconciled dead field; (d) committed_config_tag now cache-only (audit remaining readers). All cosmetic. + +- **5.1 done.** Coverage audit found all 5 scenarios + 5 callbacks already covered by per-task tests; the genuine gap was the task's literal observable ("exercises every callback AND the marker path"). Added ONE integration test `test_every_callback_and_marker_path_end_to_end` (test_term_safety_adapter.c, test-only, empty production diff) driving a leader-lifecycle flow through the REAL seams: ha_primary_log_append→raft_log_append→on_persist_entry (job@1, marker@2, job@3 term-stamped); on_get_entry_term read-back; on_send_append_entries captures the marker entry (lsn2/WAL_CONFIG_MARKER/payload40) via repl_send_ae_capture_hook; on_commit_advanced ceiling caps then real ha_raft_on_repl_ack→raft_advance_commit releases; on_truncate_from drops LSN3, committed job+marker prefix + derived config intact. RED proven by neuter-and-revert. 33 Tests 0 Failures under DEFAULT ASAN (leak-detect ON), no new leak. Reviewer APPROVED (independently rebuilt the coverage map; confirmed real seams + load-bearing assertions). + - **NEXT (5.x live env):** 5.2 (clean single-failover integration) _Depends 3.3_ and 5.4 (live chaotic-multi-kill regression) _Depends 5.2,5.3,4.3_ require a real 3-node deploy (/xce-deploy-ha) on a real-fs (bug-924: overlayfs fdatasync degrades meson + would corrupt timing-sensitive live runs). 5.3 (walcompare tool) is build-only (no live env). The live-send retirement (term-0 repl_broadcast → library send loop) deferred from 3.2/3.3 lands/validates in 5.2/5.4. diff --git a/src/controller/testsuite/test_term_safety_adapter.c b/src/controller/testsuite/test_term_safety_adapter.c index a32fae63..fb7f4a26 100644 --- a/src/controller/testsuite/test_term_safety_adapter.c +++ b/src/controller/testsuite/test_term_safety_adapter.c @@ -11,6 +11,7 @@ #include #include "config.h" +#include "config_repl.h" #include "persist.h" #include "wal_types.h" #include "repl.h" @@ -1588,6 +1589,185 @@ static void test_repl_ack_out_of_range_peer_rejected(void) printf("test_repl_ack_out_of_range_peer_rejected: PASS\n"); } +/* ================================================================== */ +/* Task 5.1 — every-callback + marker-path end-to-end integration */ +/* ================================================================== */ + +/* + * The per-task tests above each exercise ONE callback (or one leg of the + * marker path) in isolation. This integration test drives a single realistic + * leader-lifecycle flow in which ALL FIVE adapter callbacks AND the config + * marker path interoperate end-to-end, so a regression that breaks the + * COMPOSITION of the callbacks (not just one in isolation) is caught here: + * + * on_persist_entry — primary job append + config-marker append both + * land in the WAL with the live term stamped + * on_get_entry_term — the leader's last-log term is read back + * on_send_append_entries— the leader emits the marker entry to a peer with the + * correct (prev tuple, leader_commit) and the entry's + * recorded term/type/payload + * on_commit_advanced — a current-term quorum ACK advances commit; the + * marker enters the committed prefix and the DERIVED + * committed config (config_committed_marker) equals it + * on_truncate_from — a divergent same-LSN tail is truncated then the + * leader's entry re-persisted, the committed prefix + * (including the committed marker) left intact + * + * Against any single broken callback the corresponding assertion below fails + * (e.g. neutering on_commit_advanced leaves committed_lsn behind the durable + * majority so config_committed_marker returns -ENOENT; neutering on_truncate_from + * leaves the stale divergent term at the conflicting LSN). + */ +static void test_every_callback_and_marker_path_end_to_end(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_ts_e2e_all_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t T = 9; + + cluster_t *c = make_cluster(tmp); + TEST_ASSERT_NOT_NULL(c); + + /* Bring the node up as the raft LEADER at term T (init-restore floor path, + * task 2.5). A 3-peer cluster so a majority is self + 1 standby. */ + make_leader(c, T, /*n_peers*/3); + TEST_ASSERT_EQUAL_INT(0, c->ha.my_priority); /* self_idx 0 */ + TEST_ASSERT_EQUAL_UINT64(0, c->ha.raft.log_len); + + /* The cluster's own commit engine over the same persist (3 peers). */ + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&c->commit, &c->persist, /*n_peers*/3, /*n_vcpus*/8)); + + /* ── on_persist_entry (job append) ────────────────────────────────────── + * A primary job-state append routes through the library → on_persist_entry + * stamps the live term at LSN 1. */ + wal_jobstate_t js = { .jobid = 77, .state = 2 }; + uint64_t lsn_job = 0; + TEST_ASSERT_EQUAL_INT(0, + ha_primary_log_append(c, WAL_JOB_UPDATE, &js, sizeof(js), &lsn_job)); + TEST_ASSERT_EQUAL_UINT64(1, lsn_job); + TEST_ASSERT_EQUAL_UINT64(1, c->ha.raft.log_len); + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 1)); + + /* ── on_persist_entry (config marker — the marker path) ───────────────── + * A config marker rides the SAME unified append path (R7.1/R7.3): it lands + * at LSN 2 with the live term stamped, exactly like a job entry. */ + uint8_t sha[32]; + for (int i = 0; i < 32; i++) sha[i] = (uint8_t)(0xA0 + i); + uint64_t tag = 0xA0A1A2A3A4A5A6A7ULL; /* be64 of the first 8 sha bytes */ + uint64_t lsn_marker = 0; + TEST_ASSERT_EQUAL_INT(0, config_append_marker(c, tag, sha, &lsn_marker)); + TEST_ASSERT_EQUAL_UINT64(2, lsn_marker); + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.log_len); + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 2)); + + /* A third, NON-committed job entry at LSN 3 (current term). It is appended + * BEFORE the standby is registered so the primary's repl_broadcast is a + * no-op (registering the dummy int ep first would fan the append out to a + * non-endpoint → ASAN SEGV — the same ordering constraint the send-AE tests + * above document). LSN 3 is the divergent suffix the on_truncate_from leg + * removes once the committed prefix (LSN 1,2) is established below. */ + wal_jobstate_t js2 = { .jobid = 78, .state = 1 }; + uint64_t lsn3 = 0; + TEST_ASSERT_EQUAL_INT(0, + ha_primary_log_append(c, WAL_JOB_UPDATE, &js2, sizeof(js2), &lsn3)); + TEST_ASSERT_EQUAL_UINT64(3, lsn3); + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 3)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(&c->persist)); + + /* ── on_get_entry_term ────────────────────────────────────────────────── + * The leader's last-log term (LSN 3) reads back as T via the wired callback. */ + TEST_ASSERT_EQUAL_UINT64(T, + ha_cb_get_entry_term_for_test(&c->ha, c->ha.raft.log_len)); + + /* ── on_send_append_entries ───────────────────────────────────────────── + * The leader emits the marker entry (LSN 2) to peer 0, carrying the prev + * tuple (LSN 1, term T), leader_commit, and the marker's own recorded term + + * WAL_CONFIG_MARKER type + 40-byte reference payload (read from its own WAL). */ + register_one_standby(c); + repl_ae_capture_t cap; + memset(&cap, 0, sizeof(cap)); + repl_send_ae_capture_hook = ∩ + int rc = ha_cb_send_append_entries_for_test(&c->ha, /*peer*/0, + /*prev_idx*/1, /*prev_term*/T, + /*leader_commit*/0, /*n*/1); + repl_send_ae_capture_hook = NULL; + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(1, cap.called); + TEST_ASSERT_EQUAL_UINT64(1, cap.prev_log_index); + TEST_ASSERT_EQUAL_UINT64(T, cap.prev_log_term); + TEST_ASSERT_EQUAL_UINT64(2, cap.entry_lsn); + TEST_ASSERT_EQUAL_UINT64(T, cap.entry_term); + TEST_ASSERT_EQUAL_UINT8(WAL_CONFIG_MARKER, cap.entry_type); + TEST_ASSERT_EQUAL_UINT32(WAL_CONFIG_MARKER_PAYLOAD_LEN, cap.payload_len); + + /* ── on_commit_advanced (+ committed marker derive) ───────────────────── + * Before commit advances, the marker is appended but NOT in the committed + * prefix, so the derived committed config is absent. */ + uint64_t got_tag = 0; + uint8_t got_sha[32]; + TEST_ASSERT_EQUAL_INT(-ENOENT, + config_committed_marker(c, &got_tag, got_sha)); + + /* on_commit_advanced is LOAD-BEARING (production order: the ACK path fires + * on_commit_advanced, which records the library ceiling, BEFORE the barrier + * re-evaluates the durable majority). Fire the WIRED callback at + * commit_index=1 FIRST to set the ceiling at 1, THEN supply a durable + * majority covering LSN 2. committed_lsn caps at the library ceiling (1), + * BELOW the durable majority (2). This can only hold if on_commit_advanced + * fed the library current-term commit point as the barrier ceiling (the + * term-blind safe-commit is no longer the source); a no-op callback would + * let the barrier release committed_lsn to the durable majority (2). */ + ha_cb_commit_advanced_for_test(&c->ha, 1); + atomic_store(&c->persist.repl.standbys[0].acked_lsn, 2); + commit_on_ack(&c->commit, /*peer_idx*/0, /*acked_lsn*/2); + TEST_ASSERT_EQUAL_UINT64(1, atomic_load(&c->persist.committed_lsn)); + + /* Now route the real current-term quorum ACK through the library + * (raft_on_append_entries_reply → raft_advance_commit → on_commit_advanced). + * The agreed entries are current-term (T) so the library commit advances to + * 2 and the callback raises the ceiling, releasing committed_lsn to 2. */ + TEST_ASSERT_EQUAL_INT(0, + ha_raft_on_repl_ack(c, /*abs_peer_idx*/1, /*acked_lsn*/2, /*reply_term*/T)); + TEST_ASSERT_EQUAL_UINT64(2, c->ha.raft.commit_index); + TEST_ASSERT_EQUAL_UINT64(2, atomic_load(&c->persist.committed_lsn)); + + /* The marker (LSN 2) is now in the committed prefix → the DERIVED committed + * config equals the appended marker's {tag, sha} (R7.7). */ + TEST_ASSERT_EQUAL_INT(0, config_committed_marker(c, &got_tag, got_sha)); + TEST_ASSERT_EQUAL_UINT64(tag, got_tag); + TEST_ASSERT_EQUAL_MEMORY(sha, got_sha, 32); + + /* ── on_truncate_from ─────────────────────────────────────────────────── + * The uncommitted suffix (the LSN 3 job entry appended above) is removed by + * truncate-from LSN 3, while the committed prefix (job LSN 1 + committed + * marker LSN 2) stays byte-stable (R3.2/R3.3). on_truncate_from is the + * callback under test; the committed marker derive must still hold after. */ + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 3)); /* present */ + TEST_ASSERT_EQUAL_INT(0, ha_cb_truncate_from_for_test(&c->ha, 3)); + TEST_ASSERT_EQUAL_UINT64(0, persist_entry_term(&c->persist, 3)); /* gone */ + + /* The committed prefix is intact: LSN 1 (job) + LSN 2 (marker) keep their + * terms, and the committed marker still derives to the same {tag, sha}. */ + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 1)); + TEST_ASSERT_EQUAL_UINT64(T, persist_entry_term(&c->persist, 2)); + TEST_ASSERT_EQUAL_INT(0, config_committed_marker(c, &got_tag, got_sha)); + TEST_ASSERT_EQUAL_UINT64(tag, got_tag); + TEST_ASSERT_EQUAL_MEMORY(sha, got_sha, 32); + + /* All five term-safety callbacks remain wired (non-NULL) through the flow. */ + TEST_ASSERT_NOT_NULL(c->ha.raft.cbs->on_persist_entry); + TEST_ASSERT_NOT_NULL(c->ha.raft.cbs->on_get_entry_term); + TEST_ASSERT_NOT_NULL(c->ha.raft.cbs->on_send_append_entries); + TEST_ASSERT_NOT_NULL(c->ha.raft.cbs->on_truncate_from); + TEST_ASSERT_NOT_NULL(c->ha.raft.cbs->on_commit_advanced); + + commit_engine_fini(&c->commit); + teardown_cluster(c); + printf("test_every_callback_and_marker_path_end_to_end: PASS\n"); +} + /* ------------------------------------------------------------------ */ /* Suite entry point */ /* ------------------------------------------------------------------ */ @@ -1626,4 +1806,6 @@ void test_term_safety_adapter(void) RUN_TEST(test_repl_ack_abs_peer_idx_endpoint_type_roundtrip); RUN_TEST(test_repl_ack_heartbeat_standby_absolute_slot); RUN_TEST(test_repl_ack_out_of_range_peer_rejected); + /* Task 5.1: every-callback + marker-path end-to-end integration. */ + RUN_TEST(test_every_callback_and_marker_path_end_to_end); }