myxce/.kiro/specs/raft-apply-decoupling/tasks.md

Implementation Plan

• 1. Commit engine event-driven resolve (keystone prerequisite)

• 1.1 (P) Add per-LSN continuation registration to the commit engine for the synchronous (WAIT) commit mode, replacing the blocking wait

  • Register a continuation (callback + context) against an appended LSN instead of parking the calling thread
  • Preserve the existing per-LSN bookkeeping and admission-cap discipline already used by the commit engine
  • Observable: a unit test can register a continuation for an LSN and the calling thread returns immediately, before the LSN commits
  • Requirements: 1.3, 1.4, 9.1, 9.2
  • Boundary: Commit Engine Resolve — commit.c per-LSN registry only

• 1.2 Build the generic resolve primitive and wire the majority-ack (committed) outcome

  • Implement commit_resolve(): invoke a registered continuation with a committed or failed outcome, exactly once, idempotent on re-resolve
  • On majority ack, invoke the continuation with a committed outcome
  • Dispatch every continuation invocation off the calling thread using the existing non-blocking task-submission primitive, never inline on the replication-ACK or iomgr path
  • Observable: forcing majority-ack in a test resolves the registered continuation exactly once with a committed outcome
  • Requirements: 1.3, 9.3, 9.4
  • Note: real timeout/demotion/truncation detection (as opposed to the generic resolve primitive) is 1.3/1.4 — see Implementation Notes

• 1.3 Arm a timeout for registered async continuations

  • commit_submit_async() no longer parks a thread that would otherwise notice its own pthread_cond_timedwait deadline expire — converting away from the blocking wait removes the ONLY mechanism that ever detected a commit timing out. Add an explicit timeout (e.g., a delaymgr-driven check, or a deadline recorded at registration and swept periodically) that resolves a still-pending registered continuation as FAILED with -ETIMEDOUT if majority is not reached within the caller-supplied timeout
  • Preserve the existing default timeout value and error code so callers observe identical failure timing/vocabulary to today's blocking path — this is a "no regression" requirement (11.1), not just a new capability
  • Observable: an async-registered continuation for an LSN that never reaches majority is resolved as FAILED with -ETIMEDOUT within the configured timeout window, with no thread blocked to detect it, verified by a test that lets real time (or a mocked clock) elapse past the deadline without ever calling commit_resolve() manually
  • Requirements: 1.3, 1.4, 9.3, 9.4
  • Boundary: Commit Engine Resolve — commit.c only

• 1.4 Wire demotion and log-truncation detection to resolve pending continuations as failed

  • When this node is demoted (loses leadership) or truncates its log tail (conflict reconcile), any still-pending registered continuation for an LSN affected by that transition must resolve as FAILED — otherwise a demoted/truncated node leaves requesters permanently unresolved
  • Find the existing demotion/role-transition point and the existing log-truncation point and add a call into the commit engine's resolve path for every pending LSN they invalidate
  • Observable: forcing a demotion or a truncation while a continuation is registered for an affected LSN resolves it as FAILED exactly once, with the existing error-code vocabulary
  • Requirements: 1.3, 1.4, 9.3, 9.4
  • Boundary: Commit Engine Resolve integration points — this task is a narrow, explicit exception to the "commit.c only" boundary used by 1.1-1.3: it may touch the demotion/role-transition site and the log-truncation site (wherever they currently live) in addition to commit.c, but must not touch job.c or any command-handler code
  • Depends: 1.3

