myxce/.kiro/steering/persistence.md
Rongsong Shen d32f06e5fc
docs: formalize save/restore field-completeness as a standing invariant
Prompted by rediscovering the same bug class in two places this
session: the controller's WAL already has this documented for one
mechanism (persistence.md's APPLY_CLEAN/bug-1678 section, xce-audit-
invariants controller dimension 7) but the scheduler's separate
snapshot save/load mechanism had the identical gap (job->allocs, this
session's scheduler-snapshot-job-allocation brief) with no equivalent
tracking. Closes that gap and makes the invariant self-reinforcing
going forward rather than something that has to be independently
rediscovered per persistence mechanism:

- xce-audit-invariants: new scheduler dimension 4 ("Field-complete
  snapshot save/load parity"), sibling of controller dimension 7,
  scoped to snapshot.c/snapshot_wire.c/snapshot_load.c.
- persistence.md: new "Snapshot Save/Load Field-Completeness
  (Scheduler)" section cross-referencing the WAL-side bug-1678 writeup
  and the new scheduler dimension.
- CLAUDE.md: new top-level Key Rules bullet stating the invariant
  plainly, plus adding persistence.md to the steering-file pointer
  table (it was missing).
- kiro-spec-design's design-review-gate.md (the mandatory pre-write
  gate) and kiro-validate-design's design-review.md (the optional deep
  review): new conditional review sections -- applies only when a
  design touches a save/restore mechanism, requires enumerating every
  persisted field's capture status rather than trusting an aggregate
  "state looks right" check. Catches this class of gap at design time,
  before implementation, rather than only via a later audit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
2026-08-27 08:59:48 +00:00

15 KiB

WAL & Persistence Durability

How the controller turns appended state into durable, recoverable, majority-backed state. This is the persistence-layer companion to ha-async-io.md (HA protocol / replication) and the src/controller/CLAUDE.md Protocol Verification Checklist (the authoritative safety gate). Owning code: src/controller/persist.c, src/include/persist.h, src/controller/commit.c. Landed by spec ha-commit-durability (2026-06-18, closes bug-661).

Core Invariant: Acknowledged ⇒ Durable on a Majority

When the controller replies ok to a client, the operation's WAL entry is fdatasync'd on a fault-tolerant majority (the acking member + ⌊N/2⌋ others). The reply path is unchanged — it is gated on the commit barrier; the durability guarantee comes from strengthening what "committed" means, not from a new gate.

The load-bearing rule: a member contributes to the commit majority for an LSN only once that LSN is durable locally. The primary counts its own copy at durable_lsn, never at the appended tip.

durable_lsn — The Prefix Watermark

persist_t.durable_lsn (_Atomic uint64_t) is the highest LSN whose bytes are fdatasync'd locally. It is a prefix watermark, never a per-entry set: durable_lsn ≥ L implies every entry ≤ L is durable (because the WAL is a contiguous single-writer log). Invariants:

  • durable_lsn <= current_lsn - 1 (you cannot be durable past what you have appended).
  • Monotonic non-decreasing except clamp-on-truncate (persist_truncate_wal_above clamps durable_lsn down to the new tip). Because the clamp lowers durable_lsn, a truncate cutoff that lags the true commit point physically destroys committed entries — only ever call persist_truncate_wal_above with the votes-derived post-election safe_committed_lsn or the library (term,lsn) conflict point, NEVER with the lagging last_known_committed_lsn (bug-828).
  • Advances only after a successful fdatasync — fail-loud: on fdatasync error the watermark holds and an ERROR is logged. Never advance a watermark on a failed sync.

Read it via persist_durable_lsn(p). The commit engine's self-cap (majority_candidate_locked in commit.c) uses exactly this value.

Group-Commit Flusher (the fsync_worker pattern)

