myxce/.kiro/specs/ha-commit-durability/brief.md

Brief: ha-commit-durability

Problem

A 3-node HA controller failover test permanently lost committed jobs. The client submitted 10 jobs across several forced primary kills; the 7 earliest (JobIds 0-0-1781708993..1781709035, WAL LSN ≤20) — which had been replicated and majority-ACKed, and survived the first failover — were gone by the end, while only the 3 latest survived. Losing an acknowledged, committed job is a correctness (data-loss) failure, the most severe class for this system.

Current State

The HA consistency machinery is implemented (commit engine, WAL replication, convergence/truncate-on-demotion, single-leader own-term gate, PreVote, vote-freshness bug-339). But "committed" does not imply "durable on a majority":

• WAL fsync is asynchronous — a timer thread (persist.c:173 fsync_worker) fdatasyncs every wal_sync_ms. persist_append does not fsync before returning; the standby does not fsync before sending its WalAck.
• The commit engine advances committed_lsn on majority ACK (commit.c:303 commit_on_ack, repl_wait_majority_ack). An ACK only proves the entry reached the OS page cache — it can be durable on zero disks at the moment it is declared committed.
• Recovery silently truncates to nothing. When killed nodes restart, persist_recover hits a CRC chain break at a zeroed/never-fsync'd region (ctrl-3.log:1597 prev_crc=00000000 at offset 40) and stops replay at ~0 entries — but the node still becomes eligible.
• A hollow node wins and overwrites the good timeline. With durability lost on the killed nodes and failover quorum collapsed to required=1, the freshness gate cannot fence the stale candidate. ctrl-3 became PRIMARY term=4 advertising committed_lsn=21 over an empty job table (ctrl-3.log:1612); the node that did hold the data (ctrl-1, applied_lsn=22) converged down to it (ctrl-3.log:1618). LSN ≤20 discarded forever.

This violates the Leader-Completeness assumption documented in src/controller/CLAUDE.md ("committed entries are on a majority" presumes majority-ACK ⇒ durable-on-a-majority) and the TLA+ mapping log[i] = WAL on disk.

Desired Outcome

1. An operation reported committed is durable on a majority of nodes before it is declared committed — surviving simultaneous crash + restart of any minority.
2. A node that cannot recover its WAL to a valid committed tip is fenced from leadership (fail-loud), never silently truncating to zero and then leading; the cluster never converges onto a timeline below its known committed high-water mark.

Approach

Two coupled requirements (chosen scope: Durability + promotion fence):

• R1 Durable commit (fsync-before-ACK): the standby fdatasyncs the appended WAL bytes before emitting WalAck; the primary counts its own vote toward majority only after its local fdatasync. committed_lsn advances only on a durable majority. Preserve throughput via group-commit / batched fdatasync (amortize the sync across pending appends) rather than one sync per entry. Keep wal_sync_ms async path only for non-committing/best-effort writes, if any remain.
• R2 Recovery & promotion fence: a WAL CRC/replay failure that prevents reconstructing a contiguous committed tip makes the node ineligible to become primary until it has caught up from a healthy peer (FATAL or fence, never silent truncate-to-zero-then-lead). A node must not win election or self-promote advertising a committed_lsn below the cluster's known committed high-water mark, even under degraded single-voter quorum.

Honor the user's fail-loud preference: surface the real durability/recovery failure rather than masking it.

Scope

• In: fsync-before-ACK on the standby apply path and primary self-count; durable-majority gating of committed_lsn; group-commit batching to bound the latency cost; fencing an un-recovered/corrupt-WAL node from promotion; preventing a sub-high-water-mark leader from being elected under degraded quorum; unit tests for each; the 3-node live E2E that reproduces this incident (kill-primary-under-load, assert zero committed-job loss).
• Out: WAL wire format / ACK message shape (ha-wal-replication); per-LSN majority math (ha-commit-engine — consumed, the durability precondition of its ACK input is what changes); demotion truncate/self-restart and rejoin reconciliation of an intact log (ha-convergence-recovery); promotion no-op / own-term gate (ha-single-leader-role); raw vote-grant arithmetic (raft-library — this spec adds the durability+high-water precondition, not new vote math); entity-bearing snapshots (roadmap); client-side failover (libxce-failover).

Boundary Candidates

• Durable-commit seam: standby fdatasync-before-WalAck + primary self-fdatasync before majority self-count (the "ACK ⇒ durable" guarantee).
• Promotion-eligibility seam: recovery-integrity + committed-high-water check gating raft_cb_become_leader / PreVote eligibility.

Out of Boundary

• Tuning the async fsync_worker cadence as a "fix" (it is the defect for committing writes, not a knob to turn).
• Changing how a majority is counted (commit-engine) or how votes are granted (raft-library) beyond adding the durability/high-water precondition.
• Worker-reported fact resync across restart (worker-rejoin-resync, roadmap backstop).

Upstream / Downstream

• Upstream: ha-commit-engine (committed_lsn semantics it now gates on durable ACKs), ha-wal-replication (ACK/catch-up machinery this makes durability-correct), persist.c fsync infrastructure, ha-prevote + vote-freshness (bug-339) eligibility path it strengthens.
• Downstream: ha-convergence-recovery E2E (its single-outcome guarantee depends on this), component-failover-adoption and any real failover deployment.

