lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
myxce/.kiro/specs/ha-commit-durability/tasks.md

23 KiB
Raw Blame History

Implementation Plan

TDD is mandatory (project rule): write the failing test in testsuite/ before the C change (red → green → refactor). Re-verify HA changes against raft/raft.tla and the src/controller/CLAUDE.md Protocol Verification Checklist. Most core tasks edit the same correctness-critical files (persist.c, commit.c, ha.c, cluster.c) and are therefore sequential by design; only genuinely file-disjoint analysis is marked (P).

  • 1. Foundation: durability verification seam and deterministic reproduction

  • 1.1 Add a configurable flush-cadence override

    • Parse a --wal-sync-ms controller flag (following the existing --sync-timeout-ms convention) into the persistence config before persistence starts; default behavior unchanged.
    • Observable: starting a controller with --wal-sync-ms <N> sets the flush cadence to N; omitting it preserves the current default.
    • Requirements: 3.1
    • Boundary: DurabilityConfig

  • 1.2 Add the test-only durability-verification hook

    • Add an XCE_TESTING-gated seam that lets a test assert an acknowledged operation is recoverable after an immediate ungraceful termination; compiled out of production builds.
    • Observable: in a test build the hook is callable and the production build links without it.
    • Requirements: 3.2, 3.3
    • Boundary: DurabilityTestSeam

  • 1.3 Drive a deterministic durability reproduction from the soak harness

    • Extend test-commit-durability.sh with a deterministic mode that widens the batching window / uses the seam and kills a durable contributor in-window.
    • Observable: on the current (unfixed) build the deterministic mode reports DATA LOSS (RED), proving bug-661 without relying on race timing.
    • Requirements: 3.1, 3.2
    • Depends: 1.1, 1.2

  • 2. Durable commit mechanism (acknowledgment ⇒ majority persistence)

  • 2.1 Introduce durable-LSN tracking via a group-commit flusher

    • Repurpose the background fsync thread from a timer into a demand-driven flusher: an append signals it; it performs one fdatasync over all pending appends, advances a durable-LSN watermark, and wakes the commit evaluation.
    • Guard against lost wakeups: the signal is level-triggered on the appended-vs-synced gap so an entry is never considered durable before its fdatasync (gating item).
    • Observable: a unit test shows one fdatasync advances the durable-LSN past several appended LSNs under load, and the durable-LSN never exceeds the synced tip.
    • Requirements: 1.1, 1.6
    • Boundary: DurableCommit

  • 2.2 Count self toward the commit majority only when durable

    • Change the commit-majority evaluation so the local member's self-contribution is capped at the durable-LSN watermark, not the appended tip; clamp the watermark on WAL truncation.
    • Observable: a unit test shows committed_lsn does not advance over an entry until that entry is durable locally, and the self-cap equals the durable-LSN.
    • Requirements: 1.2, 1.4
    • Boundary: DurableCommit

  • 2.3 Pin standby persist-before-acknowledge with a regression test

    • Add a regression test asserting the standby apply path fdatasyncs a replicated entry before sending its acknowledgment.
    • Observable: the test fails if the standby ACK is emitted before the durable write and passes against the current behavior.
    • Requirements: 1.3
    • Boundary: DurableCommit

  • 2.4 Reconcile the dormant synchronous-append commit path

    • The synchronous-append path advances the commit position directly on majority acknowledgment and is built on the premise that the append already fsync'd inline — a premise group-commit removes. Confirm it has no production caller and delete it, or route it through the same durable-majority evaluation so it can never advance the commit position on a non-durable entry.
    • Observable: a code search shows no path advances the commit position on majority ACK without the durable precondition; the dormant path is removed or re-routed, and a test guards against a future caller reintroducing an ACK-without-durability commit.
    • Requirements: 1.1, 1.2
    • Boundary: DurableCommit
    • Depends: 2.1

  • 3. Recovery integrity (no silent truncate; durable floor)

  • 3.1 Return a recovery integrity verdict and mark a truncated member non-promotable

    • Make WAL replay distinguish a clean log end from a corruption-stopped end (CRC chain break / mismatch / short read) and surface that verdict; a corruption-truncated recovery is never reported as a clean recovery and sets a member-level "recovery-truncated" flag that the promotion gate (4.3) consumes.
    • Observable: a unit test injecting a CRC break at the first entry yields a TRUNCATED verdict and sets the non-promotable flag, while an intact log yields CLEAN; an empty first-boot WAL is CLEAN (not corruption).
    • Requirements: 2.1
    • Boundary: RecoveryIntegrity

  • 3.2 Pin the fail-loud vs fence boot policy as a decision with a verification table

    • Pin the concrete policy (gating item, decision only — no behavior change yet): a truncated member fences from leadership at boot, attempts catch-up from a healthy peer, and terminates loudly only on a bounded failure to reach a committed-consistent state — never a silent indefinite fence. Encode the decision as a table of input states → expected outcome.
    • Observable: the policy and its decision table are recorded, and skeleton tests assert each row's expected outcome (initially failing), establishing the contract 3.3 and 4.3 implement.
    • Requirements: 2.2, 2.4
    • Boundary: RecoveryIntegrity

  • 3.3 Implement loud FATAL on bounded-unrecoverable recovery

    • Implement the FATAL branch of the 3.2 policy: when a recovery-truncated member cannot reach a committed-consistent state within the bounded catch-up window, terminate loudly with the truncation boundary logged; a reachable healthy peer never triggers FATAL.
    • Observable: the 3.2 decision-table rows for the FATAL/no-FATAL cases pass; logs show the truncation boundary on FATAL.
    • Requirements: 2.4
    • Depends: 3.1, 3.2
    • Boundary: RecoveryIntegrity

  • 3.4 Re-derive the durable floor from the recovered WAL tip

    • On recovery, publish the member's durable floor as the highest intact replayed LSN so later advertisement is backed by durable state rather than a volatile in-memory position.
    • Precision: this floor is the durable last-log index (WAL tip), used by the Raft freshness gate — not the commit position; the post-election safe-commit point continues to come from voter replies, not from this floor.
    • Observable: a unit test shows that after a truncating restart the published durable floor equals the highest intact LSN (the WAL tip), not a stale higher in-memory value or the commit position.
    • Requirements: 2.3
    • Boundary: RecoveryIntegrity

  • 4. Promotion fence and durable-backed freshness

  • 4.1 (P) Pin the freshness-floor formula and re-verify election safety and liveness

    • Define the exact reconciliation of the durable-derived freshness floor against the existing in-memory floor — the durable quantity is the durable last-log index (WAL tip), not the commit position — then re-check it against raft/raft.tla for Leader Completeness AND liveness (no election livelock) plus the Protocol Verification Checklist (gating item; analysis only, no shared source files).
    • Scope of the raft.tla check: it validates the election/freshness logic (the mlastLogIndex/LastTerm semantics this change touches), which raft.tla models directly. It does NOT validate the durability of that floor — raft.tla assumes a durable log (see 5.3). The durability of the WAL-tip derivation is verified by tests (3.4), not by raft.tla.
    • Observable: the pinned formula is recorded and the model/checklist verification passes for both a truncated-candidate-denied case and a longest-durable-log-still-elects case.
    • Requirements: 2.3
    • Boundary: FreshnessFloor

  • 4.2 Derive the vote and PreVote freshness floor from durable WAL state

    • Implement the pinned formula so a candidate cannot advertise, and a voter cannot grant against, a position beyond durable WAL state; the post-election safe-commit point continues to come from voter replies, never a stale ctrlhello.
    • Observable: a node restarted with a truncated log is denied (vote and PreVote), while the node holding the longest durable log still wins — no livelock.
    • Note: 4.2 and 4.3 both edit the same election code (cluster.c vote/PreVote and ha.c promotion); they are sequential (4.2 before 4.3), not parallel.
    • Requirements: 2.3
    • Depends: 4.1, 3.4

  • 4.3 Add the pre-promotion eligibility gate and fence release

    • Before a member becomes primary, refuse promotion if it carries the recovery-truncated flag (from 3.1) or its durable position is below the cluster committed high-water mark; extend the existing catch-up fence to cover the promoting/recovered member and release it when catch-up passes the last committed boundary. Reuse the durable-LSN truncate-clamp from 2.2 so the gate evaluates a clamped durable position at the promotion-path truncate site.
    • Observable: a recovery-truncated or behind member is denied promotion until catch-up completes, then regains eligibility; a hollow node cannot win even at minimal quorum.
    • Requirements: 2.2, 2.5, 2.6
    • Depends: 4.2, 3.1, 3.2, 2.2

  • 5. Integration and validation

  • 5.1 Wire durable-majority commit end to end

    • Connect the durable-LSN self-count and the commit barrier so the client reply is released only after the entry is durable on a majority; assert liveness (the flusher does not stall heartbeat/election handling).
    • Observable: an integration test confirms reply-ok only after durable-majority, killing the sole durable contributor mid-commit leaves the acknowledged op present on the survivors, and the full controller suite passes under ASAN.
    • Requirements: 1.1, 1.4, 1.5, 1.6, 2.5
    • Depends: 2.2, 4.2

  • 5.2 Prove zero acknowledged-job loss across failover

    • Run the deterministic durability E2E and the rolling SIGKILL+restart soak; both must show every acknowledged job surviving every failover.
    • Observable: the deterministic mode that was RED in 1.3 now passes (GREEN), and the soak reports zero DATA LOSS across rounds.
    • Requirements: 1.1, 1.5, 2.2, 2.3, 2.6
    • Depends: 5.1, 4.3, 1.3

  • 5.3 Re-verify the formal safety checklist and flip the durability gap note

    • Re-run the raft/raft.tla alignment and the Protocol Verification Checklist (commit rule, leader completeness, commitIndex monotonic) against the integrated implementation.
    • IMPORTANT — raft.tla does NOT model durability: it restores log intact from "stable storage" on restart (raft.tla:165-174), i.e. it assumes what bug-661 violates. A passing TLC/alignment run on the unmodified spec therefore does NOT prove the commit-durability invariant. That invariant is verified by the unit/integration/E2E tests (2.x, 5.1, 5.2) plus the manual checklist — not by raft.tla. Do not conclude durability is proven from raft.tla alone. Formal durability proof would require extending raft.tla with a durable-vs-volatile log model (out of scope unless explicitly opted in).
    • Update src/controller/CLAUDE.md: convert the "Commit Durability — KNOWN GAP (bug-661)" note into the now-true guarantee and tick the post-ha-commit-durability checklist item.
    • Observable: the checklist passes with the durable self-count and the durable-backed freshness floor in place; the durability invariant is backed by passing tests (not by raft.tla); the CLAUDE.md gap note is removed; any deviation is recorded.
    • Requirements: 1.2, 2.3
    • Depends: 5.1

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).
  • 1.3: Deterministic mode = bash src/tools/test-commit-durability.sh --deterministic proves the durability GAP via strace, NOT joblist data-loss. SCOPE CLARIFICATION (review-ratified): single-host SIGKILL committed-job loss is physically NON-deterministic — persist_wal_async pwrites inline (page cache survives process death) and standbys fdatasync-before-WalAck (persist.c:515), so a committed job is always durable off the primary; a data-loss run flips GREEN/RED/RED. The deterministic slice: with --wal-sync-ms 30000 the primary acks "jobsubmit ok" with ZERO primary fdatasync in the submit→ack window (exec-wrap strace -f -tt -e trace=fdatasync,fsync; PTRACE_ATTACH -p is yama-blocked). RED = in-window primary syncs==0 (15/15 across 5 runs; primary=0 vs standby=2). This is a valid RED→GREEN gate: the task-2.x fsync-before-ack fix makes the primary count ≥1 ⇒ GREEN. Original racy soak retained as the default mode. Boundary: only src/tools/test-commit-durability.sh (flag plumbed via deploy-ha.sh start.sh $* passthrough — no C edits). For task 5.2's E2E, prefer this strace gap-proof over joblist-loss for a deterministic durability assertion.
  • 2.1: persist_t.durable_lsn (_Atomic uint64_t, accessor persist_durable_lsn()) = prefix watermark advanced ONLY after a successful fdatasync. fsync_worker is now a demand-driven group-commit flusher: persist_wal_async pwrites under p->lock, releases, then signals fsync_cond. Lost-wakeup guard is LEVEL-triggered on flusher_work_pending_locked (durable_lsn < current_lsn-1), re-checked under p->lock before wait and after each flush (reviewer-confirmed sound). Invariants: monotonic, durable_lsn <= current_lsn-1, clamped on truncate (clamp_durable_lsn_locked at both truncate paths). fail-loud rc-gate: durable_lsn does NOT advance on fdatasync error (all 4 sites: flusher main/fallback, persist_stop, persist_flush) — logs ERROR. Test fault-injection seam: #ifdef XCE_TESTING toggle persist_test_set_force_fsync_fail() (production-excluded, nm-verified). NOT yet consumed by commit (that is 2.2). FOLLOW-UP (non-blocking, out of scope for 2.1): bound the flusher retry on SUSTAINED fdatasync failure (backoff / rate-limited logging) — currently tight-spins (releases p->lock each iter, so liveness OK, but CPU/log burn on a degrading disk). NOTE: a HANGING (not erroring) fdatasync blocks appends since it runs under p->lock — pre-existing, not 2.1's boundary.
  • 2.2: majority_candidate_locked (commit.c:93) self-cap = persist_durable_lsn(ce->p) (was highest_appended_lsn); appended tip kept only for the assert(cand <= highest_appended_lsn) never-beyond-log check. So committed_lsn cannot advance over an entry until it is durable locally (Req 1.2/1.4). Durable-advance WAKE (the wire 2.1 deferred): persist defines a callback hook on_durable_advance(void*)/durable_cb_ctx + persist_set_durable_cb(); commit registers commit_on_durable_advance (ctx=engine) in commit_engine_init, unregisters in commit_engine_fini. Flusher invokes it ONLY when durable_lsn advanced, AFTER releasing p->lock → callback takes ce->lock + broadcasts ce->cond. Respects commit→persist dependency direction (persist never includes commit). Lock order p->lock→ce->lock only (cycle-free). Lost-wakeup-free: broadcast follows the release-store; waiter re-reads durable under ce->lock; timedwait backstop. Shutdown UAF closed: persist_stop JOINS the flusher BEFORE commit_engine_fini (cluster.c:1675 vs 1695); final stop-flush does NOT fire the callback; fini also unregisters defensively. GOTCHA (bug-704): self-cap broke 4 existing commit-path fixtures (test_repl_majority/coldstart_lsn0/commit_barrier_rollout/demotion_convergence) that primed current_lsn but not durable_lsn — fix = prime durable_lsn = appended tip in any in-memory fixture that sets current_lsn without writing real WAL bytes (the flusher can't advance durable for a fake log). Any future commit-engine test must prime durable_lsn too.
  • 2.3: Standby persist-before-ACK pinned + Req-1.3 hole closed. persist_wal_write_replicated (persist.c:741) now rc-gates the inline fdatasync: on failure (or flusher_fsync_forced_fail) it logs ERROR + return -EIO (symmetric with the 2.1 flusher gate) — the standby will NOT ACK a non-durable entry. Caller ha_apply_single_entry (ha.c:1135-1166) ALREADY routes a negative return to no-apply/no-ACK (ha_send_wal_ack_reply reached only on rc==0), so NO ha.c change was needed (boundary stayed persistence-only). Ordering pin = #ifdef XCE_TESTING atomic_uint test_replicated_fdatasync_count incremented at the sync site; test_replicated_persist_syncs_before_return asserts it increments inside the call (page-cache means a disk re-read can't prove fsync — use the counter). test_replicated_sync_failure_no_ack REDs without the gate. Seam production-excluded (nm-verified). HOUSEKEEPING: controller tests mkdtemp into /tmp/xce_* and don't clean up — thousands accumulate and eventually make mkdtemp fail → misleading suite-wide "Expected Non-NULL" cascade. Run rm -rf /tmp/xce_* if that appears (pre-existing test pattern, not a regression).
  • 2.4: SPEC INACCURACY corrected — persist_wal_sync_append is NOT dormant; it is the LIVE job-submit path (persist_job_hook_fun calls it at persist.c:1911). Took the "route through durable-majority eval" option (delete infeasible). Production path (ce!=NULL, lsn!=0) was already durable-safe via 2.2's majority_candidate_locked self-cap. Closed the hazard in the LEGACY branch: the former unconditional atomic_store(committed_lsn, written_lsn) on majority ACK is now sync_append_legacy_commit_store() which advances ONLY when written_lsn <= persist_durable_lsn(p) (LSN-0 edge: store of 0 valid). Added the design's debug invariant committed_lsn <= durable_lsn (#ifdef XCE_TESTING assert at commit.c:162 + inside the legacy store) as the future-proof guard. Fixed the stale "after local fsync via persist_wal_async" comment (2.1 deferred the fsync). CODE-SEARCH (authoritative): the ONLY committed_lsn stores are — engine advance (commit.c, durable-capped by 2.2), legacy sync_append (now gated), and the two persist_truncate_wal_above cutoff stores (NOT ACK-driven; each immediately clamp_durable_lsn_locked). ha.c/ha_convergence.c stores are last_known_committed_lsn (DIFFERENT field). So no path advances commit on ACK without the durable precondition. Seam persist_test_sync_append_legacy_commit_store is production-excluded.
  • 3.1: recover_verdict_t {RECOVER_CLEAN, RECOVER_TRUNCATED} (persist.h) + persist_t.recovery_verdict field + accessors persist_recovery_verdict()/persist_recovery_truncated() (NOT XCE_TESTING-gated — 4.3 consumes them in production). replay_wal classifies: CRC chain break / CRC mismatch / short header/payload read → TRUNCATED; clean entry boundary / pre-allocated-zero tail / empty WAL / snapshot-sentinel+clean-replay → CLEAN. Verdict is monotonic (init CLEAN once; only ever set TRUNCATED; a later clean segment can't un-set it). Verdict = STRUCTURAL integrity, NOT "behind the cluster" (the behind-high-water case is 4.2/4.3). 3.1 ONLY sets state + WARNs — does NOT FATAL (3.3) or fence (4.3). DRY: did NOT extract a shared scanner (replay_wal mutates persist_t; the 1.2 seam is read-only) — no third copy added. ENV FINDING (see cerebrum 2026-06-18): this sandbox's /tmp can be an overlay that EIOs on fdatasync; because committed_lsn is now durable-gated (2.x), an fdatasync EIO correctly HOLDS committed_lsn, which makes test_fact_hook_append_is_retained_and_nonblocking (commit_barrier_rollout) FAIL on overlay /tmp — confirmed pre-existing (fails identically on the clean pre-spec baseline), NOT a regression. Final validation (5.x) must run on a real fsync-able fs.
  • 3.2: DECISION-ONLY (no production change). Boot Policy Decision Table recorded in design.md §"Boot Policy Decision Table (task 3.2)" — 7 rows over {verdict CLEAN/TRUNCATED × at-above/below HWM × peer reachable × bounded-catch-up succeeds/fails} + snapshot-sentinel + empty-first-boot. Outcomes: ELIGIBLE / FENCE+CATCH-UP→ELIGIBLE / bounded FATAL. AUTHORITATIVE INVARIANT: never a silent indefinite fence — every truncated/behind member resolves to exactly one terminal (catch-up→ELIGIBLE, or BOUNDED FATAL). Snapshot-sentinel is NEVER FATAL on a legitimate sentinel. 7 skeleton tests in test_commit_durability.c (row1,2,3,4,4b,5,6) are TEST_IGNORE_MESSAGE-skipped (PENDING 3.3/4.3) so the suite stays GREEN — they are the RED contract 3.3 (FATAL rows) and 4.3 (fence/eligible rows) turn green. NOTE: pre-existing ASAN leaks in unrelated tests (test_demotion_convergence, ha_start_election_async, ha_rdhup_integration) make the STANDALONE controller_unit_test exit=1, but meson reports Fail:0 — don't be fooled by standalone exit code.
  • 3.3: FATAL branch = pure decision persist_recover_decision(recover_policy_inputs_t) → recover_decision_t {ELIGIBLE, FENCE, FATAL} (persist.c) + action persist_recover_fatal_if_unrecoverable() which logs the truncation boundary (durable_floor_lsn/cluster_hwm_lsn + window ms) at ERROR then terminates via persist_fatal_hook (extern void(*)(const char*), NULL in prod → tlog_exit(); abort(); mirrors ha_fatal_hook; tests install it to capture instead of abort). FATAL ONLY for bounded-unrecoverable + NO healthy peer (rows 4/4b); a reachable peer NEVER FATALs (gated behind if (!healthy_peer_reachable)); eligibility rows 1/2/6 never FATAL. Bound = FINITE DEFAULT_RECOVERY_CATCHUP_WINDOW_MS=30000 (persist.h) — keys off the catch_up_succeeded bounded-window result, never an indefinite wait. Turned 3.2 skeleton rows 4/4b GREEN + added test_boot_policy_peer_reachable_never_fatal; rows 1/2/3/5/6 stay IGNORE (FENCE→ELIGIBLE is 4.3). DEFERRED to 4.3/5.x: wiring this decision into the live boot/promotion path (3.3 is the unit-tested decision + terminate only; no ha.c/cluster.c edits).