fsync_worker is demand-driven, not a timer. On append, persist_wal_async pwrites under p->lock, releases, then signals the flusher. The flusher does one fdatasync covering all pending appends, advances durable_lsn, then wakes commit evaluation. This amortizes one sync across the high-volume WAL_MODE_RETAIN fact stream (job start/finish/fail/rusage).

Two correctness rules when touching this path:

  • Lost-wakeup guard is level-triggered on the appended-vs-synced gap (durable_lsn < current_lsn - 1), re-checked under p->lock before waiting and after each flush — never edge-triggered. An entry must never be considered durable before its fdatasync.
  • A sustained fdatasync failure must still back off (bug-1563). The level-triggered gap above can't distinguish "fresh work just landed" from "still stuck from the last failed attempt" — both read as pending. fsync_worker skips its wait ONLY when the just-completed flush made genuine progress (succeeded) or there was nothing pending to begin with; a flush that WAS pending but made no progress (failed) still waits out the normal wal_sync_ms cadence before retrying. Without this, a sustained failure (real disk error, or this sandbox's overlayfs quirk) spins flusher_flush_once/fdatasync as fast as the CPU/lock allow — a live 40-round chaotic soak hit 300k+ failed attempts on one node in under a minute (one core pinned, log flooded) before this fix.
  • Commit wake-up via callback, not a back-reference. Both the flusher AND persist_flush (below) call on_durable_advance(ctx) (registered with persist_set_durable_cb, ctx = the commit_engine_t) only when durable_lsn advanced, and after releasing p->lock. Lock order is strictly p->lock → ce->lock (cycle-free; persistence never includes commit). On shutdown, join the flusher before commit_engine_fini or the callback can fire into freed state. persist_flush firing this callback is itself a fix (bug-1564) — it used to advance durable_lsn silently, so a WAIT-blocked commit caller was only ever woken by the async flusher's own next successful cycle, never by a synchronous flush. The gap was latent until bug-1563's backoff fix above removed an accidental cover: pre-1563, the flusher's un-throttled retry loop happened to notice any state change and re-fire the callback itself within microseconds, masking that persist_flush's own advance was unobserved.

--wal-sync-ms (default DEFAULT_WAL_SYNC_MS = 10) is the maximum batching delay, a latency/throughput tunable — it is never a durability gap. An acked op is durable regardless of its value (a large value just widens the batch window; the commit barrier still waits for durable_lsn).

Standby Persist-Before-ACK

A standby fdatasyncs a replicated entry before it sends its WalAck. persist_wal_write_replicated rc-gates the inline sync and returns -EIO on failure; the caller only ACKs on rc == 0. A standby must never ACK a non-durable entry — that is half of the majority guarantee.

Recovery Integrity Verdict

replay_wal / persist_recover distinguish a clean log end from corruption and surface a recover_verdict_t:

  • RECOVER_CLEAN — valid CRC chain to a clean entry boundary, OR an empty first-boot WAL, OR a pre-allocated-zero tail, OR a snapshot sentinel + clean replay.
  • RECOVER_TRUNCATED — replay stopped on structural corruption (CRC chain break, CRC mismatch, short header/payload read).

The verdict is monotonic (init CLEAN; only ever set TRUNCATED; a later clean segment cannot un-set it) and is structural integrity only — "behind the cluster" is a separate concern handled by the freshness/promotion fence. Recovery never silently discards committed entries. Consume via persist_recovery_verdict(p) / persist_recovery_truncated(p) (these are NOT XCE_TESTING-gated — the promotion gate reads them in production).

Durable Floor & Durable-Backed Freshness

On recovery, the member publishes its durable floor = highest intact replayed LSN (the WAL tip) via persist_durable_floor(p). It is assigned, not max'd, so a truncating restart lands at the lower intact tip and never over-advertises. This floor is a durable last-log index, not the commit position.

Election freshness is then durable-backed (formula lives in ha.c/cluster.c, consumed from here):

  • A candidate advertises bare persist_durable_floor for the LSN axis — it may only claim a position it can durably back.
  • A voter floor = max(durable_floor, last_known_committed_lsn) — durable tip with the committed-high-water backstop retained (bug-339).