Existing Spec Touchpoints

• Extends/strengthens: ha-wal-replication (durability precondition on ACK), ha-commit-engine (durable-majority), ha-convergence-recovery (assumes intact persisted committed state — this spec makes that assumption true).
• Adjacent (avoid overlapping): ha-single-leader-role, raft-library, ha-prevote.

Constraints

• C / gnu17, src/controller/persist.c, commit.c, repl.c, ha.c, cluster.c; TDD mandatory (tests in testsuite/ before C changes).
• Async-I/O rule: never block the iomgr thread — fdatasync must not stall heartbeat/vote handling; use the dedicated persist/fsync thread + group-commit and signal the commit waiter on completion.
• Must verify against raft/raft.tla and the src/controller/CLAUDE.md Protocol Verification Checklist (commitIndex monotonic; leader holds all committed entries).
• Live E2E needs /xce-deploy-ha (cluster at /tmp/xce-ha-deploy); reproduction harness = xce_test driving jobsub + primary kills. An automated soak reproduction now exists at src/tools/test-commit-durability.sh (rolling SIGKILL+restart under load, asserts every acked job survives).
• Performance: durable-majority adds sync latency to the submit path — group-commit is required, not optional.

Reproduction Findings (2026-06-17, from building the E2E)

• bug-661 is a RACE, not a deterministic failure. wal_sync_ms is hardcoded to DEFAULT_WAL_SYNC_MS = 10 ms (config.h:86) with no CLI/env/config override. The committed-but-not-fsync'd window is ~10ms, so a single-node SIGKILL+restart only loses a committed job when the kill lands inside that window AND the truncated node re-leads before catch-up repairs its WAL. The soak reproduced real loss once (4 committed jobs) but passed an 8-round run clean — confirming the rarity.
• Required test seam (NEW spec item): a configurable fsync interval (e.g. --wal-sync-ms, plumbed into persist_config_t.wal_sync_ms). Setting it to several seconds makes every committed entry provably un-fsync'd at kill time, turning the racy soak into a deterministic RED. This seam is also the natural way to unit-test fsync-before-ACK. None exists today — add it as part of Requirement 1's testability (cf. the convergence spec's XCE_TESTING restart seam).
• Acceptance asymmetry: a soak FAIL proves the bug; a soak PASS does not prove durability (the race may simply not have fired). After the fix, the soak must pass deterministically regardless of timing — that, plus the deterministic --wal-sync-ms test, is the real acceptance bar.
• Amplifier confirmed: loss needs a killed+restarted node to re-lead with a truncated WAL before replication catch-up repairs it. The fix must therefore also fence a node that has not caught up to the committed high-water mark from leading (Requirement 2).

Design Decision — durability mechanism (2026-06-17, with user)

Invariant (user-stated, authoritative): when the controller replies ok to a client, the operation's WAL entry MUST already be persisted (fdatasync'd) on a fault-tolerant majority — self + ⌊N/2⌋ standbys.

Exact defect located: commit.c:majority_candidate_locked counts the primary's own contribution as cap = highest_appended_lsn(p) — every appended (page-cache) entry, not every fdatasync'd one. On 3 nodes a commit needs self + 1 standby; the standby ACK is durable (persist_wal_write_replicated:515 fdatasyncs before replying) but the primary's copy is not (it relies on the fsync_worker timer). So a "committed" entry is durable on exactly 1 node → zero durability fault-tolerance. That is bug-661.

Chosen direction (user): synchronous persistence — eliminate fsync_worker as the durability mechanism.

• The submit path already blocks off the iomgr thread waiting for majority replication (job.c:568-569; persist_commit_barrier → commit_submit(COMMIT_MODE_WAIT)), so adding the primary's own fdatasync on that path is liveness-safe — it does not stall heartbeats.
• Minimal correctness change: (1) primary fdatasyncs its entry before its self-contribution covers that LSN (symmetric with the standby path at persist.c:515); (2) commit.c self-contribution uses a durable_lsn (highest fdatasync'd), not highest_appended_lsn; (3) reply stays gated on the commit barrier → reply-ok ⇒ durable-majority.
• Throughput: per-entry fdatasync. The dominant amplifier (a job array fanning out to N entries → N fsyncs) is removed by the roadmap item job-array-single-wal-record (one WAL record per array → one fsync). Cross-reference that item; this spec assumes it lands or is acceptable interim.
• Group-commit IS in scope (revised 2026-06-18 at design-review). Initially deferred, but /kiro-validate-design found the highest-volume WAL path is WAL_MODE_RETAIN facts (job start/finish/fail/rusage) — not array-collapsible, not covered by job-array-single-record — so a pure per-entry synchronous design penalizes the dominant writes. Decision (with user): repurpose fsync_worker into a demand-driven group-commit flusher (batch fdatasync, advance durable_lsn, wake commit-eval) as part of this spec. The roadmap "WAL group-commit batching" item is absorbed here.
• Open for design: whether to delete the fsync_worker thread entirely or retain a minimal best-effort flush for any non-client-acked / local-only writes (e.g. checkpoint markers already fsync inline; audit wal_classify for any write that gates no reply). Confirm no client-acked op bypasses the commit barrier.