From 47532f0a76f8886bb96b43e6b22b85088d9824e7 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 27 Aug 2026 08:26:09 +0000 Subject: [PATCH] fix(ha): first-boot promotion evaluated eligibility before its own WAL recovery ran (bug-1696) Chaotic HA soak (post-bug-1695) reported committed_job_loss every round. raft_cb_become_leader() computed the promotion WAL-truncation floor and evaluated the full promotion-eligibility gate (durable_floor/ recovery_truncated/apply_verdict) BEFORE cluster_init_phase2() -- the only call that runs persist_recover() on this process's first-ever boot. Those fields all come from persist_recover(); read before it runs, a freshly persist_init()'d struct reports durable_lsn=0, making the eligibility gate wrongly judge a genuinely corrupt/divergent node ELIGIBLE. Unreachable before bug-1695's fix (a restarted node's stale term previously always made it defer to a peer instead of re-electing on the same boot with real data at stake) -- bug-1695 correctly restored real re-election on restart and newly exposed this pre-existing boot-ordering flaw. Reordered raft_cb_become_leader() so cluster_init_phase2()/ persist_recover() runs before the eligibility gate and truncation-floor computation. Split the pure vote-derived safe_committed_lsn computation (reads only ha->elect_ctx, safe pre-recovery) from the truncation-floor computation (needs post-recovery persist state) -- the former stays early, the latter moves to run after phase2. New regression test CRC-corrupts a real on-disk WAL and confirms first-boot promotion now correctly refuses instead of destroying committed data. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg --- src/controller/ha.c | 268 ++++++----- .../testsuite/test_job_array_replay_fence.c | 433 ++++++++++++++++++ 2 files changed, 575 insertions(+), 126 deletions(-) diff --git a/src/controller/ha.c b/src/controller/ha.c index 32790b8c..ee470163 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -1633,67 +1633,149 @@ static void raft_cb_become_leader(void *ctx, uint64_t term) /* Task 4.1 (ha-repl-hardening): compute safe_committed_lsn from vote replies. * Uses voter LSNs collected during election (ha->elect_ctx->votes[]) to find * the highest LSN held by a majority without an extra round-trip. - * Design §Flow 0: Post-Election — Determine Safe Commit Point. */ - { - ha_elect_ctx_t *ectx = (ha_elect_ctx_t *)atomic_load_explicit( - &ha->elect_ctx, memory_order_acquire); - if (ectx) { - uint64_t safe_lsn = 0; - uint64_t safe_tag = 0; - ha_compute_safe_committed_lsn(ectx, &safe_lsn, &safe_tag); - promotion_safe_lsn = safe_lsn; - /* Store results for use in WAL truncation and post-election commit. - * safe_lsn is the highest LSN on a majority; entries above it - * may not have been committed on all nodes and must be truncated. - * - * bug-1060 (Leader Completeness, Raft §5.4): the truncation cutoff is - * clamped to the leader's OWN durable tip so it never destroys entries - * this node holds durably — those were acked by this node as a standby - * and so were committed by the prior leader. Truncating to the bare - * majority element (which can sit BELOW the leader's durable tip when - * a slower peer is in the granting majority) diverged WALs and dead- - * locked the next election. With the clamp, persist_truncate_wal_above - * is a no-op on the leader's durable log; safe_lsn only governs removal - * of a non-durable divergent tail strictly above the durable tip. */ - if (c) { - uint64_t durable_tip = persist_durable_floor(&c->persist); - uint64_t trunc_floor = - ha_promotion_truncate_floor(safe_lsn, durable_tip); - persist_truncate_wal_above(&c->persist, trunc_floor); - /* 2026-07-10 (chaotic-soak finding): persist_truncate_wal_above - * only advances persist->committed_lsn on the branches that - * perform a REAL truncation; its no-op path (this leader's own - * log already covers safe_lsn -- the common case) leaves - * committed_lsn untouched at its pre-election value, stalling - * commit-gated apply until the normal ack-driven commit path - * organically catches up seconds later. See - * persist_promotion_advance_committed's doc comment. */ - persist_promotion_advance_committed(&c->persist, safe_lsn); - } - /* Task 4.2 (ha-repl-hardening): set committed_lsn = safe_lsn. - * persist_promotion_advance_committed (above) advances - * c->persist.committed_lsn in the production path (c != NULL), - * durable-capped and monotonic. Also update - * ha->last_known_committed_lsn so the test-mode decision loop - * (XCE_TESTING path in standby heartbeat reply handler) and any - * code reading ha->last_known_committed_lsn see the safe commit - * point immediately after promotion. - * Design §Flow 0 step 5: committed_lsn = safe_committed_lsn. - * Req 2.3 (ha-repl-hardening). */ - atomic_store_explicit(&ha->last_known_committed_lsn, safe_lsn, - memory_order_release); - /* Update committed config tag baseline for the post-election decision - * loop. bug-1462: clamp to the leader's OWN loaded config tag — - * unlike WAL LSN, a node's actively-loaded config is never - * speculative, so the bare majority floor (correct for LSN) must - * not be allowed to strand a real config behind an all-zero floor - * reported by voters that never received a live config push. - * See ha_promotion_config_tag_floor's doc comment (ha.h). */ - uint64_t committed_tag_floor = - ha_promotion_config_tag_floor(safe_tag, ectx->self_tag); - atomic_store_explicit(&ha->committed_config_tag, committed_tag_floor, - memory_order_release); + * Design §Flow 0: Post-Election — Determine Safe Commit Point. + * + * bug-1696: this computation is PURE — it reads only ha->elect_ctx (votes + * already collected during the election), never c->persist — so it is safe + * and deliberately kept BEFORE cluster_init_phase2()/persist_recover() + * below. The truncation step that DOES need post-recovery persist state + * (persist_durable_floor()/persist_truncate_wal_above()) is split out + * further down, AFTER phase2 has run — see the bug-1696 comment there for + * why: reading persist_durable_floor() before this process's own + * persist_recover() has ever run for this boot returns a bogus 0, and + * truncating the WAL to that bogus floor destroys real durably-committed + * data (bug-1696 root cause). */ + ha_elect_ctx_t *promo_ectx = (ha_elect_ctx_t *)atomic_load_explicit( + &ha->elect_ctx, memory_order_acquire); + if (promo_ectx) { + uint64_t safe_lsn = 0; + uint64_t safe_tag = 0; + ha_compute_safe_committed_lsn(promo_ectx, &safe_lsn, &safe_tag); + promotion_safe_lsn = safe_lsn; + /* Task 4.2 (ha-repl-hardening): set committed_lsn = safe_lsn. + * persist_promotion_advance_committed (called below, after phase2) + * advances c->persist.committed_lsn in the production path (c != + * NULL), durable-capped and monotonic. Also update + * ha->last_known_committed_lsn so the test-mode decision loop + * (XCE_TESTING path in standby heartbeat reply handler) and any + * code reading ha->last_known_committed_lsn see the safe commit + * point immediately after promotion. + * Design §Flow 0 step 5: committed_lsn = safe_committed_lsn. + * Req 2.3 (ha-repl-hardening). */ + atomic_store_explicit(&ha->last_known_committed_lsn, safe_lsn, + memory_order_release); + /* Update committed config tag baseline for the post-election decision + * loop. bug-1462: clamp to the leader's OWN loaded config tag — + * unlike WAL LSN, a node's actively-loaded config is never + * speculative, so the bare majority floor (correct for LSN) must + * not be allowed to strand a real config behind an all-zero floor + * reported by voters that never received a live config push. + * See ha_promotion_config_tag_floor's doc comment (ha.h). */ + uint64_t committed_tag_floor = + ha_promotion_config_tag_floor(safe_tag, promo_ectx->self_tag); + atomic_store_explicit(&ha->committed_config_tag, committed_tag_floor, + memory_order_release); + } + + /* ── bug-1696 fix: cluster_init_phase2()/persist_recover() MUST run before + * anything below reads persist_durable_floor()/persist_recovery_truncated()/ + * persist_apply_verdict() or calls persist_truncate_wal_above() ────────── + * Moved up from its original position (after the truncation-floor + * computation and the full promotion-eligibility gate) so that this + * process's first-ever persist_recover() for THIS boot always runs BEFORE + * any of that code reads recovery-derived persist state. Before this + * reorder, a freshly-persist_init()'d persist_t (durable_lsn=0) could be + * read as the "durable floor" and truncated to on a restart that won its + * own promotion on the very same boot it recovered on — physically + * destroying real committed WAL data before it was ever read + * (Leader Completeness violation, bug-1696). is_first_boot/phase2_done + * semantics are unchanged: still computed here, still latched only on a + * successful phase2 call, still a once-per-process-lifetime call. */ + int is_first_boot = !ha->phase2_done; + + /* First boot: call cluster_init_phase2 */ + if (is_first_boot) { + int p2rc = 0; +#ifdef XCE_TESTING + if (ha_phase2_hook != NULL) + p2rc = ha_phase2_hook(c); +#else + if (c != NULL) + p2rc = cluster_init_phase2(c); +#endif + if (p2rc == 0) { + ha->phase2_done = 1; + } else { + /* bug-1477: do NOT latch phase2_done on failure, and do NOT + * proceed to CTRL_PRIMARY with an unwired commit engine — the + * member stays ALIVE, non-promotable, and retries (a later + * promotion attempt succeeds once ha_schedule_phase2_retry + * completes). Preserved verbatim from the pre-bug-1696 refusal + * path: no truncation/eligibility-gate code has run yet at this + * point (it has all been moved below this block), so there is + * nothing to undo on this refusal. */ + WARN("HA: promotion REFUSED — cluster_init_phase2 failed " + "(rc=%d); member stays ALIVE, non-promotable, retrying " + "(bug-1477, never self-terminate)", p2rc); + if (c != NULL) + ha_schedule_phase2_retry(c); + return; /* do NOT become CTRL_PRIMARY with an unwired commit engine */ } + + /* bug-1445 fix (kept post-bug-1696-reorder as an inert defense-in- + * depth double-check — see below): cluster_init_phase2() above just + * ran this process's very FIRST persist_recover()/replay_wal() -- the + * ONLY thing that can engage the Live Apply Resource Fence (job- + * array-single-wal-record tasks 4.1/4.2, extended to boot replay by + * task 7.2) for THIS node. Originally (pre-bug-1696) the main + * eligibility gate ran BEFORE this phase2 call and so necessarily + * read persist_apply_resource_fenced() as false (a fresh persist_t + * defaults to unfenced); this re-check compensated for that + * blindness immediately after the only call that could have changed + * the answer. Post-bug-1696-reorder, the main eligibility gate below + * now runs AFTER phase2 and already reads the correct, current + * apply_resource_fenced value directly -- so this block can no + * longer diverge from that gate's own answer (nothing async runs in + * between) and is strictly redundant. Left in place rather than + * deleted: zero behavioral risk (same flag, same logic, same refusal + * shape as the gate below) and removing it is not needed to fix + * bug-1696. */ + if (c != NULL && persist_apply_resource_fenced(&c->persist)) { + WARN("HA: promotion REFUSED — this node's own first-ever boot " + "replay (cluster_init_phase2, just completed) engaged the " + "Live Apply Resource Fence; the earlier eligibility check " + "in this same election evaluation necessarily missed it " + "(bug-1445) — member stays ALIVE, non-promotable, " + "retrying apply (job-array-single-wal-record Req 9.4/9.5)"); + ha_arm_promotion_fence_timer(ha, c, + persist_recovery_truncated(&c->persist), + persist_durable_floor(&c->persist), + promotion_safe_lsn); + return; + } + } + + /* bug-1696 fix: the truncation-floor computation split out from the pure + * safe_lsn computation above -- see that comment for why. Reads + * persist_durable_floor(&c->persist) only now, AFTER cluster_init_phase2()/ + * persist_recover() has run above, so it reflects the REAL on-disk WAL + * tip for this boot (never a bogus pre-recovery 0). Gated on promo_ectx + * (matches the original code: no ectx means no vote-derived safe_lsn, so + * no truncation was ever computed) and c (production-only). */ + if (promo_ectx && c) { + uint64_t durable_tip = persist_durable_floor(&c->persist); + uint64_t trunc_floor = + ha_promotion_truncate_floor(promotion_safe_lsn, durable_tip); + persist_truncate_wal_above(&c->persist, trunc_floor); + /* 2026-07-10 (chaotic-soak finding): persist_truncate_wal_above + * only advances persist->committed_lsn on the branches that + * perform a REAL truncation; its no-op path (this leader's own + * log already covers safe_lsn -- the common case) leaves + * committed_lsn untouched at its pre-election value, stalling + * commit-gated apply until the normal ack-driven commit path + * organically catches up seconds later. See + * persist_promotion_advance_committed's doc comment. */ + persist_promotion_advance_committed(&c->persist, promotion_safe_lsn); } /* ── ha-commit-durability task 4.3: pre-promotion eligibility gate ─────── @@ -1868,72 +1950,6 @@ static void raft_cb_become_leader(void *ctx, uint64_t term) return; /* do NOT become CTRL_PRIMARY; never self-exit */ } - int is_first_boot = !ha->phase2_done; - - /* First boot: call cluster_init_phase2 */ - if (is_first_boot) { - int p2rc = 0; -#ifdef XCE_TESTING - if (ha_phase2_hook != NULL) - p2rc = ha_phase2_hook(c); -#else - if (c != NULL) - p2rc = cluster_init_phase2(c); -#endif - if (p2rc == 0) { - ha->phase2_done = 1; - } else { - /* bug-1477: do NOT latch phase2_done on failure, and do NOT - * proceed to CTRL_PRIMARY with an unwired commit engine — mirror - * the upgrade_materialization_fenced refusal just above (stay - * ALIVE, non-promotable, retry; a later promotion attempt - * succeeds once ha_schedule_phase2_retry completes). Before this - * fix, phase2_done was latched unconditionally, so this failure - * path never existed for the code below to worry about. */ - WARN("HA: promotion REFUSED — cluster_init_phase2 failed " - "(rc=%d); member stays ALIVE, non-promotable, retrying " - "(bug-1477, never self-terminate)", p2rc); - if (c != NULL) - ha_schedule_phase2_retry(c); - return; /* do NOT become CTRL_PRIMARY with an unwired commit engine */ - } - - /* bug-1445 fix: cluster_init_phase2() above just ran this process's - * very FIRST persist_recover()/replay_wal() -- the ONLY thing that - * can engage the Live Apply Resource Fence (job-array-single-wal- - * record tasks 4.1/4.2, extended to boot replay by task 7.2) for - * THIS node. The eligibility gate above (~line 1337) necessarily - * read persist_apply_resource_fenced() as false: a fresh persist_t - * defaults to unfenced (persist_init) and this is the first replay - * that could ever have set it -- so that gate's ELIGIBLE decision - * was made blind to whatever this first replay just did. Re-check - * here, once, right after the only call that could have changed the - * answer: if this node's own first-ever replay just fenced it, - * refuse promotion now instead of falling through to CTRL_PRIMARY - * below on a decision that is now stale. Every later election in - * this process's lifetime is unaffected -- is_first_boot is false - * by then, so the gate above already covers it correctly (this is - * exactly what the sibling test, - * test_replay_wal_engages_fence_on_resource_exhaustion_failure, - * proves). Mirrors the existing refusal branch above verbatim (same - * WARN shape, same ha_arm_promotion_fence_timer() call, same - * unconditional-refuse treatment as apply_divergent/ - * self_restart_pending) rather than inventing a second mechanism. */ - if (c != NULL && persist_apply_resource_fenced(&c->persist)) { - WARN("HA: promotion REFUSED — this node's own first-ever boot " - "replay (cluster_init_phase2, just completed) engaged the " - "Live Apply Resource Fence; the earlier eligibility check " - "in this same election evaluation necessarily missed it " - "(bug-1445) — member stays ALIVE, non-promotable, " - "retrying apply (job-array-single-wal-record Req 9.4/9.5)"); - ha_arm_promotion_fence_timer(ha, c, - persist_recovery_truncated(&c->persist), - persist_durable_floor(&c->persist), - promotion_safe_lsn); - return; - } - } - /* ha-single-leader-role (Req 1.1): enter CTRL_PRIMARY directly — there is * no transitional COMMITTING state. New-leader safety comes from the * own-term commit rule (the WAL_NOOP + writable_lsn gate armed below), diff --git a/src/controller/testsuite/test_job_array_replay_fence.c b/src/controller/testsuite/test_job_array_replay_fence.c index b4ca11d6..0b5ee472 100644 --- a/src/controller/testsuite/test_job_array_replay_fence.c +++ b/src/controller/testsuite/test_job_array_replay_fence.c @@ -66,6 +66,7 @@ #include #include +#include #include #include #include @@ -657,6 +658,436 @@ test_first_boot_promotion_evaluates_own_first_replay_fence(void) "PASS\n"); } +/* ── bug-1696 (positive-path sanity check, NOT the RED/GREEN discriminator + * -- see test_first_boot_promotion_refuses_when_own_first_recovery_finds_ + * corruption below for that): a node's very FIRST-EVER promotion evaluation + * must still succeed and preserve its own genuinely durable WAL when that + * WAL is intact ───────────────────────────────────────────────────────── + * + * IMPORTANT CORRECTION (found while proving this test's RED phase against + * the unfixed ha.c -- do not re-introduce the original hypothesis this + * comment used to state): the original bug-1696 write-up hypothesized that + * the pre-recovery promotion-truncation-floor computation + * (persist_durable_floor()==0 before recovery) would make + * persist_truncate_wal_above() PHYSICALLY DESTROY real durable WAL data left + * over from a prior life of this process. Source-tracing + * truncate_wal_above_locked() (persist.c) disproves that specific mechanism: + * its own no-op guard is `if (p->current_lsn == 0 || cutoff_lsn + 1 >= + * p->current_lsn) return 0;`, and persist_init() unconditionally resets + * current_lsn=1 on every fresh boot regardless of what is physically on + * disk -- so a truncate call issued BEFORE persist_recover() has ever + * re-derived current_lsn from the real WAL scan is ALWAYS a no-op (nothing + * looks "written this boot" yet to truncate). This test proves that + * empirically: it reproduces the exact fixture the original hypothesis + * described (N real, fsynced entries from a prior life, then a truly fresh + * persist_t on the same workdir, is_first_boot=true) and confirms the data + * survives with or WITHOUT the bug-1696 reorder fix applied -- i.e. this + * specific scenario was never actually destructive. It is kept as a + * regression/sanity guard (the reorder must not newly break the healthy + * first-boot-promotion path) but does NOT discriminate the bug. + * + * The REAL, mechanically-provable bug-1696 hazard is different: the + * promotion-ELIGIBILITY GATE (persist_recovery_truncated()/persist_ + * durable_floor()/persist_apply_verdict()) is ALSO evaluated before + * cluster_init_phase2()/persist_recover() has run, so it can wrongly judge + * a genuinely CORRUPT or DIVERGENT node ELIGIBLE (reading the fresh- + * persist_init "clean" defaults) and let it become CTRL_PRIMARY -- only + * discovering the real corruption a few lines later, too late to matter. + * See test_first_boot_promotion_refuses_when_own_first_recovery_finds_ + * corruption below for the actual RED/GREEN proof of that hazard, and + * buglog.json's bug-1696 entry for the full corrected root-cause writeup. + */ +static void test_first_boot_promotion_never_truncates_own_durable_wal(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_trf_nodataloss_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + const uint64_t n_committed_entries = 5; + + /* ── "boot 1": physically append N real, fsynced WAL_NOOP entries, then + * tear down -- no cluster needed (persist_wal_async is a physical-write + * -only path when p->cluster is NULL, mirrors the sibling test's phase + * 1 and test_replicated_apply_chain_survives_cold_restart's writer). ── */ + { + persist_config_t cfg1 = persist_config_defaults(); + cfg1.wal_sync_ms = 5000; + persist_t *p1 = calloc(1, sizeof(persist_t)); + TEST_ASSERT_NOT_NULL(p1); + persist_set_workdir(p1, tmp, "ctrl-test"); + TEST_ASSERT_EQUAL_INT(0, persist_init(p1, &cfg1, NULL)); + TEST_ASSERT_EQUAL_INT(0, persist_start(p1)); + for (uint64_t i = 0; i < n_committed_entries; i++) { + TEST_ASSERT_EQUAL_INT(0, + persist_wal_async(p1, WAL_NOOP, NULL, 0)); + } + TEST_ASSERT_EQUAL_INT(0, persist_flush(p1)); + persist_stop(p1); + persist_fini(p1); + free(p1); + } + + /* ── "boot 2" / restart: fresh cluster_t + its EMBEDDED persist_t point + * at the SAME on-disk workdir. Fresh persist_t -> durable_lsn=0 until + * persist_recover() runs -- exactly a real restart. ─────────────────── */ + trf_build_cluster(); + + persist_config_t cfg2 = persist_config_defaults(); + cfg2.wal_sync_ms = 5000; + persist_set_workdir(&g_trf_c.persist, tmp, "ctrl-test"); + TEST_ASSERT_EQUAL_INT(0, + persist_init(&g_trf_c.persist, &cfg2, &g_trf_c)); + TEST_ASSERT_EQUAL_INT(0, persist_start(&g_trf_c.persist)); + g_trf_c.persist.recovery_verdict = RECOVER_CLEAN; + + /* Precondition: never recovered yet -- durable_floor reads 0 regardless + * of the N real entries physically on disk. This is the exact blind + * spot bug-1696 exploits. */ + uint64_t durable_floor_precondition = + persist_durable_floor(&g_trf_c.persist); + + ha_config_t hcfg = { + .heartbeat_interval_ms = 3000, + .heartbeat_timeout_ms = 10000, + .election_timeout_ms = 2000, + .probe_connect_timeout_ms = 500, + .max_promotion_retries = 5, + }; + TEST_ASSERT_EQUAL_INT(0, ha_init(&g_trf_c.ha, &hcfg, 0)); + g_trf_c.ha.raft_c = &g_trf_c; + g_trf_c.ha.repl = NULL; /* no healthy peer reachable at this level */ + + TEST_ASSERT_EQUAL_INT(0, + delaymgr_init(&g_trf_c.delaymgr, 5, trf_run_delay_task)); + TEST_ASSERT_EQUAL_INT(0, delaymgr_start(&g_trf_c.delaymgr)); + + /* raft_cb_become_leader's tail (on a successful promotion) reaches + * ha_arm_own_term_gate -> commit_submit/commit_set_writable_lsn, which + * needs a wired commit engine -- unlike the sibling REFUSAL tests above, + * THIS test drives promotion all the way through to CTRL_PRIMARY, so it + * must be initialized (mirrors test_convergence_verification.c's + * cv_setup, the only other fixture in this suite that reaches a + * successful raft_cb_become_leader_with_cluster_for_test promotion). */ + TEST_ASSERT_EQUAL_INT(0, + commit_engine_init(&g_trf_c.commit, &g_trf_c.persist, + /*n_peers=*/0, /*n_vcpus=*/4)); + + /* Single-node-shaped elect_ctx: n_peers=0, self_lsn=0 -- required to even + * exercise the truncation code path (gated on ha->elect_ctx != NULL) and + * reproduces the bug narrative's "self also advertises ~0 before its own + * recovery" condition. */ + ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx)); + TEST_ASSERT_NOT_NULL(ctx); + ctx->n_peers = 0; + ctx->self_lsn = 0; + ctx->self_tag = 0; + pthread_mutex_init(&ctx->lock, NULL); + sem_init(&ctx->done, 0, 0); + atomic_store_explicit(&g_trf_c.ha.elect_ctx, (void *)ctx, + memory_order_release); + + /* The crux of the reproduction: ha->phase2_done is 0 (fresh ha_init + * above), so raft_cb_become_leader's is_first_boot block fires and calls + * this hook -- which calls persist_recover() for real, from INSIDE this + * same election evaluation, racing the truncation/eligibility code + * exactly like production. */ + ha_phase2_hook = trf_phase2_hook_calls_persist_recover; + + raft_cb_become_leader_with_cluster_for_test(&g_trf_c, /*term=*/1); + + ha_phase2_hook = NULL; /* restore -- shared process-global test seam */ + + int32_t role_after_promotion = + atomic_load_explicit(&g_trf_c.ha.role, memory_order_acquire); + uint64_t durable_floor_after_promotion = + persist_durable_floor(&g_trf_c.persist); + bool truncated_after_promotion = + persist_recovery_truncated(&g_trf_c.persist); + + atomic_store_explicit(&g_trf_c.ha.elect_ctx, NULL, memory_order_release); + pthread_mutex_destroy(&ctx->lock); + sem_destroy(&ctx->done); + free(ctx); + + delaymgr_stop(&g_trf_c.delaymgr); + delaymgr_fini(&g_trf_c.delaymgr); + ha_fini(&g_trf_c.ha); + + commit_engine_fini(&g_trf_c.commit); + persist_stop(&g_trf_c.persist); + persist_fini(&g_trf_c.persist); + trf_teardown_cluster(); + + /* ── Independent THIRD-boot verification: a brand fresh persist_t (no + * cluster involved at all, so this reflects ONLY what is physically on + * disk after everything above) recovering the SAME workdir -- the + * `waldump`-equivalent proof the real bug was confirmed with. ────────── */ + uint64_t third_boot_recovered_current_lsn = 0; + { + persist_t *p3 = calloc(1, sizeof(persist_t)); + TEST_ASSERT_NOT_NULL(p3); + persist_set_workdir(p3, tmp, "ctrl-test"); + persist_config_t cfg3 = persist_config_defaults(); + cfg3.wal_sync_ms = 5000; + TEST_ASSERT_EQUAL_INT(0, persist_init(p3, &cfg3, NULL)); + TEST_ASSERT_EQUAL_INT(0, persist_start(p3)); + TEST_ASSERT_EQUAL_INT(0, persist_recover(p3)); + third_boot_recovered_current_lsn = p3->current_lsn; + persist_stop(p3); + persist_fini(p3); + free(p3); + } + + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + if (system(cmd) != 0) + fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmp); + + /* ── Assertions ───────────────────────────────────────────────────── */ + TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, durable_floor_precondition, + "fixture precondition: a never-recovered persist_t must read " + "durable_floor=0 regardless of the N real entries on disk -- this " + "is the exact blind spot bug-1696 exploits"); + + TEST_ASSERT_EQUAL_INT_MESSAGE((int32_t)CTRL_PRIMARY, role_after_promotion, + "this node is structurally healthy (clean recovery, no faults) and " + "must actually become CTRL_PRIMARY on this election -- the bug is " + "that it used to get there by first destroying its own WAL, not " + "that it fails to promote"); + TEST_ASSERT_FALSE_MESSAGE(truncated_after_promotion, + "a clean recovery of genuinely-intact WAL data must never be " + "reported as recovery-truncated"); + + TEST_ASSERT_GREATER_OR_EQUAL_UINT64_MESSAGE( + n_committed_entries, durable_floor_after_promotion, + "bug-1696: promotion must NEVER truncate this node's own durable " + "WAL to a bogus pre-recovery floor -- durable_floor after promotion " + "must still reflect the N entries committed before this restart, " + "not collapse to ~0/1 because the truncation floor was computed " + "before cluster_init_phase2()/persist_recover() ever ran"); + + TEST_ASSERT_GREATER_OR_EQUAL_UINT64_MESSAGE( + n_committed_entries + 1, third_boot_recovered_current_lsn, + "bug-1696 (independent physical proof): a THIRD boot recovering the " + "SAME on-disk workdir after everything above must still find all N " + "originally-committed entries physically present -- current_lsn == " + "N+1 (one past the highest surviving LSN) proves the real on-disk " + "WAL was never truncated away by the second boot's promotion"); + + printf("test_first_boot_promotion_never_truncates_own_durable_wal: " + "PASS\n"); +} + +/* ── bug-1696 (ACTUAL RED/GREEN proof): the promotion-eligibility gate must + * NEVER judge a node ELIGIBLE using pre-recovery-blind defaults when its own + * first-ever replay (racing this SAME election evaluation) would have + * discovered the log is genuinely CORRUPT ─────────────────────────────────── + * + * raft_cb_become_leader()'s main eligibility gate reads persist_recovery_ + * truncated()/persist_durable_floor()/persist_apply_verdict() to decide + * ELIGIBLE vs REFUSE. Before the bug-1696 fix, that gate ran BEFORE cluster_ + * init_phase2()/persist_recover() -- the ONLY call that can discover a real + * on-disk WAL is structurally corrupt for THIS process's first-ever boot. + * A freshly persist_init()'d persist_t defaults recovery_verdict= + * RECOVER_CLEAN (persist.c ~1168) and durable_lsn=0 (persist.c ~1163) -- + * "clean and empty", not "unknown" -- so the gate necessarily judged a node + * whose WAL is ACTUALLY corrupt as ELIGIBLE, let it fall through to + * CTRL_PRIMARY, and only discovered the real corruption a few lines later + * (too late: the bug-1445 re-check immediately after phase2 only re-checks + * persist_apply_resource_fenced(), never persist_recovery_truncated()). + * This is the general form of the hazard bug-1445 already patched narrowly + * for ONE flag; bug-1696 is the same hazard for the far more consequential + * durable_floor/recovery_truncated gate inputs. + * + * Fixture: "boot 1" writes two real, fsynced WAL entries, then the FIRST + * entry's on-disk CRC is corrupted (identical technique to test_commit_ + * durability.c's test_recover_verdict_crc_break_first_entry_truncated) -- + * persist_recover(), once it actually runs, will classify this as + * RECOVER_TRUNCATED. "boot 2" points a fresh cluster_t + persist_t at the + * SAME corrupted workdir with ha->phase2_done fresh (0, is_first_boot=true) + * and ha_phase2_hook wired to call persist_recover() for real, from INSIDE + * this same election evaluation -- exactly production's ordering. A single- + * node-shaped ha_elect_ctx_t (n_peers=0, self_lsn=0, no reachable peer) + * isolates the recovery_truncated conjunct: promotion_safe_lsn=0 means the + * HWM comparison alone could never explain a refusal, so if this node is + * refused, recovery_truncated is what did it. + * + * RED before the fix: role_after_election == CTRL_PRIMARY (the gate judged + * this node ELIGIBLE while blind, and nothing after phase2 re-checked + * recovery_truncated before falling through to CTRL_PRIMARY) despite + * persist_recover() -- which DID run moments later, inside the very same + * call -- correctly detecting RECOVER_TRUNCATED. + * GREEN after the fix: phase2/persist_recover() runs BEFORE the gate, so the + * gate sees the real RECOVER_TRUNCATED verdict and REFUSES promotion (stays + * alive, fenced, in_catch_up=1, never self-exits). + */ +static void +test_first_boot_promotion_refuses_when_own_first_recovery_finds_corruption(void) +{ + char tmp[256]; + strcpy(tmp, "/tmp/xce_trf_corruptboot_XXXXXX"); + TEST_ASSERT_NOT_NULL(mkdtemp(tmp)); + + /* ── "boot 1": two real, fsynced WAL_NOOP entries, then die. ─────────── */ + { + persist_config_t cfg1 = persist_config_defaults(); + cfg1.wal_sync_ms = 5000; + persist_t *p1 = calloc(1, sizeof(persist_t)); + TEST_ASSERT_NOT_NULL(p1); + persist_set_workdir(p1, tmp, "ctrl-test"); + TEST_ASSERT_EQUAL_INT(0, persist_init(p1, &cfg1, NULL)); + TEST_ASSERT_EQUAL_INT(0, persist_start(p1)); + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p1, WAL_NOOP, NULL, 0)); + TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p1, WAL_NOOP, NULL, 0)); + TEST_ASSERT_EQUAL_INT(0, persist_flush(p1)); + persist_stop(p1); + persist_fini(p1); + free(p1); + } + + /* Corrupt the FIRST entry's on-disk CRC -- breaks the chain at LSN 1 + * (identical technique to test_commit_durability.c's + * test_recover_verdict_crc_break_first_entry_truncated). A future + * persist_recover() over this workdir will classify RECOVER_TRUNCATED. */ + { + char seg_path[512]; + snprintf(seg_path, sizeof(seg_path), + "%s/ctrl-test/data/wal/seg-%016" PRIx64 ".wal", tmp, + (uint64_t)1); + int fd = open(seg_path, O_RDWR); + TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "first WAL segment must exist"); + wal_entry_hdr_t hdr; + TEST_ASSERT_EQUAL_INT((int)sizeof(hdr), + (int)pread(fd, &hdr, sizeof(hdr), 0)); + hdr.crc ^= 0xDEADBEEFu; /* corrupt the entry CRC */ + TEST_ASSERT_EQUAL_INT((int)sizeof(hdr), + (int)pwrite(fd, &hdr, sizeof(hdr), 0)); + fsync(fd); + close(fd); + } + + /* ── "boot 2" / restart: fresh cluster_t + persist_t on the SAME + * corrupted workdir. Never recovered yet -- the pre-recovery defaults + * (recovery_verdict=RECOVER_CLEAN, durable_lsn=0) read "clean and + * empty" regardless of the real on-disk corruption. This is the exact + * blind spot bug-1696 exploits. ──────────────────────────────────────── */ + trf_build_cluster(); + + persist_config_t cfg2 = persist_config_defaults(); + cfg2.wal_sync_ms = 5000; + persist_set_workdir(&g_trf_c.persist, tmp, "ctrl-test"); + TEST_ASSERT_EQUAL_INT(0, + persist_init(&g_trf_c.persist, &cfg2, &g_trf_c)); + TEST_ASSERT_EQUAL_INT(0, persist_start(&g_trf_c.persist)); + + bool recovery_truncated_precondition = + persist_recovery_truncated(&g_trf_c.persist); + recover_verdict_t recovery_verdict_precondition = + persist_recovery_verdict(&g_trf_c.persist); + + ha_config_t hcfg = { + .heartbeat_interval_ms = 3000, + .heartbeat_timeout_ms = 10000, + .election_timeout_ms = 2000, + .probe_connect_timeout_ms = 500, + .max_promotion_retries = 5, + }; + TEST_ASSERT_EQUAL_INT(0, ha_init(&g_trf_c.ha, &hcfg, 0)); + g_trf_c.ha.raft_c = &g_trf_c; + g_trf_c.ha.repl = NULL; /* no healthy peer reachable at this level */ + + TEST_ASSERT_EQUAL_INT(0, + delaymgr_init(&g_trf_c.delaymgr, 5, trf_run_delay_task)); + TEST_ASSERT_EQUAL_INT(0, delaymgr_start(&g_trf_c.delaymgr)); + ha_set_promotion_fence_window_ms(60); + + /* Single-node-shaped elect_ctx: n_peers=0, self_lsn=0, no reachable peer + * -- isolates the recovery_truncated conjunct (promotion_safe_lsn=0 + * means the HWM comparison alone can never explain a refusal here). */ + ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx)); + TEST_ASSERT_NOT_NULL(ctx); + ctx->n_peers = 0; + ctx->self_lsn = 0; + ctx->self_tag = 0; + pthread_mutex_init(&ctx->lock, NULL); + sem_init(&ctx->done, 0, 0); + atomic_store_explicit(&g_trf_c.ha.elect_ctx, (void *)ctx, + memory_order_release); + + /* The crux of the reproduction: ha->phase2_done is 0 (fresh ha_init + * above), so raft_cb_become_leader's is_first_boot block fires and calls + * this hook -- which calls persist_recover() for real, from INSIDE this + * same election evaluation, racing the eligibility gate exactly like + * production. */ + ha_phase2_hook = trf_phase2_hook_calls_persist_recover; + + raft_cb_become_leader_with_cluster_for_test(&g_trf_c, /*term=*/1); + + ha_phase2_hook = NULL; /* restore -- shared process-global test seam */ + + int32_t role_after_election = + atomic_load_explicit(&g_trf_c.ha.role, memory_order_acquire); + int in_catch_up_after_election = + atomic_load_explicit(&g_trf_c.ha.in_catch_up, memory_order_acquire); + bool recovery_truncated_after_election = + persist_recovery_truncated(&g_trf_c.persist); + recover_verdict_t recovery_verdict_after_election = + persist_recovery_verdict(&g_trf_c.persist); + + atomic_store_explicit(&g_trf_c.ha.elect_ctx, NULL, memory_order_release); + pthread_mutex_destroy(&ctx->lock); + sem_destroy(&ctx->done); + free(ctx); + + ha_set_promotion_fence_window_ms(0); /* restore default */ + ha_cancel_promotion_fence_timer(&g_trf_c.ha); + delaymgr_stop(&g_trf_c.delaymgr); + delaymgr_fini(&g_trf_c.delaymgr); + ha_fini(&g_trf_c.ha); + + persist_stop(&g_trf_c.persist); + persist_fini(&g_trf_c.persist); + trf_teardown_cluster(); + + char cmd[512]; + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp); + if (system(cmd) != 0) + fprintf(stderr, "cleanup warning: rm -rf %s failed\n", tmp); + + /* ── Assertions ───────────────────────────────────────────────────── */ + TEST_ASSERT_FALSE_MESSAGE(recovery_truncated_precondition, + "fixture precondition: a never-recovered persist_t must read " + "recovery_truncated=false (the fresh-persist_init default) " + "regardless of the real on-disk corruption -- this is the exact " + "blind spot bug-1696 exploits"); + TEST_ASSERT_EQUAL_INT_MESSAGE(RECOVER_CLEAN, recovery_verdict_precondition, + "fixture precondition: the pre-recovery default verdict reads CLEAN, " + "not the real corruption"); + + TEST_ASSERT_TRUE_MESSAGE(recovery_truncated_after_election, + "this node's own first-ever replay (triggered from inside this SAME " + "election evaluation) must have discovered and recorded the real " + "on-disk corruption by the time this call returns"); + TEST_ASSERT_EQUAL_INT_MESSAGE(RECOVER_TRUNCATED, + recovery_verdict_after_election, + "the real recovery verdict must be RECOVER_TRUNCATED once this " + "node's own replay has actually run"); + + TEST_ASSERT_NOT_EQUAL_MESSAGE((int32_t)CTRL_PRIMARY, role_after_election, + "bug-1696: a node whose OWN first-ever replay (racing this SAME " + "election evaluation) discovers a genuinely CORRUPT/TRUNCATED WAL " + "must be refused promotion -- not silently become CTRL_PRIMARY on " + "an eligibility decision made blind, before that first replay ever " + "ran"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, in_catch_up_after_election, + "a node refused for this reason must be fenced via the SAME " + "existing in_catch_up mechanism every other refusal reason uses, " + "and stay ALIVE (never self-exit)"); + + printf("test_first_boot_promotion_refuses_when_own_first_recovery_finds_" + "corruption: PASS\n"); +} + #endif /* XCE_TESTING */ void test_job_array_replay_fence(void) @@ -665,6 +1096,8 @@ void test_job_array_replay_fence(void) #ifdef XCE_TESTING RUN_TEST(test_replay_wal_engages_fence_on_resource_exhaustion_failure); RUN_TEST(test_first_boot_promotion_evaluates_own_first_replay_fence); + RUN_TEST(test_first_boot_promotion_never_truncates_own_durable_wal); + RUN_TEST(test_first_boot_promotion_refuses_when_own_first_recovery_finds_corruption); #else printf("test_job_array_replay_fence: skipped (no XCE_TESTING)\n"); #endif