• 1.5 Unit test the commit engine's resolve behavior under concurrent in-flight commits

  • Cover exactly-once resolution across majority ack, timeout, demotion, and truncation, exercised through the real detection paths built in 1.3/1.4 (not by calling commit_resolve() directly, which 1.2's own tests already cover)
  • Cover that a burst of concurrently pending commits does not exhaust the bounded request-handling capacity
  • Observable: a test suite run demonstrates bounded worker usage and exactly-once resolution under a concurrent-commit burst
  • Requirements: 9.1, 9.2, 9.3, 9.4
  • Depends: 1.4

• 2. Shared apply seam for committed entries

• 2.1 (P) Extract the guarded state-transition logic used by fact handling into a form usable outside the receipt-time call site

  • Keep the existing monotonic-rank checks (reject backward transitions, no-op duplicate terminal transitions) intact and unchanged in behavior
  • Decouple the transition logic from firing its own hook inline, so hook-firing can be relocated by a later task without touching this logic again
  • Observable: the extracted transition logic can be invoked directly by a test without going through the worker-report receipt path, and produces the same guard behavior as before extraction
  • Requirements: 3.3, 7.1, 7.2
  • Boundary: Apply Seam — persist.c / job.c apply-dispatch and replay call sites only, no dependency on the commit-engine change in task 1

• 2.2 Build the single dispatch function that applies one committed entry to in-memory state, covering every classified entry type

  • Route job submission through the existing idempotent, jobid-addressable construction primitive instead of a raw uncoordinated insert
  • Route job state transitions through the extracted guarded logic from 2.1
  • Route queue control and worker control entries through their existing restore primitives
  • Fire the entry type's post-mutation notification hook from inside this function, after the mutation completes
  • Observable: calling the dispatch function directly with a sample entry of each classified type produces the correct in-memory mutation and fires exactly one post-mutation notification
  • Requirements: 2.2, 3.1, 3.2, 4.1, 4.2, 5.2
  • Depends: 2.1

• 2.3 Point restart replay's per-entry application at the new dispatch function

  • Replace replay's direct calls to the individual restore primitives with calls to the dispatch function from 2.2
  • Observable: restart replay of a log containing every classified entry type produces identical materialized state to before this change, verified by an existing or updated replay test
  • Requirements: 3.1, 3.2
  • Depends: 2.2

• 2.4 Unit test idempotent and ordered re-application of the dispatch function

  • Cover applying the same committed entry twice is a no-op with respect to observable state
  • Cover that a duplicate or out-of-order fact entry does not regress a job's lifecycle state
  • Observable: a test applies a FINISH entry followed by a stale, previously-unapplied START entry for the same job and confirms the job remains in its terminal state
  • Requirements: 3.4, 7.1, 7.2, 7.3
  • Depends: 2.2

• 3. Command apply-after-commit ordering

• 3.1 (P) Stop constructing the in-memory job object before the commit barrier

  • Change job submission to carry forward only the data needed to build the WAL entry payload, without inserting anything into the in-memory job table until the (still-synchronous, at this point) commit call returns success
  • Remove the now-unreachable code path that previously ran the durability step from inside the notification hook for job submission
  • Observable: a job submission whose commit fails or times out leaves no job object in the in-memory job table; a successful submission still replies synchronously exactly as before this task
  • Requirements: 1.2, 2.1, 2.3
  • Boundary: Raft Step (Command) — job.c handle_jobsubmit and its call chain only, no dependency on task 1

• 3.2 Convert job submission to register its commit outcome through the new continuation mechanism instead of blocking

  • Replace the synchronous commit call from 3.1 with a registration against the task 1 continuation API
  • On a failed or timed-out outcome, reply failure directly from the continuation — no apply is needed on this path
  • On a committed outcome, hand the entry off to the ordered apply pump (task 6) rather than applying or replying from this continuation directly, so this entry's apply and reply happen in strict commit order relative to every other entry
  • Observable: the calling thread returns before the commit resolves; a forced failure/timeout outcome still replies failure immediately with no job ever appearing (the committed-outcome path has no independently observable behavior until task 6 completes it)
  • Requirements: 1.1, 1.4, 2.1, 2.3
  • Depends: 1.1, 1.2, 1.3, 1.4

• 3.3 Unit test the command failure path leaves no observable trace

  • Force repeated commit failures and timeouts for job submission and confirm no job object is ever created
  • Observable: the test suite demonstrates zero phantom job objects across repeated forced commit failures
  • Requirements: 2.1, 2.3
  • Depends: 3.2

• 4. Fact apply-after-commit split

• 4.1 (P) Change worker-fact receipt handling to validate and durably append only

  • Remove the state mutation and the post-mutation notification hook firing from the receipt-time handlers for job start, finish, fail, and update reports
  • Keep the existing pre-mutation validation hook firing at receipt time, unchanged
  • Observable: after a worker reports a job event, the job's in-memory state is unchanged immediately after the receipt handler returns
  • Requirements: 5.1
  • Depends: 2.1
  • Boundary: Fact Ingestion — job.c on_job_start/on_job_finish/on_job_fail/on_job_update receipt handlers only, disjoint from handle_jobsubmit

• 4.2 Unit test durable retention of facts submitted while majority is unreachable

  • Submit facts while majority is unreachable and confirm they are durably appended and that job state remains unchanged for the duration of the outage
  • Confirm no local-only apply code path exists by inspection of the receipt handler from 4.1 (no runtime check needed — the guarantee is structural)
  • Observable: the test demonstrates zero state mutation during a simulated quorum-loss window
  • Requirements: 6.1, 6.2
  • Depends: 4.1

• 5. Restart apply-correctness verification

• 5.1 (P) Compute a post-replay expectation from the committed log tip and compare it against materialized state

  • Derive a lightweight expectation (e.g., last-applied position and a count or checksum of materialized entities) from the recovered log
  • Compare the expectation against the actual state produced by replay through the shared apply dispatch function
  • Observable: a clean replay produces a CLEAN verdict alongside the existing structural (CRC) verdict
  • Requirements: 8.1, 8.2
  • Depends: 2.3
  • Boundary: Restart Replay & Verification — persist.c replay/recovery path only

• 5.2 Route a verification mismatch through the existing non-promotable fence mechanism

  • Reuse the existing loud-fence function and sticky fence flag rather than introducing a new terminal or fencing path
  • Observable: an injected materialization mismatch produces a DIVERGENT verdict, a loud operator-visible log entry, and a non-promotable fenced state
  • Requirements: 8.3, 8.4
  • Depends: 5.1

• 5.3 Unit test the DIVERGENT verdict path stays alive and keeps retrying

  • Force a DIVERGENT verdict and confirm the process does not exit or abort
  • Confirm the fence clears once a subsequent recovery reaches a consistent state
  • Observable: the test process remains running and retrying after a forced DIVERGENT verdict, never terminating
  • Requirements: 8.4
  • Depends: 5.2

• 6. Ordered apply pump integration

• 6.1 Build the strict-order walk that applies every newly committed entry

  • Walk entries from the last-applied position up to the current committed position, in ascending order, with no skipping or reordering
  • Invoke the shared apply dispatch function for every entry in that range, regardless of its commit mode
  • Observable: a burst of interleaved command and fact entries committing out of submission order still applies in commit order
  • Requirements: 3.3, 7.3
  • Depends: 1.2, 2.2

• 6.2 Fire a command's registered continuation immediately after its entry applies

  • After the walk in 6.1 applies an entry that has a registered commit continuation (from task 3.2), invoke that continuation now, after the mutation, not before
  • Leave fact entries with no further action beyond what the apply dispatch function itself performs
  • Observable: a successful job submission's client reply carries the job id only after the job is visible in the in-memory job table; the reply always occurs strictly after the mutation
  • Requirements: 1.1, 2.1, 2.2, 2.3
  • Depends: 6.1, 3.2

• 6.3 Integration test command and fact apply/notify timing through the full pump

  • Cover a successful job submission becoming visible and notified only after commit, exercised through the full pump (superseding the earlier synchronous-path check from task 3.1)
  • Cover that worker slot release, scheduler dispatch triggering, and client/apiserver notification for a fact do not occur until that fact's entry is committed
  • Observable: the test suite shows zero fact-reaction side effects during a delayed-commit window, and a submitted job's reply always follows its visibility in the job table
  • Confirm a consumer acting on the post-apply notification never needs to separately re-check a commit watermark, since the notification's occurrence already implies the entry is committed and applied
  • Requirements: 2.2, 4.3, 5.2, 5.3
  • Depends: 6.2, 4.1

• 6.4 Integration test quorum-loss-then-restore catch-up ordering through the full pump

  • Submit a mix of facts and commands while majority is unreachable, then restore majority and confirm every retained entry commits and applies in its original log order with no local-apply escape hatch exercised
  • Observable: the test demonstrates full, correctly-ordered catch-up after quorum returns, with zero out-of-order application
  • Requirements: 6.3, 7.3, 9.4
  • Depends: 6.1, 4.2

• 7. Conformance, regression, and existing-behavior preservation

• 7.1 Extend the deterministic conformance harness with apply-ordering assertions

  • Add an assertion that no entry's state mutation or reactions occur before that entry commits
  • Add an assertion that re-applying an already-committed entry is idempotent
  • Observable: the conformance harness run reports both new assertions passing alongside its existing safety checks
  • Requirements: 10.1, 10.3
  • Depends: 6.3, 6.4

• 7.2 Add a live-cluster regression scenario that kills the primary mid-fact-stream

  • Drive a continuous worker-fact stream, kill the primary partway through, and let a new primary take over
  • Assert no phantom slot-frees, no committed-state divergence, and no notification for a fact that never committed
  • Observable: the scenario completes with zero divergence and zero phantom reactions across repeated runs
  • Requirements: 10.2, 10.4
  • Depends: 6.3, 6.4

• 7.3 Re-run the full existing automated test suite and update only the tests tied to the old ordering

  • Run the existing unit, integration, and HA soak suites
  • Update only the specific tests that asserted the old apply-before-commit ordering to assert the new ordering instead
  • Observable: the full suite passes with zero committed-job loss and zero state divergence across the existing failover soak
  • Requirements: 11.1, 11.2, 11.3
  • Depends: 7.1, 7.2

• 7.4 Measure sustained fact-stream throughput against the pre-feature baseline

  • Record a throughput baseline on the pre-feature code, then drive the same sustained high-volume worker-fact stream against the completed feature
  • Confirm no per-fact synchronous commit barrier was introduced and throughput does not regress
  • Observable: a recorded throughput comparison shows no regression relative to the pre-feature baseline
  • Requirements: 11.4
  • Depends: 7.3

Implementation Notes

• 1.1: commit_submit_async() reuses the exact wait_admit_locked/wait_release_locked/inflight_waits admission-cap discipline as COMMIT_MODE_WAIT — task 1.2's commit_resolve() MUST call wait_release_locked() when resolving an async-registered entry, or the shared admission cap leaks. Continuation registry is calloc(n_vcpus, ...), sized so n_continuations >= wait_cap(ce) always holds; lsn == 0 is the free-slot sentinel.
• 1.1 review: bare controller_unit_test (no --run filter) segfaults deterministically in this environment (SSL_CTX_free/iomgr_fini crash, test_ha_vep_teardown.c) — reproduced identically on the pre-task base commit, confirmed pre-existing and unrelated. Logged as bug-1406. Also test_commit_advance_retry_clears_push_before_commit_fence (test_config_repl.c) fails independently of this feature, also reproduced on base — logged as bug-1407, flagged because it's adjacent to commit_advance_locked, which task 1.2 modifies. Always validate with a filtered --run <group> invocation or the tightly-coupled commit-engine cluster, not the bare unfiltered binary, until 1406/1407 are separately fixed.
• 1.2: implemented commit_resolve() + wired it into all 6 commit.c-internal paths that can advance committed_lsn/writable_lsn (the COMMITTED/majority-ack outcome). Chose VTASK_LIVE_BLOCK for dispatch (never admission-gated, unlike VTASK_REQ_BLOCK which can fast-reject — unacceptable for "exactly-once, never unresolved"). Reviewer caught a real task-graph gap, not a defect in 1.2 itself: converting COMMIT_MODE_WAIT away from a blocking wait also removes the only mechanism that ever detected a commit timing out (the caller's own pthread_cond_timedwait deadline) — nothing yet arms an equivalent timeout for async-registered continuations, and nothing yet wires demotion/log-truncation detection to call commit_resolve(FAILED, ...). Without this, a pending async command during quorum loss would hang forever instead of failing — worse than today's behavior. Split out as new tasks 1.3 (timeout arming) and 1.4 (demotion/truncation wiring), inserted before the renumbered 1.5 (was 1.3, concurrency test — now exercises the real detection paths from 1.3/1.4 instead of calling commit_resolve() directly). Task 3.2 now also depends on 1.3/1.4, since its own "failure/timeout outcome replies" observable is meaningless without them.
• 1.3: added a dedicated per-commit-engine sweep thread (not the shared delaymgr — would violate boundary + delaymgr's 200ms poll granularity vs. DEFAULT_SYNC_TIMEOUT_MS=500ms would add timing jitter). Race rule: majority-satisfied always wins over an already-expired deadline when both are true under the same lock hold. Round 1 REJECTED: commit_sweep_once() computed the next-deadline snapshot under ce->lock, unlocked to dispatch, then slept on the stale snapshot — a commit_submit_async() registration/broadcast landing during that unlocked dispatch window was lost (no thread parked on the condvar to receive it), so the sweep thread could sleep on an absent/late deadline indefinitely (worst case: unbounded delay in a low-traffic quorum-loss scenario, violating the "identical failure timing" no-regression bar). Fixed by holding ce->lock continuously across scan→wait decisions (releasing only for the unlocked dispatch step) and always re-scanning under the lock immediately before ever calling cond_wait/cond_timedwait — the standard recheck-then-wait idiom. Round 2 reviewer independently reproduced the round-1 bug shape to confirm the fix actually closes it, not just narrows it. Precedent for 1.4: any future code that needs to interrupt the sweep thread's wait (e.g., a demotion/truncation handler resolving a pending LSN out-of-band) must go through the same lock+broadcast discipline commit_submit_async() uses, or it risks the identical lost-wakeup class of bug.
• 1.4: added commit_resolve_pending_above(ce, lsn_floor, rc) (snapshot-then-commit_resolve()-per-entry, no parallel bookkeeping). Truncation: a single new on_truncate/truncate_cb_ctx callback slot on persist_t (same inversion-of-control shape as the existing on_durable_advance hook) fires from persist_truncate_wal_above after releasing p->lock, only when a truncation actually removed entries — this ONE wrapper covers all three real truncation call sites (raft_cb_become_leader's post-election truncate, the library's on_truncate_from conflict-reconcile, ha_demotion_convergence_fn's post-demotion truncate), verified by the reviewer reading all three. Demotion: raft_cb_become_follower calls commit_resolve_pending_above(&c->commit, 0, -XCE_ERR_STANDBY) (lsn_floor=0 → fails everything — a demoted node can never authoritatively confirm anything pending; verified against the CP posture). Known residual, non-blocking: a fourth internal truncation path (persist.c inside persist_wal_write_replicated's tip-conflict-repair branch, STANDBY replicated-apply only) calls truncate_wal_above_locked() directly, bypassing the new wrapper — safe today only because demotion resolves ALL pending continuations before role flips to STANDBY (so nothing can still be registered by the time that path runs), but this is an implicit ordering invariant, not an asserted one. Flag for whoever next touches the STANDBY replicated-apply path: add a defensive comment/assertion there, or route it through the wrapper too, before relying on it further. Also: bug-1407 was believed closed at this point (70/70 across 3 runs) — see 1.5's note below, this was a false reading, not a real fix.
• 2.2: implemented apply_committed_entry(persist_t*, uint32_t wal_type, uint64_t lsn, const void* payload, uint32_t len) in persist.c/persist.h, matching design.md's signature. Routes WAL_JOB_SUBMIT through restore_job() (idempotent on duplicate jobid — no-op, Req 7.1), WAL_QUEUE_CTRL/WAL_WORKER_CTRL through their existing restore primitives, and fires each type's post-mutation hook exactly once, after mutation. Confirmed permanent, non-cosmetic deviation from design.md's Apply Seam wording: ALL nine job-state fact WAL types (WAL_JOB_{START,FINISH,FAIL,STOP,RESUME,UPDATE,RUSAGE,CKPT,PREASON}) route through the single guard-only apply_job_update_transition() core from 2.1 — NOT the type-specific apply_job_{start,finish,fail}_transition cores — and fire one generic HOOK_DETAIL_JOB_UPDATE hook, not the type-specific HOOK_DETAIL_JOB_START/_EXIT/_FAIL. Root cause, independently verified by two review rounds: the on-disk wire format for every one of these types (wal_jobstate_t {jobid, state}) genuinely only carries the resulting state, not the richer job_pidinfo_t/xce_jobfinish_t/job_failinfo_t payload the type-specific cores/hooks require — this is a pre-existing wire-format reality (apply_wal_entry already collapsed these the same way before this task), not a shortcut invented here. design.md's Apply Seam section and Fact Ingestion → Event Contract section currently describe per-type hook dispatch that this wire format cannot support as written — this is a real, standing design/reality conflict, not yet reconciled in design.md itself. No task in the current plan (2.3, 4.1, 4.2, 6.1, 6.2, 6.3) enriches the wire format to close this gap — confirmed by reading all of them; task 4.1 is exclusively about removing receipt-time mutation/hook-firing, not payload enrichment. Explicit blocking-style call-out for whoever implements 4.1/6.2/6.3: wiring this dispatch function into a live fact-apply path will permanently change every worker-reported job start/finish/fail notification to a generic UPDATE detail instead of the type-specific one, unless the wire format is enriched first or design.md is corrected to formally accept this substitution — verify this is still the accepted behavior before wiring it live, don't assume it'll be fixed by then. Took 3 review rounds to land: round 1 rejected two doc-comment defects (a citation blaming the wrong crash site, and a dangling pointer to task 4.1 as the fix location); round 2's fix corrected the crash site (real one is sched.c's sched_job_start_fun/_exit_fun/_fail_fun, which unconditionally dereference info->jobid before any NULL check) but fabricated a nonexistent function name (encode_JobPIDInfo_ptr) as supporting evidence; round 3 replaced it with a narrower, fully source-verified claim (apiserver.c's equivalent functions store the raw pointer into a notify union without dereferencing it at that call site — real, checked line-by-line). Lesson: when a reviewer catches a citation error, the safest fix is the narrowest claim you can personally verify by reading the actual code, not a broader-sounding replacement — round 2's attempt to explain "why the alternative is fine" reached for an unverified downstream mechanism and got caught fabricating it.
• 2.1: split mark_job_start/mark_job_finish/mark_job_fail/mark_job_update (job.c) into four hook-free cores (apply_job_{start,finish,fail,update}_transition, new job_transition_result_t enum in cluster.h) containing only the monotonic-rank guard + mutation, verified an unmodified lift line-by-line against pre-extraction code. Wrappers peek the same guard predicate to decide hook-firing, then delegate to the core — no unlock/relock gap between peek and delegated call, so no new race from the double-evaluation. mark_job_fail's pre-existing lack of a guard is preserved, not added or removed. New test_apply_seam.c (8 tests) calls the cores directly, bypassing on_job_* receipt handlers entirely — this is the form task 2.2 (dispatch function) and task 6.1 (apply pump) will call into. Non-blocking follow-up flagged by reviewer: the new doc-comments on apply_job_fail_transition/apply_job_update_transition overclaim a universal "caller holds RTTI_LOCK_WRITE" precondition — job_ctrl (job.c:396, pre-existing, untouched) calls mark_job_update/mark_job_fail under only RTTI_LOCK_READ. No new race (extraction added no unlock/relock), but whoever writes task 2.2/6.1 should not trust that comment's blanket claim — verify the actual lock held at each call site instead.
• 1.5: pure test-only close-out of task-group 1 — 3 new genuine-multi-thread tests added across test_commit_resolve.c/test_demotion_convergence.c/test_persist_truncate.c (real pthreads, not sequential calls dressed as concurrent): an admission-cap burst with a concurrent monitor thread, a mixed majority-ack/timeout-sweep burst proving bounded dispatch-thread usage, and a genuine two-real-thread ack-vs-sweep-thread race (closing the "single-thread white-box only" gap both task-1.3 review rounds had flagged as a nice-to-have). No production code touched. bug-1407 saga, definitively resolved: it was NEVER a source bug. 1.4's "closed, 70/70" and 1.5's initial "reopened, 10/10 failing" were BOTH real, correct, contradictory measurements of the exact same source commit (9b4b805) — the difference was a stale/corrupted incremental-build artifact in the shared build/ directory, which had been ninja -C build-rebuilt incrementally across all 5 tasks without ever a fresh meson setup. Proven by isolating each measurement in its own throwaway git worktree add + fresh meson setup/ninja: pre-feature commit and the exact failing commit both pass cleanly (0 failures) on a fresh build; only the main worktree's stale incremental build/ produced the failure. Fixed with rm -rf build && meson setup build --buildtype=debug -Db_sanitize=address && ninja -C build; bug-1407 is now genuinely CLOSED with high confidence. Lesson for every remaining task in this series: distrust a surprising/contradictory ninja -C build incremental result before concluding a source regression — rebuild clean (or isolate via a throwaway worktree) first. Never use git checkout <sha> -- . to inspect an old commit in place — it silently reverts working-tree files against current HEAD; use git worktree add <path> <sha> instead. Both lessons are now in .wolf/cerebrum.md Do-Not-Repeat and .wolf/buglog.json bug-1407.