Result: a truncated/behind node cannot win an election or pass the promotion gate; the most-durable reachable node always can (no livelock).

What Persistence Does NOT Persist

  • Never write committed_lsn to disk. Raft keeps the commit index volatile; durability is about the log, not the commit index. The post-election safe-commit point is re-derived from voter replies (ha_compute_safe_committed_lsn), never from a stale ctrlhello.
  • LSN 0 is a reserved sentinel — never a real entry; the first append is LSN 1. The commit engine rejects lsn == 0.

Apply Correctness: APPLY_DIVERGENT (materialized state vs. WAL, not durability)

A separate verdict from recover_verdict_t (above): persist_apply_verdict(p) (APPLY_CLEAN/APPLY_DIVERGENT) answers "does the live materialized state (job/queue tables) actually match what a clean replay of the durable WAL would produce" — durability says the bytes are safe; this says the applied result is trustworthy. Computed once, at boot, in persist_recover(): an entity-count check (WAL_JOB_SUBMIT entries scanned vs. live table growth) plus a position check against p->last_applied_lsn (the LOGICAL apply cursor — never the physical WAL-scan tip, which also advances on checkpoint-marker/unknown-type entries and so falsely triggers DIVERGENT on an ordinary crash-during-checkpoint, bug-1419).

Unconditional, restart-only fence (deliberate — landed by raft-apply-decoupling task 5.1-5.3, closes bug-1418's promotion-gate liveness-deadlock family): once APPLY_DIVERGENT, the promotion gate (ha.c raft_cb_become_leader and promotion_fence_timeout_fn) refuses CTRL_PRIMARY unconditionally — no peer/HWM/durable-floor condition may clear it live, since a divergent local materialization can't be trusted for a peer/catch-up comparison either. Only a fresh, clean persist_recover() replay clears it (apply_verdict has exactly two production write sites: persist_init()'s reset and persist_recover()'s boot-time computation — no live path resets it). Do not "fix" an apply-divergent liveness stall by relaxing this gate — the actual fix is eliminating the false-positive DIVERGENT verdicts at the source (a boot-time TOCTOU where a returning node's verify pass raced the live apply pump / live-replication apply path, bug-1416, closed via persist_t.recovery_complete + persist_mark_recovery_pending called before any network traffic is processed).

APPLY_CLEAN is entity-count + cursor-position only — it does NOT prove per-field completeness (bug-1678). persist_apply_verdict()'s two checks (WAL_JOB_SUBMIT-count-vs-table-growth, last_applied_lsn position) both passed while a job's allocs (the resource allocation the scheduler assigned) silently replayed as NULL after a full restart — job.state was correctly RUN, the row existed, the cursor was in the right place, and APPLY_CLEAN reported healthy the entire time. Root cause: WAL_JOB_DISPATCH's payload (wal_jobstate_t) never carried the allocation at all, so no apply-side check could have caught the gap even in principle — the field was never in the log to compare against. The real invariant APPLY_CLEAN should imply — but today only partially enforces — is: online-apply and replay-apply of the same committed log MUST produce byte-identical materialized state for every persisted field of every entity, not just matching row counts and an advancing cursor. When adding a new WAL fact type or a new field to an existing one, verify explicitly that (a) the field is actually encoded into the WAL payload (not just the in-memory struct at dispatch time), and (b) both apply seams — apply_committed_entry() (primary boot replay) and apply_wal_entry() (standby catch-up / replicated-apply) — decode and adopt it. Tracked as unimplemented future work at the systemic level in roadmap.md's raft-apply-decoupling item #4 / raft-sim-conformance follow-up (a generic per-field snapshot-diff regression harness); until that lands, treat this as a standing audit dimension (see xce-audit-invariants skill) rather than a one-time fix.

Snapshot Save/Load Field-Completeness (Scheduler)

The identical invariant applies to the scheduler's own, separate persistence mechanism (xce_sched_core_save_snapshot()/xce_sched_core_load_snapshot(), src/scheduler/src/snapshot{,_wire,_load}.c) -- a completely different code path from this file's WAL-based durability (no shared code, no shared fixes), but the same failure shape: a snapshot-restored entity can report the correct state while silently missing a field that state depends on, with nothing in the load path able to detect it because the field was never written to the wire type in the first place. Confirmed instance: job->allocs/job->accounts (a running job's resource allocation) were never captured by build_wire_job() -- snapshot_wire.c:167's own comment flags it -- so a snapshot-restored RUNNING job showed the right state but permanently zero resource usage, the scheduler-side mirror of this section's job->allocs/EXEC_HOST story above (bug-1678). Scoped as its own spec, scheduler-snapshot-job-allocation, rather than folded into WAL work, since the two persistence mechanisms don't share fixes. Unlike the WAL side, snapshot.c's own "Scope decisions" header already documents SOME gaps explicitly (job->priority, queue project_name/job_pick_plugin) -- those are fine, the invariant is about UNDOCUMENTED gaps. Tracked as xce-audit-invariants' scheduler dimension 4 -- when adding a new field to a snapshot-round-tripped entity, verify explicitly that it's written in the matching build_wire_*() function AND read back in xce_sched_core_load_snapshot(), or add it to the documented-gaps list if omitting it is a deliberate choice.

Fail-Loud, Never Self-Exit

When a member cannot reach a committed-consistent state (recovery-truncated/behind, no healthy peer), it enters a loud permanent fence: log a loud operator-visible ERROR (truncation boundary vs cluster HWM), set the sticky recovery_unrecoverable flag, hold non-promotable, stay alive, and keep retrying — it self-heals when a peer returns. It MUST NEVER exit/abort as a designed response. The decision is the pure persist_recover_decision → RECOVER_DECISION_{ELIGIBLE,FENCE,FENCE_UNRECOVERABLE}; the action is persist_recover_loud_fence_if_unrecoverable (no terminal). Authoritative statement: error-handling.md + src/controller/CLAUDE.md "Process Lifecycle".

Testing Durability Deterministically

  • Force durability synchronously with persist_flush(p) (does an fdatasync, advances durable_lsn, and fires on_durable_advance on genuine advance — bug-1564) when a test needs durable_lsn ≥ L before exercising a commit. The demand-driven flusher is async, so any test that synchronously expects "covering ACK ⇒ committed advances" must first make the entry durable (mirror this in in-memory fixtures by priming durable_lsn to the appended tip). A WAIT-blocked commit caller now wakes from either path — don't assume only the async flusher can release one.
  • XCE_TESTING seams (e.g. persist_test_set_force_fsync_fail, persist_test_durable_lsn_on_disk) are compiled out of production — verify exclusion with nm.
  • Gate test runs on the ANSI-stripped :FAIL: count from the test binary's stdout, not on meson's Fail:0 summary (meson aggregates and hides Unity sub-group failures).
  • Run final durability validation on a real fsync-able filesystem — some sandbox /tmp mounts mis-handle fdatasync.

Quick Reference

Concern Symbol / Site
Durable watermark persist_durable_lsn(p) / persist_t.durable_lsn
Durable floor (WAL tip) persist_durable_floor(p)
Commit self-cap majority_candidate_locked (commit.c) caps self at durable_lsn
Group-commit wake (async + sync) persist_set_durable_cb → on_durable_advance(ctx), fired by both fsync_worker and persist_flush on genuine advance
Synchronous flush (tests) persist_flush(p) — also fires on_durable_advance (bug-1564)
Recovery verdict persist_recovery_verdict(p) / persist_recovery_truncated(p)
Loud fence (no exit) persist_recover_loud_fence_if_unrecoverable, recovery_unrecoverable
Flush cadence (max batch delay) --wal-sync-ms, DEFAULT_WAL_SYNC_MS = 10