myxce/.kiro/specs/raft-ha-core-safety/tasks.md
Rongsong Shen 01b7d6b7d5 test(raft-ha-core-safety): follower-reconvergence discriminator — Phase 4 sound (tasks 4.1/4.2)
Deterministic follower-level test drives the REAL ha_consume_append_entries
with a leader-style backtracking AE sequence against a multi-entry former-
primary divergent tail (12-15 @old term, committed 1..11). Follower
backtracks, conflict-truncates 14/15, accepts leader term-6 entries,
preserves committed prefix. Reviewer adversarially falsified (neuter intake
=> reached=0). VERDICT: follower-side reconvergence + applied-agreement gate
already sound on baseline => NO Phase-4 production fix needed. Redirects the
live-stall root cause to Phase 5 (channel-flap + in_catch_up-blocks-prevote-
reply). 4.1 + apply-gap pins green; 14/0 suite.
2026-06-28 03:42:06 +00:00

20 KiB
Raw Blame History

Implementation Plan: raft-ha-core-safety

TDD (project steering, mandatory): every implementation sub-task is preceded by a testsuite sub-task that lands failing (RED) tests first, then implements to GREEN. The deterministic harness (Phase 1) is the RED infrastructure that pins every later fix to a walcompare/invariant violation. No (P) markers: Phases 2–6 modify shared files (ha.c, persist.c, config_repl.c) and the RED cases share test_ha_core_safety.c, so the plan is sequential — concurrent edits would contend (same constraint as ha-convergence-recovery). Foundation Phase 1 establishes shared prerequisites and is also sequential. Correctness oracle: verify every change against raft/raft.tla and the invariant checklist in src/controller/CLAUDE.md (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering/Durability). Verify anchors before editing (files drift): the design and research.md carry 2026-06-27 file:line anchors; re-grep before each edit. Root cause (task 1.3, DONE + verified): the term-blind hypothesis is refuted. The real defect is cross-term catch-up payload loss — ha_stream_wal_to_standby silently ships PLEN=0 on a short pread/malloc-fail, and the (term,lsn)-only persist guard is payload-blind. Phase 2 targets the streamer + payload-aware guard (research.md §"Task 1.3"). A green synthetic unit test over a still-red soak does not satisfy 1.1/8.1 — closure requires the real-fs chaotic soak (7.2) clean.

  • 1. Foundation: deterministic chaotic harness and live-divergence reproduction

  • 1.1 Extend the divergence detector to a machine-checkable per-member report

    • Extend walcompare to emit a parseable per-member (lsn -> term, payload-crc, present/MISSING) report over the committed prefix, and classify each divergence as committed vs uncommitted
    • Observable completion: walcompare prints a parseable per-member report and a committed-vs-uncommitted classification; the existing soak still detects the round-3 LSN divergence through it
    • Requirements: 7.1, 7.5
    • Boundary: Deterministic Chaotic Harness
  • 1.2 Add a chaotic soak mode with an isolated verdict

    • Add an overlapping-kill mode to the soak driver (kills landing inside election windows, not only sequential kill→wait→rejoin); produce a verdict that counts walcompare divergences and committed-job loss while excluding client-discovery (bug-854) and overlayfs fdatasync (bug-924) failures from the divergence/job-loss result
    • Observable completion: the soak runs chaotic mode against real-fs release binaries and emits a verdict whose divergence/job-loss counts are unaffected by bug-854/bug-924
    • Requirements: 7.5, 8.4
    • Boundary: Acceptance Soak Gate, Deterministic Chaotic Harness
  • 1.3 Reproduce and classify the live divergence, then bisect to a seam

    • Run the chaotic soak to capture the actual walcompare committed-prefix divergence (RED), classify it via (term, lsn, pcrc) (which member, which term, committed vs uncommitted), and bisect it to the concrete ingest path; confirm or refute the term-blind-bypass hypothesis
    • Observable completion: a recorded RED divergence with member/term/lsn classification and the identified seam, written to research.md; the hypothesis is confirmed or a new trigger is named before any fix
    • Requirements: 7.1, 1.1
    • Boundary: Deterministic Chaotic Harness
  • 1.4 Stand up the unit/integration harness infrastructure

    • Create test_ha_core_safety.c with the deterministic gate/probe seams (no sleeps; teardown drains pending delaymgr tasks to avoid the bug-1224 teardown-race UAF), the (term, lsn)/apply-gap/kill-injection fixtures, and register it in the controller unit-test build; include one smoke-RED case proving the harness can observe a divergence. The per-defect RED bodies are owned by their phases (2.3 ingest, 4.1 reconvergence, 5.1 election) — this task provides the shared seams they build on, not those cases
    • Observable completion: test_ha_core_safety builds and runs, the injection fixtures and probe seams are usable by later phases, and the smoke-RED case FAILs on the baseline with no ASAN teardown errors
    • Requirements: 7.1, 7.4
    • Boundary: Deterministic Chaotic Harness
  • 1.5 Make soak primary-detection robust to the release-build log buffer

    • Task 1.3 found the release build block-buffers its --logfile and flushes only on signal, so the soak's log-grep primary/convergence detection cannot see an elected primary and false-aborts. Switch the soak's primary + convergence detection to a client probe (ctrlstatus/xce_test) instead of log-grep, so the chaotic soak runs unattended on release binaries. (Only if a client probe is insufficient, fall back to a controller release log-flush change that does not alter production logging semantics.)
    • Observable completion: run-ha-soak.sh --chaotic on the ~/tmp real-fs path with the src/build.release binaries detects the elected primary each round via the client probe and no longer false-aborts on "no primary within Ns"; a short (2-round) run reaches its verdict line
    • Requirements: 7.5, 8.3
    • Boundary: Acceptance Soak Gate, Deterministic Chaotic Harness
    • Depends: 1.2
  • 2. Committed-prefix divergence fix: cross-term catch-up payload integrity + payload-aware guard

    • Root cause (task 1.3, verified): the cross-term catch-up streamer silently ships PLEN=0 on a short pread/malloc-fail, and the (term,lsn)-only persist guard is payload-blind. Term and ingest-path are correct — the payload is lost.
  • 2.1 RED: payload-aware persistence guard tests

    • Add cases: an empty-payload (PLEN=0) entry written at an existing LSN N followed by the correct full-payload entry must end with the FULL payload (today the (term,lsn)-only idempotency guard silently no-ops the corrective write); the reverse (full then empty at the same (term,lsn)) must NOT degrade the stored entry to empty; an identical (term, payload-crc) re-write is a safe idempotent no-op
    • Observable completion: the new persistence cases FAIL on the baseline because the guard compares only (term, lsn) and cannot see the payload difference
    • Requirements: 1.1, 1.2, 2.1, 2.2, 3.1, 3.4
    • Boundary: Persist Conflict Guard
  • 2.2 Make the persistence guard (term, lsn, payload-crc)-aware

    • Extend the LSN-only idempotency check (persist.c:822): identical (term, payload-crc) at an LSN is an idempotent no-op; a differing payload-crc (or term) at an existing LSN is surfaced for repair, never a silent return 0; never accept a PLEN=0 write as the durable form of a known-non-empty entry; preserve persist-before-ACK, the term index, and WAL_LEGACY_TERM
    • Observable completion: the 2.1 cases pass; an empty-over-real write at an existing committed LSN no longer silently survives; identical re-writes stay idempotent
    • Requirements: 1.1, 1.2, 2.1, 2.2, 2.4, 3.1
    • Boundary: Persist Conflict Guard
  • 2.3 RED: cross-term catch-up streamer payload-integrity tests

    • Add a case reproducing the live signature: a cross-term catch-up (stored term < leader current term, bug-1067 path) whose entry-body pread returns short / malloc fails must NOT result in a persisted PLEN=0 entry at a committed LSN — instead the stream must error and retry. Also add the defensive case that a term==0-shaped catch-up entry routes through the reconcile seam (no direct-persist bypass), and the unrecoverable-fence case (a node that cannot obtain the majority-agreed committed prefix refuses promotion instead of serving divergent state). Instrument payload_len at send and at on_persist_entry to disambiguate send-side-empty vs transitive-empty propagation
    • Observable completion: the streamer case FAILs on the baseline because the short-read/malloc-fail path silently ships PLEN=0; the seam-routing and fence cases FAIL because those paths/fence do not exist yet
    • Requirements: 1.1, 1.2, 2.1, 3.1, 3.2, 3.3, 7.1
    • Boundary: Catch-up Payload Integrity, Unified Entry Ingest
    • Depends: 2.2
  • 2.4 Fix the streamer payload integrity + defensive seam routing + fence

    • Core: in ha_stream_wal_to_standby (config_repl.c ~2154-2163), fail loud on a short pread/malloc-fail with a bounded retry instead of freeing the buffer and shipping PLEN=0; a non-empty entry is never transmitted empty. Disambiguate and handle send-side-empty vs transitive-empty propagation per the 2.3 instrumentation. Defensive: retire the term-blind ha_apply_single_entry direct-persist branch so all ingest routes through the seam; surface genuinely-unrecoverable divergence as a loud non-promotable fence (never self-terminate); defer file I/O to delaymgr (never the callback/iomgr thread)
    • Observable completion: 2.3 passes (streamer never ships empty; seam-routing and fence cases green); on the 1.3 reproduction path (real-fs release chaotic soak) walcompare reports zero committed divergence at the previously-divergent LSNs
    • Requirements: 1.1, 1.2, 1.3, 2.1, 2.3, 3.1, 3.2, 3.3, 3.4
    • Boundary: Catch-up Payload Integrity, Unified Entry Ingest
    • Depends: 2.2, 2.3
  • 3. Snapshot-boundary reconciliation

  • 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
    • Boundary: Snapshot Boundary Reconcile
  • 3.2 Establish the snapshot (term, lsn) boundary

    • Make snapshot install establish a verifiable (term, lsn) at the checkpoint so post-install AE intake reconciles correctly and no committed entry below the checkpoint can diverge
    • Observable completion: 3.1 passes; a member that installed a snapshot matches the leader's (term, payload-crc) at and below the checkpoint boundary
    • Requirements: 3.1, 5.1
    • Boundary: Snapshot Boundary Reconcile
    • Depends: 2.4
  • 4. Standby reconvergence completeness

    • Root cause (Phase-4/5 investigation, research.md): a recovered standby holding a divergent uncommitted tail above the new leader's committed point (former-primary shape, e.g. ctrl-2 with 12-15 at an old term while leader committed 11) never reaches ae_already_have — it loops on ae_reject_logmismatch, its control channel flaps, and it never rejoins. A short-suffix standby (ctrl-3) DOES reconverge, so the backtrack works for small divergence but not the former-primary tail.
  • 4.1 RED: divergent-tail reconvergence + apply-gap gate tests

    • Primary case: construct a recovered standby with a MULTI-entry divergent uncommitted tail above the leader's committed point and assert it reconverges to follower — truncates the conflicting suffix and reaches ae_already_have (no permanent ae_reject_logmismatch loop). Secondary case: a rejoining node with an injected catch-up apply gap must not be reported converged/healthy until applied_lsn and log_len equal the leader's committed boundary
    • Observable completion: both cases FAIL on the baseline (the divergent-tail case loops on log-mismatch / never truncates; the apply-gap case reports healthy at match-index alone)
    • Requirements: 5.1, 5.2, 5.3, 5.4, 7.2
    • Boundary: Standby Reconvergence
  • 4.2 Verify divergent-tail reconvergence + applied-agreement gate (no production fix needed)

    • Discriminator outcome (4.1, adversarially confirmed): the follower-side AE log-matching backtrack + follower-AE-conflict-truncate path ALREADY reconverges a multi-entry former-primary divergent tail (truncates 12-15 old-term, accepts the leader's term-6 entries, preserves committed prefix 1..11) when driven deterministically WITHOUT a channel flap; and the applied-agreement healthy gate (ha_process_standby_state) already blocks "converged" until applied_lsn == committed_lsn. Neutering ha_consume_append_entries makes the test fail (reached=0), proving the real path does the work. No production change required in Phase 4 — the live stall is NOT follower-side reconvergence. The fix moves to Phase 5 (channel-flap suppression + in_catch_up-blocks-PreVote-reply).
    • Observable completion: 4.1's divergent-tail + apply-gap tests pass as regression pins; no production diff in Phase 4 (verified by the discriminator)
    • Requirements: 5.1, 5.2, 5.3, 5.4
    • Boundary: Standby Reconvergence
    • Depends: 2.4
  • 5. Election liveness and safety under churn

    • Root cause (Phase-4/5 investigation, research.md): the live stall is NOT a missing election-restart (retry works) — it is a recovered standby treating a TRANSIENT control-channel RDHUP as "primary died" and launching an UNWINNABLE PreVote-failover storm while a current-term primary is alive (peers deny/ignore, round times out, reconnect, repeat ~15s). The node retries an election it can never win instead of rejoining the live primary.
  • 5.1 RED: spurious-failover suppression + election liveness + Election Safety tests

    • Primary case: a recovered standby whose primary control channel flaps (transient RDHUP) while a current-term primary is alive and quorum-reachable must NOT enter an unbounded ha_begin_failover/PreVote timed out loop — it reconnects and rejoins as follower. Liveness case: on GENUINE primary loss, repeated rapid kill/restart rounds yield a higher-term primary each round; a scripted failed round is followed by another election. Safety case: across overlapping kills, at most one member ever reaches committing-primary per term (Election Safety, mapped to raft/raft.tla)
    • Observable completion: the spurious-failover case FAILS on the baseline (unbounded prevote-timeout loop while a live primary exists); the liveness cases pass/fail per the stall; the Election Safety assertion runs and reports per-term primary counts
    • Requirements: 4.1, 4.3, 6.1, 6.2, 6.3, 7.3
    • Boundary: Election Liveness
  • 5.2 Suppress unwinnable failover on transient RDHUP; keep genuine-loss liveness and single-leader safety

    • On a control-channel RDHUP, attempt reconnect + catch-up first, and/or back off failover when a node's own PreVotes repeatedly fail to reach quorum because a current-term leader exists — only escalate to a real election when the primary is genuinely unreachable to the quorum. Preserve: genuine primary loss still elects a higher-term primary (no permanent stall while quorum reachable); step-down-on-higher-term paired with election restart; at-most-one-committing-primary-per-term
    • Observable completion: 5.1 spurious-failover case passes (a recovered standby rejoins a live primary instead of prevote-spinning); genuine-loss liveness still completes each round; the Election Safety assertion still holds
    • Requirements: 4.1, 4.2, 4.3, 6.1, 6.2, 6.3
    • Boundary: Election Liveness
    • Depends: 4.2
  • 6. Integration: single-seam invariant end-to-end

  • 6.1 Verify leader→standby replication carries (term, payload) faithfully end-to-end

    • Integration check that the live leader replication AND the cross-term catch-up streaming path transmit each entry's full (term, payload) to the receiver: no term==0 on the wire from a live leader (defensive), and — the real concern — no non-empty entry ever arrives with PLEN=0; the walAck catch-up continuation feeds only the seam ingest path
    • Observable completion: an end-to-end test (or instrumented soak round) shows leader and standby hold identical (term, payload-crc) for every replicated and cross-term-caught-up LSN; walcompare --report clean across members
    • Requirements: 2.1, 2.3, 3.2
    • Boundary: Catch-up Payload Integrity, Unified Entry Ingest
    • Depends: 2.4
  • 6.2 Verify the single-seam invariant and restart identity

    • Assert no ingest path (live replication, catch-up batch, snapshot post-install, rejoin) writes the WAL outside the reconcile seam, and that each entry's (term, lsn) identity survives a restart/recovery. Carry-forward from 2.4: the term-blind ha_apply_single_entry direct-persist branch was NOT structurally retired in 2.4 (its empty-over-full safety is enforced by the 2.2 payload guard and pinned by test_2_3_termblind_route_payload_guarded/test_2_3_unrepairable_catchup_raises_nonpromotable_fence). This task must either structurally retire that bypass (route term-0 catch-up through the seam, with its own RED first) OR formally document it as safe-by-persist-guard and adjust the invariant wording accordingly
    • Observable completion: an integration test confirms every ingest path reconciles through the seam and that entry terms are preserved across restart; the term-blind bypass is either removed (no direct-persist path remains) or documented safe-by-guard with the pin tests cited
    • Requirements: 1.1, 1.2, 2.4, 3.1
    • Depends: 2.4, 3.2, 4.2
  • 7. Validation: regression and acceptance soak gate

  • 7.1 Full controller suite under ASAN with the new harness green

    • Build and run the full controller unit suite under ASAN (suite-by-suite if the aggregate hits the known 300s cap); fix any regression from the ingest/persist/election changes
    • Observable completion: the full suite is green under ASAN and every test_ha_core_safety case passes with no ASAN errors
    • Requirements: 7.4
    • Depends: 6.2, 5.2
  • 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
    • Depends: 1.5, 6.2, 5.2
  • 7.3 Hypothesis-validation closeout

    • Confirm the live soak divergence is eliminated by the implemented fixes (not by synthetic unit tests alone); if any residual divergence remains, classify it via walcompare (term, lsn, pcrc), record the new finding in research.md, and add a follow-up fix task rather than declaring closure
    • Observable completion: the soak divergence count is zero with closure justified by the live soak, OR a recorded new finding plus a follow-up task; raft/raft.tla invariants (Election Safety, Leader Completeness, State Machine Safety, Commit Ordering) reviewed against the final behavior
    • Requirements: 1.1, 1.4, 7.1, 8.1
    • Depends: 7.2

Implementation Notes

  • Task 2.2 — payload repair must be TIP-ONLY (consensus-safety, caught in review round 1). The (term,lsn,payload-crc)-aware guard repairs an empty-over-real entry by truncate-to-lsn-1+re-append, but this is safe ONLY when lsn == current_lsn-1 (the conflict is at the tip with nothing committed above). For a BELOW-tip conflict (lsn < current_lsn-1 — the actual live LSN-9 case with 10/11/12 above), truncating the tail would drop committed entries and roll committed_lsn backward (State Machine Safety violation). Below-tip is handled as a loud non-promotable fence (reuses recovery_unrecoverable sticky flag), never mutating any boundary; the real reconvergence comes from the 2.4 streamer fix + re-stream. The tip path snapshots/restores committed_lsn under p->lock so the boundary is never observably lowered. Any future in-WAL repair must preserve: never drop a committed entry above the conflict, never lower committed_lsn.

  • Task 2.4 — root-cause source fix + a deferral. ha_stream_wal_to_standby now reads the entry body via stream_read_entry_body (bounded retry, max 4 no-progress attempts, partial-read-accumulating, no sleep); on persistent short pread/malloc-fail it emits a loud ERROR and ABORTS the stream (flushes the valid contiguous prefix, drops the unreadable entry, returns -EIO) instead of shipping PLEN=0. The incomplete stream is re-driven by the ACK-driven catch-up continuation. Combined with the 2.2 self-healing guard, this closes the live cross-term payload-loss divergence. Deferred: the structural retirement of the term-blind ha_apply_single_entry bypass — its empty-over-full safety is enforced by the 2.2 guard + pinned by two passing tests, so it cannot corrupt committed state; the structural removal is tracked to task 6.2. Live ≥6-round soak proof is task 7.2.