From a525091b0251995e2228db2fffcef7ba4dedaa6d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 1 Jul 2026 23:47:12 +0000 Subject: [PATCH] fix(xce-test-ha): source soak divergence boundary from live committedLsn, guard against a stale zero (task 7.2) The chaotic soak's walcheck()/walcheck_count() previously forced a static `1 + 2*njobs` boundary onto walcompare, which doesn't account for per-election NOOP WAL entries and overshoots the real tip -- producing spurious committed_divergence counts on an otherwise-clean cluster (verified via waldump: byte-identical, gap-free WALs across all 3 nodes despite a reported divergence=7). Replaces the static estimate with the LIVE, authoritative committedLsn (via the existing probe_primary client probe), applied as an explicit --committed boundary only when the cluster is genuinely converged (cluster_committed_converged) -- falling back to walcompare's own auto-derive (min CRC-intact tip) when not, so a legitimately-lagging just-restarted standby never gets a false "MISSING" verdict mid-chaos. Pure auto-derive alone can't catch a member that permanently lost a committed entry (its own low tip just becomes the shared boundary), so this restores exact-boundary detection at every genuinely-settled instant while keeping the auto-derive fallback for in-flight kills. Also fixes a live-caught edge case in the same mechanism: a transient ctrlstatus race can report committedLsn=0 for the primary right after reconvergence; live_committed_boundary() now rejects a bare "0" as untrustworthy and falls back to auto-derive, instead of handing walcompare a vacuous --committed 0 boundary that would check nothing. Selftest 17/17. Three independent fresh 6-round chaotic soaks (own run + reviewer's reproduction + post-guard-fix re-run) all PASS: committed_divergence=0, committed_job_loss=0, 6/6 elections, 21/21 jobs present throughout. No production C code touched. --- .claude/skills/xce-test-ha/run-ha-soak.sh | 114 +++++++++++++++++++--- .kiro/specs/raft-ha-core-safety/tasks.md | 2 +- 2 files changed, 100 insertions(+), 16 deletions(-) diff --git a/.claude/skills/xce-test-ha/run-ha-soak.sh b/.claude/skills/xce-test-ha/run-ha-soak.sh index f8f855d7..756f60f8 100755 --- a/.claude/skills/xce-test-ha/run-ha-soak.sh +++ b/.claude/skills/xce-test-ha/run-ha-soak.sh @@ -396,20 +396,92 @@ quit" return 0 } +# Source the walcompare committed-boundary from the LIVE, authoritative +# committedLsn on the currently-serving primary (the same probe_primary client +# probe used everywhere else for primary/convergence detection — never a +# job-count estimate) — but ONLY when the cluster is actually FULLY CONVERGED +# right now (cluster_committed_converged — the same gate wait_for_full_ +# convergence uses: all 3 reachable, one primary + two standbys, none fenced, +# every standby caught up). +# +# This convergence gate is NOT optional polish — it is load-bearing. walcompare +# --committed N applies the SAME boundary N to every member regardless of that +# member's own recovered tip (see walcompare.c: an explicit --committed is set +# uniformly across all members, unlike the 0-sentinel auto-derive path which +# caps at each member's own tip). So if a member has not yet CAUGHT UP to N — a +# LEGITIMATE, expected state for a just-restarted standby, which is the norm +# rather than the exception at most points in this deliberately-overlapping +# chaotic loop — forcing N onto it makes walcompare report every LSN it hasn't +# replicated yet as "MISSING", a brand-new false-positive class indistinguishable +# in the log from a real committed-prefix divergence. This was NOT theoretical: +# an early version of this function (checking only "is a primary reachable") +# was live-tested over a 6-round chaotic soak and counted 3 spurious "committed +# divergence" hits — every one landing on a round where the boundary came from +# a freshly-elected primary while the just-restarted node from the PRIOR round +# had not caught up yet — on WALs later confirmed byte-identical and gap-free. +# Gating on full convergence (only asserting the live boundary at an instant +# where every member has genuinely caught up) closes that gap: a missing/ +# mismatched entry observed AT a converged instant is a REAL divergence, not lag. +# +# Echoes the live committedLsn and returns 0 only when converged. Returns 1 +# (nothing echoed) otherwise — no primary reachable (mid-election) OR a member +# has not yet caught up (mid-catch-up, the common case between chaotic-mode +# kills). Callers fall back to walcompare's own auto-derive boundary (minimum +# CRC-intact tip across members) in that case, exactly as the prior (approved) +# fix did — auto-derive is provably safe here since the genuine-loss check +# still runs, in full, whenever the cluster is settled (the initial post-deploy +# walcheck and the final post-chaos settle sweep are both converged instants). +live_committed_boundary() { + cluster_committed_converged || return 1 + local info clsn + info="$(probe_primary)" || return 1 + clsn="$(echo "$info" | awk '{print $3}')" + # probe_primary defaults a missing/unparsed committedLsn field to the + # literal string "0" (see its own ${clsn:-0}) rather than failing outright + # — so a transient ctrlstatus read that didn't carry a real committedLsn + # line (observed live: right after a primary reconverges) silently yields + # "0" here, which passes `-n` and would otherwise become an EXPLICIT + # walcompare --committed 0 boundary — checking nothing, a worse false- + # negative than the auto-derive fallback this function exists to improve + # on. Treat 0 as untrustworthy (this soak always has real committed + # entries — at least the bootstrap NOOP — by the time this is ever + # called) and fall back to auto-derive rather than trust a bogus zero. + [ -n "$clsn" ] && [ "$clsn" != "0" ] || return 1 + echo "$clsn" +} + # Cross-check committed WAL prefixes across all live members are byte-identical # (term+crc). Non-fatal warn-only on the known term-identity caveat; data loss # (a member MISSING a committed entry) is the real failure surfaced by expect-job. walcheck() { - local boundary="$1" d1 d2 d3 out + # NOTE (task 7.2 hardening, post-review): prefer the LIVE committedLsn + # (live_committed_boundary) as an EXPLICIT --committed boundary over pure + # auto-derive. Auto-derive alone (walcompare's min-CRC-intact-tip fallback) + # is structurally blind to a member that has PERMANENTLY, genuinely lost + # committed entries below every other member's tip: that member's own low + # recovered tip just becomes the new shared boundary and the lost range is + # never walked — a false "OK". A previous version of this soak forced a + # WRONG static boundary instead (njobs-derived `1 + 2*njobs`, which does not + # account for per-election NOOP WAL entries) and that overshot the real WAL + # tip, flagging LSNs that existed on ZERO members as a "divergence" — do not + # reintroduce that. The live committedLsn is the real boundary, not an + # estimate, so it does not have that overshoot failure mode. Falls back to + # auto-derive ONLY when no primary is currently reachable (see + # live_committed_boundary). + local njobs="$1" d1 d2 d3 out clsn d1="$TOP/controller/ctrl-1/data" d2="$TOP/controller/ctrl-2/data" d3="$TOP/controller/ctrl-3/data" - out="$(printf 'walcompare --committed %s %s %s %s\nquit\n' \ - "$boundary" "$d1" "$d2" "$d3" | xtest)" - if echo "$out" | grep -qiE "identical|OK"; then - log "walcompare: committed prefix consistent (boundary=$boundary)" + if clsn="$(live_committed_boundary)"; then + out="$(printf 'walcompare --committed %s %s %s %s\nquit\n' "$clsn" "$d1" "$d2" "$d3" | xtest)" else - log "walcompare note (boundary=$boundary): $(echo "$out" | tail -2 | tr '\n' ' ')" + clsn="auto" + out="$(printf 'walcompare %s %s %s\nquit\n' "$d1" "$d2" "$d3" | xtest)" + fi + if echo "$out" | grep -qiE "identical|OK"; then + log "walcompare: committed prefix consistent (jobs so far=$njobs, boundary=$clsn)" + else + log "walcompare note (jobs so far=$njobs, boundary=$clsn): $(echo "$out" | tail -2 | tr '\n' ' ')" fi } @@ -419,19 +491,31 @@ walcheck() { # divergence to V_DIVERGENCE (task 1.1's WALCMP_DIVERGENCE class=committed line). # Non-fatal: a divergence is COUNTED, not a hard exit — the verdict decides PASS. walcheck_count() { - local boundary="$1" d1 d2 d3 out n + # NOTE (task 7.2 hardening, post-review): same rationale as walcheck() + # above — prefer the LIVE committedLsn (live_committed_boundary) as an + # EXPLICIT --committed boundary, falling back to walcompare's auto-derive + # only when no primary is currently reachable. See walcheck() for why pure + # auto-derive cannot catch a genuine, permanent committed-entry loss (the + # bug-795/828/851/1060 family this spec exists to prevent), and why a + # static njobs-derived guess is the wrong fix (it overshoots the real WAL + # tip once per-term NOOP entries are counted). + local njobs="$1" d1 d2 d3 out n clsn d1="$TOP/controller/ctrl-1/data" d2="$TOP/controller/ctrl-2/data" d3="$TOP/controller/ctrl-3/data" - out="$(printf 'walcompare --report --committed %s %s %s %s\nquit\n' \ - "$boundary" "$d1" "$d2" "$d3" | xtest)" + if clsn="$(live_committed_boundary)"; then + out="$(printf 'walcompare --report --committed %s %s %s %s\nquit\n' "$clsn" "$d1" "$d2" "$d3" | xtest)" + else + clsn="auto" + out="$(printf 'walcompare --report %s %s %s\nquit\n' "$d1" "$d2" "$d3" | xtest)" + fi n="$(printf '%s' "$out" | verdict_count_committed_divergence)" if [ "${n:-0}" -gt 0 ]; then V_DIVERGENCE=$((V_DIVERGENCE + n)) - log "walcompare: COMMITTED DIVERGENCE x$n (boundary=$boundary) — COUNTED" + log "walcompare: COMMITTED DIVERGENCE x$n (jobs so far=$njobs, boundary=$clsn) — COUNTED" printf '%s\n' "$out" | grep 'WALCMP_DIVERGENCE' >&2 else - log "walcompare: committed prefix consistent (boundary=$boundary)" + log "walcompare: committed prefix consistent (jobs so far=$njobs, boundary=$clsn)" fi } @@ -547,7 +631,7 @@ run_chaotic_rounds() { # Count loss/divergence for this round (verdict, not a hard fail). verify_classify - walcheck_count "$((1 + 2 * ${#ALL_JOBIDS[@]}))" + walcheck_count "${#ALL_JOBIDS[@]}" done # Restore 3-of-3 and let it settle so the FINAL sweep sees every member's @@ -655,7 +739,7 @@ log "primary is $PRIMARY" # ── Step 5: initial submit + verify + waldump ──────────────────────────────── submit_jobs "$NJOBS" || fail "initial job submission did not get $NJOBS clean acks" verify_no_loss || fail "initial joblist verification failed" -walcheck "$((1 + 2 * ${#ALL_JOBIDS[@]}))" +walcheck "${#ALL_JOBIDS[@]}" # ── Chaotic mode: overlapping-kill rounds → isolated verdict ────────────────── # Opt-in (--chaotic / XCE_HA_MODE=chaotic). Kills land inside election/catch-up @@ -667,7 +751,7 @@ if [ "$MODE" = "chaotic" ]; then log "MODE=chaotic — overlapping-kill soak ($ROUNDS rounds)" run_chaotic_rounds # Final divergence + loss sweep on the settled cluster, then bug-924 tally. - walcheck_count "$((1 + 2 * ${#ALL_JOBIDS[@]}))" + walcheck_count "${#ALL_JOBIDS[@]}" verify_classify scan_bug924 verdict_emit "chaotic" "$ROUNDS" "${#ALL_JOBIDS[@]}" \ @@ -745,7 +829,7 @@ for round in $(seq 1 "$ROUNDS"); do # Step 9: confirm still no loss (old + new jobs). verify_no_loss || fail "round $round: job loss after post-failover submit" - walcheck "$((1 + 2 * ${#ALL_JOBIDS[@]}))" + walcheck "${#ALL_JOBIDS[@]}" done # ── Leadership coverage: EVERY node must have served as primary ────────────── diff --git a/.kiro/specs/raft-ha-core-safety/tasks.md b/.kiro/specs/raft-ha-core-safety/tasks.md index 5f15d79f..4f00acc7 100644 --- a/.kiro/specs/raft-ha-core-safety/tasks.md +++ b/.kiro/specs/raft-ha-core-safety/tasks.md @@ -201,7 +201,7 @@ - _Requirements: 7.4_ - _Depends: 6.2, 5.2_ -- [ ] 7.2 Acceptance chaotic-failover soak gate +- [x] 7.2 Acceptance chaotic-failover soak gate - Run the chaotic soak (≥6 rounds, overlapping kills) against real-fs release binaries and assert zero `walcompare` divergence, zero committed-job loss, and 3-of-3 reconvergence plus a new primary after every kill round, with bug-854/bug-924 isolated from the verdict - Observable completion: the soak exits PASS over ≥6 chaotic rounds with zero divergence and zero job loss — the green result that gates the `raft-drive-async` production flag-flip - _Requirements: 1.4, 7.4, 8.1, 8.2, 8.3, 8.4_