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

220 lines
31 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Gap Analysis — ha-commit-durability
> Brownfield analysis against the XCE controller (`src/controller/`). Informs `/kiro-spec-design`.
> Line numbers from the working tree at analysis time (2026-06-18); re-confirm before editing.
## Current-State Summary
The HA stack is mature and well-factored: per-LSN majority commit engine (`commit.c`), fan-out WAL replication with ACK/catch-up (`repl.c`, `config_repl.c`), a commit barrier that gates client replies (`persist_commit_barrier`), demotion truncate + rejoin reconciliation (`ha_convergence.c`), single-leader own-term gate, PreVote, and a vote-freshness gate. The durability gap is **beneath** all of it: "committed" (majority-ACKed) does not imply "durable on a majority", and recovery/promotion trust in-memory position rather than the durable log.
The submit reply path is already commit-gated and runs **off the iomgr thread** (`job.c:568-569` — "the AFTER_EVENT hook writes WAL and waits for majority replication"), so durability work can be added to that path without stalling heartbeats.
## Requirement-to-Asset Map
### Requirement 1 — Durable Commit (acknowledgment ⇒ majority persistence)
| Need | Existing asset | Status |
|------|----------------|--------|
| Reply gated on commit | `persist_commit_barrier` → `commit_submit(COMMIT_MODE_WAIT)` (persist.c); callers `job.c:1024`, `queue.c:144`, `worker.c:713`, `join.c:130` | ✅ Exists — reuse; strengthen meaning of "committed" |
| Standby persists before ACK | `persist_wal_write_replicated` `fdatasync` before return (persist.c:515) | ✅ Exists — **pin with regression test** (Req 1.3) |
| Primary self-counts only when durable | `commit.c:majority_candidate_locked` sets `cap = highest_appended_lsn(ce->p)` (appended, not durable) | ❌ **Missing** — core defect (bug-661) |
| Primary persists before self-count | `persist_wal_async` (persist.c:340) appends via `pwrite`, broadcasts, returns **without fsync**; durability deferred to `fsync_worker` timer (persist.c:173) | ❌ **Missing** |
| Track highest durable LSN | none — `persist_t` has `committed_lsn`/`current_lsn` but no `durable_lsn` | ❌ **Missing** (new field) |
| Liveness during durability work | commit-wait path already off iomgr (`job.c:568-569`) | ✅ Constraint satisfied by placement |
**Minimal change shape:** add `durable_lsn` to `persist_t`, advanced when the entry's bytes are `fdatasync`'d; `persist_wal_async` fsyncs (inline, synchronous design — see brief decision) and sets `durable_lsn` before returning; `majority_candidate_locked` uses `min(durable_lsn, …)` for the self contribution instead of `highest_appended_lsn`. Reply gate then enforces the invariant unchanged.
### Requirement 2 — Recovery Integrity + Leadership Fence
| Need | Existing asset | Status |
|------|----------------|--------|
| No silent truncate-to-zero on corrupt recovery | `replay_wal` breaks on CRC chain break / CRC mismatch / short read (persist.c:1021, 1050, 1042) and returns the replayed count as **success**; only `-ENOMEM` propagates (persist.c:1107, 1152) | ❌ **Missing** — silent truncate-accept is the amplifier |
| Durable floor re-derived on restart | `persist_recover` **never sets `committed_lsn`** — it stays 0 after restart; floor is reconstructed only at promotion (persist.c:1114) | ❌ **Missing** — restart advertises in-memory floor unbacked by WAL |
| Pre-promotion integrity / caught-up gate | `raft_cb_become_leader` (ha.c:551) computes `safe_lsn` from votes (ha.c:572) then `persist_truncate_wal_above(safe_lsn)` (ha.c:577) — **no check that local WAL is intact or ≥ safe_lsn** | ❌ **Missing** — hollow node can promote, truncate collapses tip without backfill |
| Freshness floor from durable WAL | `ha_vote_self_freshness` = `max(applied_lsn, last_known_committed_lsn)` — **in-memory atomics**, not re-derived from durable WAL (ha.c:2127; used cluster.c:380-396 vote, cluster.c:467-474 PreVote) | ❌ **Missing** — truncated candidate advertises a floor it can't back |
| Fence "ineligible until caught up" | `in_catch_up` atomic exists but only a **standby** uses it to deny its **own** outgoing vote (ha.c:325, cluster.c:529); no fence for a freshly-recovered/promoting node | ⚠️ Partial — extend concept |
| Re-eligibility on catch-up | standby catch-up apply `ha_apply_wal_catchup_entry` (ha.c:1192); re-inclusion exists | ✅ Exists — hook the fence release here |
### Requirement 3 — Deterministic Durability Verification
| Need | Existing asset | Status |
|------|----------------|--------|
| Configurable durability cadence / disable background flush | `wal_sync_ms` = `DEFAULT_WAL_SYNC_MS` (config.h:86, value 10); `persist_config_defaults` (persist.h:30); **no env/CLI/config override** — callers `myapp.c:164`, `cluster.c:1267` pass defaults unmutated | ❌ **Missing** — must add an override |
| Test-mode seam convention | `#ifdef XCE_TESTING` hook shims (ha.c:2270-2335), test-only `handle_request_vote` (ha.c:2161); cluster.c not linked under XCE_TESTING (cluster.c:67) | ✅ Pattern exists — follow it |
> Doc drift noted: `persistence-and-ha-design.md:4178` says `wal_sync_ms=50`; code is 10 (config.h:86). Reconcile in design.
## Implementation Approach Options
**Option A — Extend existing components (recommended).** All change sites are inside existing, well-scoped functions (`commit.c`, `persist.c`, `ha.c`, `cluster.c`). R1 = `durable_lsn` field + 2 edits (`persist_wal_async` fsync, `majority_candidate_locked` self cap). R2 = make `replay_wal` return an integrity verdict + a pre-promotion gate in `raft_cb_become_leader` + floor-from-durable-WAL in `ha_vote_self_freshness`. R3 = config override + an `XCE_TESTING` seam.
- ✅ Leverages the commit-barrier and `in_catch_up` machinery already present; smallest surface; matches TLA+ mapping in `CLAUDE.md`.
- ❌ Touches correctness-critical election/commit code — demands rigorous TDD and TLA+ re-verification.
**Option B — New durability module.** Extract a `durability`/`durable_lsn` tracker as a new component.
- ✅ Isolated unit testing. ❌ Over-engineered — `durable_lsn` is one atomic on `persist_t`; a new module adds indirection without separation benefit. Not recommended.
**Option C — Hybrid (recommended for R2 only).** Extend in place for R1/R3; add a small new **recovery-integrity + promotion-eligibility helper** (e.g. `persist_recovered_intact()` / `ha_promotion_durable_ok()`) for R2 so the integrity verdict and the fence predicate are unit-testable and don't bloat `raft_cb_become_leader`.
- ✅ Keeps the election hot path readable; testable fence predicate. Recommended: **A for R1+R3, C for R2.**
## Effort & Risk
| Req | Effort | Risk | Justification |
|-----|--------|------|---------------|
| R1 Durable commit | **M** (3–7d) | **Medium-High** | Localized (`durable_lsn` + 2 edits) but correctness-critical; needs TLA+ re-check (commit rule, leader completeness) and durability TDD. Liveness already safe (off-iomgr path). |
| R2 Recovery + fence | **L** (1–2wk) | **High** | Touches recovery, promotion, and the vote-freshness floor; must **compose with `ha-convergence-recovery`** (truncate/rejoin) and `ha-single-leader-role` (own-term gate) without double-handling, and honor `CLAUDE.md`'s "never truncate from stale ctrlhello" rule. Election-safety sensitive. |
| R3 Verification seam | **S** (1–3d) | **Low** | Config override + `XCE_TESTING` hook following an established pattern. |
## Recommendations for Design Phase
**Preferred approach:** Option A (extend) for R1 + R3; Option C (extend + small fence helper) for R2.
**Key design decisions to resolve:**
1. **Durability mechanism** — already decided (brief): synchronous persist-before-self-count, eliminate `fsync_worker` as the durability mechanism; group-commit batching deferred to roadmap. Design must confirm `fsync_worker`'s fate: delete entirely vs retain a best-effort flush for any non-client-acked / local-only write (audit `wal_classify` for writes that gate no reply; checkpoint markers already fsync inline at persist.c:573).
2. **`durable_lsn` semantics** — single atomic on `persist_t`; advanced under `p->lock` after `fdatasync`; consumed by `majority_candidate_locked`. Confirm interaction with `persist_truncate_wal_above` (must not let `durable_lsn` exceed a post-truncation tip).
3. **Recovery integrity verdict** — `replay_wal`/`persist_recover` must distinguish "clean EOF" from "stopped at corruption". Decide: FATAL on corruption (fail-loud, simplest, honors the user's fail-loud preference) vs flag-and-fence (recover what's intact, fence from leadership until caught up). The fail-loud preference (`[[feedback_fail_loud_over_timeout]]`) leans FATAL unless the node can provably catch up.
4. **Freshness floor source** — derive the candidate's advertised floor from the **durable WAL tip**, not in-memory `applied_lsn`/`last_known_committed_lsn` alone, so a truncated node cannot advertise an unbacked floor. Reconcile with `CLAUDE.md`: never truncate using a stale ctrlhello `last_known_committed_lsn`; the safe-commit point still comes from voter replies at promotion.
5. **Compose with convergence-recovery** — R2's pre-promotion fence must not duplicate `ha_convergence.c`'s demotion-truncate/rejoin logic; define the seam (this spec fences *promotion*; convergence handles *demotion* and *rejoin reconciliation* of an intact log).
**Research items to carry forward:**
- Measure `fdatasync` latency on the target storage vs the existing replication round-trip, to confirm the synchronous-design latency add is acceptable before group-commit is needed.
- Confirm no client-acked operation class bypasses the commit barrier (`wal_classify` audit) — any that do would violate the invariant.
- Decide the `wal_sync_ms` override surface (env var vs CLI flag vs config field) consistent with existing `--sync-timeout-ms` precedent (main.c `OPT_SYNC_TIMEOUT`).
- Re-verify against `raft/raft.tla` and the `CLAUDE.md` Protocol Verification Checklist after the self-count change (commit rule, leader completeness).
---
## Design Synthesis (2026-06-18)
Decisions resolved into `design.md`:
- **Durability mechanism = synchronous.** Add `persist_t.durable_lsn` (atomic, advanced after `fdatasync`); `persist_wal_async` persists before return; `commit.c:majority_candidate_locked` self-contribution uses `durable_lsn` not `highest_appended_lsn`. **`fsync_worker` removed** as the durability mechanism (checkpoint markers keep their inline fsync). Group-commit batching deferred to roadmap as a strict, contract-preserving optimization.
- **No new module (Option A) for R1/R3; small fence helper (Option C) for R2.** `durable_lsn` is one atomic — a module would be over-abstraction (build-vs-adopt: extend).
- **Recovery: fail-loud + fence.** `replay_wal` returns a verdict (`RECOVER_CLEAN` vs `RECOVER_TRUNCATED`); a truncated member is fenced from promotion and catches up from a healthy peer; FATAL only when it cannot reach a consistent committed state and is required for quorum. Balances `[[feedback_fail_loud_over_timeout]]` with availability.
- **Freshness floor from durable WAL**, not volatile in-memory `last_known_committed_lsn`; safe-commit point at promotion still from voter replies (honors `CLAUDE.md`).
- **Compose with convergence-recovery:** this spec fences *promotion* + verifies recovery integrity; convergence-recovery owns *demotion* truncate and *rejoin* of an intact log. No shared ownership of the commit index (kept volatile per Raft).
- **Liveness:** the added `fdatasync` lands on the already-off-iomgr commit path (`job.c:568-569`), so heartbeats are unaffected — Option A's only real risk is retired.
Generalizations/simplifications: reuse the existing `in_catch_up` fence concept (extend from streaming-standby to recovery-truncated promoter) rather than inventing a new fence; reuse `persist_commit_barrier` reply gate unchanged; follow `--sync-timeout-ms` and `XCE_TESTING` hook conventions for R3.
---
## Gap Re-Audit (2026-06-18)
> **Post-implementation gap audit.** All 17 tasks (1.1–5.3) + the never-self-exit
> rework are marked `[x]` (landed 2026-06-18). This pass re-verifies each EARS
> acceptance criterion against the on-disk code (codegraph + grep), tags each
> MET / Missing / Unknown / Constraint, and records the known residual
> test-thoroughness items called out in the Implementation Notes. Line numbers
> from the working tree at audit time; re-confirm before editing.
### Requirement-to-Asset Map (verified against code)
#### Requirement 1 — Durable Commit (acknowledgment ⇒ majority persistence)
| Crit | Asset (file:line) | Tag |
|------|-------------------|-----|
| 1.1 ack ⇒ durable on majority | Reply gated on commit barrier (unchanged) + self durable-capped (`commit.c:93`) + standby persist-before-ACK (`persist.c:761`); end-to-end test `test_durable_majority_commit_end_to_end` | **MET** |
| 1.2 self counts only when durable | `majority_candidate_locked` self-cap `cap = persist_durable_lsn(ce->p)` (`commit.c:93`, was `highest_appended_lsn`); debug invariant `assert(cand <= persist_durable_lsn)` (`commit.c:162`) | **MET** |
| 1.3 standby persists before ACK | `persist_wal_write_replicated` inline `fdatasync` rc-gated, `return -EIO` on failure so the standby does NOT ACK (`persist.c:761-773`); caller `ha_apply_single_entry` ACKs only on rc==0 (no-apply/no-ACK on negative). Pinned by `test_replicated_persist_syncs_before_return` / `test_replicated_sync_failure_no_ack` (counter `test_replicated_fdatasync_count`) | **MET** |
| 1.4 not reported until durable-majority | committed_lsn cannot advance over a non-durable entry (1.2 cap) → existing WAIT-mode reply gate enforces it unchanged | **MET** |
| 1.5 minority loss survives | Property test `test_sole_durable_contributor_loss_survives` (committed ⇒ ≥⌊N/2⌋+1 durable copies, drop one); live failover proof via soak (5.2) | **MET** (live N=2 fault is decision/property-level — see residual G1) |
| 1.6 liveness during durability work | Group-commit flusher (`fsync_worker`) syncs OFF the iomgr/caller thread; `fdatasync` under `p->lock` only (never with `fsync_mutex`); `test_durability_work_off_iomgr_preserves_liveness` asserts flush on a non-caller thread + heartbeat reads <50ms | **MET** |
Mechanism note: `persist_wal_async` no longer fsyncs inline (`persist.c:643-652` signals the flusher); durability watermark `persist_t.durable_lsn` (`_Atomic uint64_t`) advanced ONLY after a successful `fdatasync` (flusher `persist.c:281-311`, `persist_flush` `persist.c:801-806`). `fsync_worker` repurposed from fixed-interval timer to demand-driven group-commit flusher; checkpoint markers retain their inline `fdatasync` (`persist.c:162`). Legacy `sync_append` ACK-commit store now durable-gated (`sync_append_legacy_commit_store`, store only when `written_lsn <= durable_lsn`).
#### Requirement 2 — Recovery Integrity + Leadership Fence
| Crit | Asset (file:line) | Tag |
|------|-------------------|-----|
| 2.1 no silent truncate-accept | `replay_wal(p, from_lsn, recover_verdict_t*)` classifies CRC chain break / CRC mismatch / short header / short payload → `RECOVER_TRUNCATED`; clean EOF / pre-alloc zero tail / empty WAL → `RECOVER_CLEAN` (`persist.c:1411-1605`); verdict stored on `persist_t.recovery_verdict`, accessors `persist_recovery_verdict`/`persist_recovery_truncated` (production, not XCE_TESTING-gated) | **MET** |
| 2.2 ineligible until caught up | Pre-promotion gate in `raft_cb_become_leader` (`ha.c:952-1010`): REFUSE if `persist_recovery_truncated` OR `durable_floor < promotion_safe_lsn` (read AFTER truncate-clamp), set `in_catch_up`, return WITHOUT becoming primary | **MET** |
| 2.3 below-HWM never authoritative | Durable-backed freshness floor: candidate advertises bare `ha_candidate_advertise_lsn = persist_durable_floor` (`ha.c:2587`), voter floor `max(durable_floor, learned)`; promotion gate compares clamped `durable_floor` vs voter-reply `promotion_safe_lsn`; floor re-derived from recovered WAL tip in `persist_recover` (assign, not max) | **MET** |
| 2.4 loud alert, hold non-promotable, STAY ALIVE, never self-exit | `persist_recover_loud_fence_if_unrecoverable` (`persist.c:1787`): loud ERROR with truncation boundary + sticky `_Atomic bool recovery_unrecoverable`, **NO abort/exit/tlog_exit**; `RECOVER_DECISION_FENCE_UNRECOVERABLE` (non-terminating). grep confirms zero abort/exit on the recovery path; `persist_fatal_hook` removed from production (only a test comment remains) | **MET** |
| 2.5 preserve durable-commit at degraded/minimal quorum | Hollow node refused regardless of quorum size (gate is unconditional on truncated/behind) + 4.2 freshness floor; `test_promotion_gate_hollow_refused_at_minimal_quorum` (decision-level, Req 2.5) | **MET** (decision-level; no dedicated live N=2 fault-injection — residual G1) |
| 2.6 re-eligibility on catch-up | Fence/flag clear automatically when `!truncated && durable_floor >= HWM`: `ha_evaluate_promotion_fence` ELIGIBLE branch clears `in_catch_up` (`ha.c:592-597`); `promotion_fence_timeout_fn` releases + `persist_clear_recovery_unrecoverable` on catch-up (`ha.c:755-763`); gate ELIGIBLE branch clears + cancels timer (`ha.c:986-993`) | **MET** |
Bounded-window catch-up (task 4.4): `raft_cb_become_leader` NEVER FATALs/aborts synchronously — on FENCE/would-be-unrecoverable it arms a one-shot `promotion_fence_timeout_fn` (`DEFAULT_RECOVERY_CATCHUP_WINDOW_MS=30000`) that re-reads live state, releases on catch-up, re-emits the loud alert + re-arms if still hollow with no peer — never exits (`ha.c:721-807`). UAF-safe cancel; cancelled in `ha_fini`.
#### Requirement 3 — Deterministic Durability Verification
| Crit | Asset (file:line) | Tag |
|------|-------------------|-----|
| 3.1 configurable durability cadence | `--wal-sync-ms` flag (`main.c:67,118`) → `myconf.wal_sync_ms` → `persist_config_set_wal_sync_ms` (`persist.h:130`) applied at BOTH persist_init sites (`myapp.c:169` production, `cluster.c:1284` phase2); 0-sentinel = keep `DEFAULT_WAL_SYNC_MS`. Test seam `persist_test_durable_lsn_on_disk` (read-only on-disk CRC-validated durable LSN) + `persist_test_set_force_fsync_fail` fault injection, all XCE_TESTING-gated (`persist.h:445-506`) | **MET** |
| 3.2 ack ⇒ recoverable after ungraceful kill in verify-mode | Deterministic E2E (`test-commit-durability.sh --deterministic`): with `--wal-sync-ms 30000`, in-window primary `fdatasync >= 1` (=2) precedes ack ("durable-before-ack"), flipped RED→GREEN stable 3/3 / 9-9 probes (5.2) | **MET** |
| 3.3 same observable ack semantics with/without verify-mode | Documented as a property in the test header (`test_commit_durability.c:33-34`): verify-mode "changes nothing observable about the acknowledgment contract" — but there is **NO dedicated A/B assertion** running the same op with mode on vs off and asserting identical observable acknowledgment | **PARTIAL** (residual G2) |
Constraint compliance: `safe_committed_lsn` arithmetic UNTOUCHED (voter-reply-sourced, honors `CLAUDE.md` "never truncate from stale ctrlhello"). raft.tla election/freshness logic maps faithfully; raft.tla does **NOT** model durability (restores `log` intact on restart, `raft.tla:165-174`) — the commit-durability invariant is verified by tests 2.x/5.1/5.2 + the manual Protocol Verification Checklist, NOT by raft.tla. (Tagged **Constraint** — by-design deferral, see residual G3.)
### Residual Gaps
| ID | Gap | Severity | Notes |
|----|-----|----------|-------|
| **G1** | Req 2.5 has no dedicated **minimal-quorum live fault-injection** test (N=2, lose the sole durable contributor across a real failover). Current coverage = decision/property-level (`test_promotion_gate_hollow_refused_at_minimal_quorum`, `test_sole_durable_contributor_loss_survives`) + the rolling-SIGKILL soak. | **LOW** | Invariant is enforced by the unconditional gate + freshness floor and the soak observed zero loss; a targeted N=2 case would harden the assertion against a future regression but is not a correctness gap. |
| **G2** | Req 3.3 has no **verify-mode A/B observable-equivalence** assertion (same op, mode on vs off, identical externally observable acknowledgment). Property is asserted only by header comment + the structural fact that the seam is read-only / production-excluded. | **LOW** | The seam (`persist_test_*`, `--wal-sync-ms`) demonstrably changes only timing, not the reply path (reply gate unchanged). An explicit A/B test would close the letter of 3.3. |
| **G3** | raft.tla does **not** model durable-vs-volatile log; the formal model cannot prove the commit-durability invariant. | **LOW (by design)** | Explicitly deferred (task 5.3 note, requirements out-of-scope). Durability is test-backed. A durable-log TLA+ extension is out of scope unless explicitly opted in. |
No CRITICAL/HIGH residuals. No criterion is **Missing** or **Unknown**: 16/16 verifiable EARS criteria are MET (1.1–1.6, 2.1–2.6, 3.1–3.2) with 3.3 PARTIAL (property held but not A/B-asserted). The never-self-exit invariant (Req 2.4) is upheld — grep/nm confirm no abort/exit/tlog_exit on the recovery/promotion path.
### Integration Risks
- **Group-commit flusher latency on a degrading disk** (Implementation Note 2.1 follow-up): the flusher tight-spins on SUSTAINED `fdatasync` failure (releases `p->lock` each iter so liveness holds, but CPU/log burn). A HANGING (not erroring) `fdatasync` blocks appends since it runs under `p->lock` — pre-existing, not introduced here. Non-blocking; carry to roadmap with group-commit batching.
- **Test-fixture durable priming (bug-704 class)**: any commit-engine / election fixture that primes `current_lsn`/`applied_lsn`/`last_known_committed_lsn` but NOT `durable_lsn` will see floor/advertise=0 and committed-stall, because both the self-cap (2.2) and the freshness floor (4.2) now read durable state. Future tests MUST prime `durable_lsn` to the modeled tip. (4 fixtures already corrected; bug-760 caught one adjacent real-flusher test.)
- **fsync-able filesystem for validation**: `/tmp` overlay in some sandboxes EIOs on `fdatasync`; because committed_lsn is now durable-gated, an EIO correctly HOLDS committed_lsn and surfaces as adjacent-test failures. Final validation MUST run on a real fsync-able fs. Gate on ANSI-stripped `:FAIL:` count (meson `Fail:0` masks Unity sub-group failures).
### Options for the Open Test-Thoroughness Items (G1, G2)
**Option A — Close both with targeted unit/integration tests (recommended).**
- G1: add an N=2 minimal-quorum integration test reusing the `test_sole_durable_contributor_loss_survives` two-persist-dir harness, but drive it through a real failover-style commit + drop-the-durable-contributor and assert the acked op is present on the survivor.
- G2: add an A/B test that submits the same op with verify-mode (`--wal-sync-ms` slow / forced) and without, asserting byte-identical externally observable acknowledgment (same rc, same reply gate release semantics).
- **Effort: S (1–3d).** **Risk: Low** — test-only, no production change; reuses existing harnesses/seams.
**Option B — Cover via the soak/E2E harness only.**
- Extend `test-commit-durability.sh` with an N=2 deterministic round (G1) and a mode-on/mode-off ack-equivalence probe (G2).
- **Effort: S (1–2d).** **Risk: Low-Medium** — E2E is less deterministic than unit; the strace-gap probe is the established deterministic surface, so prefer it over joblist-loss for G2.
**Option C — Accept as documented residuals (status quo).**
- Record G1/G2 as LOW-severity known limitations; rely on the gate/floor invariants + the zero-loss soak already in place.
- **Effort: none.** **Risk: Low** — acceptable given no CRITICAL/HIGH gap, but leaves the letter of Req 3.3 (A/B) and a dedicated Req 2.5 live case unasserted, so a future regression in those exact paths could go uncaught until soak.
**Recommendation:** Option A — both items are S-effort, test-only, and turn two LOW residuals into fully-asserted criteria; G3 stays a by-design deferral.
---
## REOPENED 2026-06-19 — cold-restart-all (total-cluster-outage) committed-job loss (bug-795)
**Trigger:** the `xce-test-e2e-primitives` spec converted `test-commit-durability.sh` to exit-code primitives and ADDED a new `run_cold_restart_recovery` phase (kill ALL 3 controllers simultaneously, then restart ALL from disk, then assert every acknowledged job recovers). Running it live on a 3-node cluster surfaced a NON-DETERMINISTIC committed-job loss that this spec's original validation never exercised.
**What was observed (3 live soak runs, sandbox 3-node cluster):**
- Rolling **single-node** SIGKILL + restart: ZERO loss every round (consistent with this spec's 5.2 result — single-host loss is non-deterministic and standbys fdatasync-before-WalAck, so a committed job is durable off the primary).
- **Cold-restart-all** (kill ALL 3, restart ALL): a job that returned `jobsubmit ok` AND survived every rolling round VANISHED after the full restart (`expect-jobs expected N, actual N-2`; `expect-job <id> not found`). PASSED in one run, FAILED in another → a **race**. Recovery logs show WAL `truncate` on the rejoining nodes.
**Why this spec did not catch it:** 5.2's soak was rolling **single-node** kills only; the durability guarantee was validated (and CLAUDE.md flipped to "GUARANTEED") against that scenario. The **total simultaneous outage** path — where ALL members lose their volatile/page-cache tail at once and recovery must reconstruct committed state from on-disk WAL alone — was never tested. When every node dies together, the "durable off the primary" assumption that makes single-host loss non-deterministic no longer applies: only what was `fdatasync`'d on a majority before the ack can survive.
**Open question — REAL durability gap vs OVERLAYFS artifact (must resolve first):**
- This sandbox's `/tmp` is **overlay** (NOT a real fsync-able fs). This spec's own notes repeatedly flag that overlay `/tmp` can EIO on `fdatasync` and that "final validation must run on a real fsync-able fs."
- If `fdatasync` on overlay is weak (returns rc=0 without true durability) OR if recovery's truncation (`replay_wal` / `persist_truncate_wal_above` / durable-floor re-derivation, tasks 3.1/3.4) is too aggressive after a total outage, a committed entry's tail could be lost or truncated on cold restart.
- The durable-gating (2.2/2.4) should make `committed_lsn` HOLD if `fdatasync` failed — so an un-fdatasync'd job should NOT have returned `jobsubmit ok`. Yet these jobs DID get `jobsubmit ok`. So either `fdatasync` returned success on overlay without real durability, or a non-durable-gated ack path exists under the total-outage timing, or recovery truncates committed entries.
**Disposition:** the `xce-test-e2e-primitives` soak is the reproduction surface and is working correctly (it detects the loss via exit codes). The fix (if real) belongs HERE. See Phase 6.
### UPDATE 2026-06-19b — trustworthy soak sharpens the finding (rolling-failover loss + liveness)
After the `xce-test-e2e-primitives` soak was HARDENED (commit d82cc78: it no longer conflates liveness with durability — durability is asserted ONLY against a confirmed-serving cluster, with a catch-up-lag confirm re-check; a non-serving cluster is INCONCLUSIVE, never "DATA LOSS"), three 6–10-round live runs give a cleaner signal:
1. **The committed-job loss reproduces in ROLLING single-node failover too — not only cold-restart-all.** One run lost a committed job at round 6 with the cluster CONFIRMED SERVING (`DATA LOSS — persistently absent from a SERVING cluster`); two other runs had zero loss. So the durability race is NON-DETERMINISTIC (~1 in 3 runs here) and broader than the cold-restart-all framing in the original reopen note — it is a general acked-but-not-durable-on-majority race that a kill can expose during normal rolling failover. **This is exactly the bug-661 invariant this spec claimed CLOSED for rolling failover (task 5.2) — it is not fully closed.** (Caveat still applies: sandbox `/tmp` is overlayfs; 6.1 must confirm on a real fsync-able fs.)
2. **Separate LIVENESS concern:** every run shows 2–3 rounds where, after killing the current primary (consistently `ctrl-1`), NO new primary served within 60s (the cluster always eventually recovered). A >60s failover for a single-node kill in a 3-node cluster is poor availability and may share a root cause with the durability race (a behind/just-rejoined node interacting with the promotion fence / freshness floor). Worth a dedicated liveness check in 6.2.
**Phase 6 scope note:** 6.1's reproduction MUST include BOTH the cold-restart-all step AND the rolling-failover rounds (the hardened soak runs both and reports trustworthy per-round verdicts), and should capture the >60s-election cases for the liveness angle.
### UPDATE 2026-06-19c — 6.1 ANSWERED: the loss is REAL, not (only) an overlay artifact
The cold-restart-all / rolling durability loss was initially suspected to be an overlay-`/tmp` `fdatasync`-EIO artifact of the dev sandbox. That is now ruled out as the primary cause:
- **User runs the soak OUTSIDE the container** (real host, real fsync-able fs, NO overlay `fdatasync` EIO) and still observes the DATA LOSS + the liveness failures. So they are genuine, not sandbox `fdatasync` noise.
- In-sandbox confirmation on a **real ext2/3 deploy dir** (`fdatasync` succeeds, ZERO EIO): a 3-round run PASSED clean, but an 8-round run reproduced BOTH a liveness cascade (after killing `ctrl-1` in round 1 the cluster never returned to 3-healthy) AND a cold-restart `expect-jobs` shortfall with a PERSISTENTLY-absent committed job — with **zero `fdatasync` EIO**. (Caveat: the 8-round run's cold-restart followed a degraded run, so the cleanest real-fs evidence is the user's outside-container host.)
**Verdict for 6.1:** the durability loss and the liveness stall reproduce off overlay → REAL controller findings, not an `fdatasync`-EIO artifact. The overlay EIO remains a SEPARATE, sandbox-only confound (it makes the dev-box `controller_unit_test` flaky — see bug-760 — and inflates loss on overlay deploys), but it is not the root cause of the soak loss.
**Phase 6 is now at 6.2 (root-cause).** Two real, non-deterministic findings to root-cause:
- **Durability:** an acked (`jobsubmit ok`) job is PERSISTENTLY absent from a SERVING cluster after failover / cold restart — i.e. bug-661 is NOT fully closed (Phase 5 only exercised rolling single-node kills with the racy soak). Buckets: acked-without-durable-majority (commit-gate timing) / recovery truncates committed entries / behind-then-killed node re-leads with a truncated log.
- **Liveness:** after killing the current primary (consistently `ctrl-1` in these runs), the cluster sometimes takes >60s — or fails entirely — to elect a serving primary, and a restarted node sometimes never returns to a healthy 3-node serving set. Possibly coupled to the promotion-fence / freshness-floor interaction with a just-rejoined node.
The hardened `xce-test-e2e-primitives` soak (trustworthy: liveness vs durability separated, confirm-settle, quiet wait-primary) is the reproduction surface for both.