lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ha): first-boot promotion evaluated eligibility before its own WAL recovery ran (bug-1696)

Browse source 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
Rongsong Shen 2026-08-27 08:26:09 +00:00
parent f886c80930
commit 47532f0a76
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 575 additions and 126 deletions
Download patch file Download diff file Expand all files Collapse all files

268
src/controller/ha.c
View file

@ -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),