spec(ha-commit-durability): REOPEN for bug-795 total-outage cold-restart-all committed-job loss
Phase 6 added (6.1 reproduce-on-real-fs gates 6.2 root-cause / 6.3 fix / 6.4 revalidate). Surfaced by the xce-test-e2e-primitives cold-restart-all soak step; Phase 5 only validated rolling single-node failover. CLAUDE.md durability claim qualified pending 6.x.
This commit is contained in:
parent
aa615a184a
commit
3ad646ad0e
4 changed files with 249 additions and 1 deletions
|
|
@ -126,6 +126,32 @@
|
|||
- _Requirements: 1.2, 2.3_
|
||||
- _Depends: 5.1_
|
||||
|
||||
- [ ] 6. REOPENED 2026-06-19 — total-cluster-outage (cold-restart-all) committed-job loss (bug-795)
|
||||
> Surfaced by the `xce-test-e2e-primitives` soak's new cold-restart-all step (kill ALL 3 + restart ALL). Phase 5 only validated rolling SINGLE-node failover; the total simultaneous outage path was never tested. Sequential by design (each task gates the next). DO NOT start a fix before 6.1 resolves real-vs-artifact.
|
||||
- [ ] 6.1 Reproduce cold-restart-all loss on a REAL fsync-able fs (rule out the overlayfs artifact)
|
||||
- Run the converted `test-commit-durability.sh` (incl. its `run_cold_restart_recovery` phase) against a live 3-node cluster on a REAL fsync-able filesystem (NOT container overlay `/tmp`) — e.g. a tmpfs-free ext4/xfs deploy dir. Repeat enough rounds to characterize the race (it is non-deterministic: ~1-in-2 in sandbox runs). Capture controller recovery logs (WAL `truncate`, durable-floor, `committed_lsn`) for a losing run.
|
||||
- Observable: a definitive verdict — EITHER the loss reproduces on a real fs (⇒ genuine total-outage durability gap, proceed to 6.2) OR it does NOT reproduce on a real fs and only occurs on overlay `fdatasync` (⇒ overlayfs artifact; record as an environment limitation, close Phase 6 with a note, and the `xce-test-e2e-primitives` task 7.2 unblocks on a real-fs run).
|
||||
- _Requirements: 1.1, 1.5_
|
||||
- _Boundary: ValidationOnly (no production change) — `src/tools/test-commit-durability.sh` is owned by xce-test-e2e-primitives; consume it as-is._
|
||||
- [ ] 6.2 Root-cause the total-outage recovery loss (conditional on 6.1 = REAL)
|
||||
- Instrument the recovery path for a simultaneous all-node SIGKILL + restart: determine whether (a) a job returned `jobsubmit ok` without being `fdatasync`'d on a fault-tolerant majority (a Req-1 commit-gate hole specific to the total-outage timing), (b) recovery (`replay_wal` / `persist_truncate_wal_above` / durable-floor re-derivation, tasks 3.1/3.4) truncates committed entries on cold restart, or (c) a just-rejoined-then-immediately-cold-killed BEHIND node re-leads with a truncated log and overwrites the good timeline (a promotion-fence/freshness-floor hole under cold restart, tasks 4.2/4.3).
|
||||
- Observable: the exact mechanism is identified with a captured trace; classified as commit-gate / recovery-truncate / promotion-fence; mapped to the specific Requirement(s) and code path it violates.
|
||||
- _Requirements: 1.1, 1.4, 1.5, 2.3, 2.6_
|
||||
- _Boundary: RecoveryIntegrity / DurableCommit (controller persist/commit/ha)_
|
||||
- _Depends: 6.1_
|
||||
- [ ] 6.3 Fix the total-outage durability/recovery path + add a deterministic regression (conditional on 6.2)
|
||||
- Repair the identified mechanism so an acknowledged job survives a simultaneous all-node kill+restart, honoring the never-self-terminate invariant (loud fence, stay alive, keep retrying — see `src/controller/CLAUDE.md` Process Lifecycle). Add a deterministic cold-restart-all regression (prefer a unit/integration harness over the racy soak; the soak's `run_cold_restart_recovery` step stays as the E2E backstop).
|
||||
- Observable: the cold-restart-all loss no longer reproduces (real fs); the new regression is RED on the unfixed build and GREEN after; the full controller suite passes under ASAN with `:FAIL:`==0.
|
||||
- _Requirements: 1.1, 1.4, 1.5, 2.3, 2.6_
|
||||
- _Boundary: DurableCommit / RecoveryIntegrity / PromotionFence_
|
||||
- _Depends: 6.2_
|
||||
- [ ] 6.4 Re-validate and reconcile the durability claim
|
||||
- Re-run the deterministic regression + the cold-restart-all soak on a real fs (GREEN); reconcile `src/controller/CLAUDE.md` "Commit Durability — GUARANTEED" to state the total-outage scenario is now covered (or, if 6.1 found an artifact, qualify the overlay-fs caveat). Unblock `xce-test-e2e-primitives` task 7.2 and re-run its soak to close it.
|
||||
- Observable: durability guarantee holds for BOTH rolling single-node failover AND total-cluster-outage cold restart, backed by the new regression; CLAUDE.md is accurate; bug-795 closed.
|
||||
- _Requirements: 1.1, 1.2_
|
||||
- _Boundary: DurableCommit + docs_
|
||||
- _Depends: 6.3_
|
||||
|
||||
## Implementation Notes
|
||||
- 1.1: Config plumbing routes through `myconf_t.wal_sync_ms` + `persist_config_set_wal_sync_ms()` (0-sentinel = keep `DEFAULT_WAL_SYNC_MS`), NOT a new config.h constant (design File Structure Plan keeps `DEFAULT_WAL_SYNC_MS`). The override is applied at BOTH `persist_init` sites — `myapp_run` step-6 (production path) and `cluster_init_phase2` (guarded by `wal_path[0]=='\0'`, skipped in production). They are mutually exclusive; apply any new persist_config override at both sites or it is silently dropped. Pre-existing latent bug (do NOT fix here): `--sync-timeout-ms` is applied only in phase2, so it is ignored on the production startup path — relevant if a later task relies on `sync_timeout_ms`.
|
||||
- 1.2: Test seam = `persist_test_durable_lsn_on_disk(persist_t*)` (decl `persist.h`, impl `persist.c`, all `#ifdef XCE_TESTING`) — re-opens WAL segment files from disk and CRC-validates the chain (read-only, state-free) to return the highest fully-persisted LSN; new test `testsuite/test_commit_durability.c` (registered in `meson.build` + `controller_test.c`). Production `xce_controller` links without it (proven via `nm`). DRY NOTE: ~115 lines of segment-scan/CRC-chain logic are duplicated from the static `replay_wal`; tasks 3.1 (recovery verdict) and 3.4 (durable floor from WAL tip) touch the SAME scan/CRC logic — consider extracting a shared read-only `wal_scan_durable_lsn`-style helper there and routing this seam through it (avoid a third copy). `XCE_TESTING` is defined only for `controller_unit_test` (`src/controller/meson.build:167`).
|
||||
|
|
@ -146,3 +172,4 @@
|
|||
- REWORK (2026-06-18, user invariant — a controller MUST NEVER self-terminate): tasks 3.3/4.3/4.4 had implemented Req 2.4 "fail loud" as a SELF-ABORT (`persist_recover_fatal_if_unrecoverable → tlog_exit(); abort()` + `persist_fatal_hook`). That violated the invariant (a self-exited controller needs MANUAL admin recovery + leaves quorum degraded; a live retrying node self-heals when a peer returns) and was the true root cause of the 5.2 soak quorum collapse. CORRECTED: removed the abort/exit terminal entirely. New: `persist_recover_loud_fence_if_unrecoverable(persist_t*)` (NO abort) logs a loud operator-visible ERROR (truncation boundary vs cluster HWM) + sets sticky `_Atomic bool recovery_unrecoverable` (accessors `persist_recovery_unrecoverable`/`persist_clear_recovery_unrecoverable`); the member STAYS ALIVE, permanently non-promotable/fenced (`in_catch_up` + the sticky flag both consulted by the promotion gate), and KEEPS RETRYING catch-up. Decision enum `RECOVER_DECISION_FATAL` → `RECOVER_DECISION_FENCE_UNRECOVERABLE` (non-terminating). 4.4 timer no longer aborts — it re-emits the loud operator alert + re-arms (operator visibility), never exits. Flag + fence CLEAR automatically on catch-up (not truncated AND durable_floor ≥ HWM) → re-eligible (Req 2.6), fully automatic. `persist_fatal_hook` removed. Verified: grep/nm show NO abort/exit/tlog_exit on the recovery path; ASAN suite :FAIL:==0; reworked tests (row4/4b → loud_fence, unrecoverable-stays-alive-loud, released-on-catchup, cancel-stops-realert) PASS. Spec updated: requirements.md Req 2.4 + design.md (decision table rows 4/4b, RecoveryIntegrity, traceability, FATAL-vs-fence open question) reworded "FATAL/terminate" → "LOUD PERMANENT FENCE (non-promotable, stays alive, keeps retrying — NEVER self-exit)". PROJECT-WIDE GUIDANCE added: `src/controller/CLAUDE.md` §"Process Lifecycle (DESIGN INVARIANT)" + `.kiro/steering/ha-async-io.md` — controllers never self-terminate. (`ha_self_restart` execv-restart in convergence/demotion is a SEPARATE primitive, untouched.)
|
||||
- 5.2: PASS on the reworked (no-self-exit) build. (a) DETERMINISTIC E2E flipped RED→GREEN, stable 3/3 runs / 9-9 probes — in-window primary fdatasync ≥1 (=2) precedes the ack ("durable-before-ack"), the inverse of 1.3's unfixed RED. (b) ROLLING SIGKILL soak (6 rounds) exit 0 = ZERO acknowledged-job loss, cluster stayed continuously available, 0 self-terminations (the no-self-exit rework removed the abort that collapsed quorum in the first attempt). Validation-only (no production code). NOTE: ran in-sandbox via background+poll on a worktree deploy dir (avoids /tmp overlay EIO); the earlier collapse was the abort terminal, now corrected — a host re-run is still good practice but the in-sandbox soak now passes with the fix.
|
||||
- 5.3: Formal re-verification (conceptual — no TLA+ toolchain in env; project practice = manual alignment + the CLAUDE.md Protocol Verification Checklist). Checklist walked against the integrated impl, all hold: votedFor/currentTerm (untouched); `commitIndex`/`committed_lsn` monotonic + never exceeds `durable_lsn` (2.2/2.4 debug-assert); leader commits via per-entry majority ack (unchanged); `safe_committed_lsn` from voter replies (untouched, 4.2/4.3); WAL truncation only above the safe point + `durable_lsn` clamped on truncate; freshness gate term-then-index with durable-backed LSN (4.1 pinned vs raft.tla `mlastLogIndex`/`LastTerm`; candidate bare `durable_floor`, voter floor `max(durable_floor, learned)`); contiguous-prefix commit; `durable_lsn` prefix watermark; causal-order append; and the NEW item — a member counts toward the commit majority only at its durable LSN, acknowledged⇒durable-on-majority — now TRUE. raft.tla alignment: the election/freshness logic maps faithfully; raft.tla does NOT model durability (assumes a durable log, restores `log` intact on restart `raft.tla:165-174`) so it CANNOT prove the commit-durability invariant — that invariant is backed by tests 2.x/5.1 + the deterministic RED→GREEN E2E + the zero-loss/available soak (5.2), NOT by raft.tla. `src/controller/CLAUDE.md`: "Commit Durability — KNOWN GAP (bug-661)" flipped to "GUARANTEED / CLOSED (landed 2026-06-18)" with the mechanism list (incl. the no-self-exit loud fence); the post-`ha-commit-durability` checklist item ticked `[x]`. ALL 17 tasks (1.1–5.3) + the no-self-exit rework COMPLETE.
|
||||
- VALIDATION FIX (2026-06-18, `/kiro-validate-impl` finding bug-760): the feature-level validation caught a deterministic 6/6 FAIL of the ADJACENT-spec test `test_fact_hook_append_is_retained_and_nonblocking` (`test_commit_barrier_rollout.c:446`, "Expected 1 Was 0"). ROOT CAUSE = task 2.2's durable-gating: committed_lsn now requires the async group-commit flusher to advance durable_lsn, but this real-persist test checked committed_lsn SYNCHRONOUSLY the instant after the covering ACK (before the flusher fdatasync'd). Task 2.2 had primed durable_lsn in the in-memory `barrier_setup_vcpus` stubs but MISSED this real-flusher test. CORRECTION of the earlier impl-notes (3.1/5.1) claim that this was a pre-existing `/tmp`-overlay fdatasync-EIO artifact: that is FALSE in this env — fdatasync returns rc=0 on /tmp here and the test passed pre-spec; it was a genuine durable-gating regression. FIX: insert `persist_flush(g_fp)` + assert `persist_durable_lsn(g_fp) >= fact_lsn` BEFORE `repl_on_ack` (commit_on_ack runs synchronously inside repl_on_ack, so the now-durable self completes the majority deterministically). Production was never affected (RETAIN facts have no client waiter; WAIT-mode replies block on the commit barrier `commit.c:334`). Verified: full controller suite 0 failures across 4 runs + meson `Ok:1 Fail:0`. GATING NOTE: meson `Fail:0` masks Unity sub-group failures — gate on the ANSI-stripped `:FAIL:` count from the test binary stdout.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue