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

Research & Design Decisions

Summary

• Feature: raft-apply-decoupling
• Discovery Scope: Complex Integration (extension of a live, fast-moving HA/Raft subsystem — light discovery upgraded to comprehensive verification because the source brief (brief.md, 2026-06-24/25) is 9 days old relative to the code and several sibling specs land in the exact same seam)
• Key Findings:
  • bug-1306 (commit 15be5d9, 2026-06-30) already extracted the WAL-append + majority-commit barrier out of persist_job_hook_fun for job submit, ahead of this spec's formal implementation — Requirement 1 is partially pre-landed for the WAIT path.
  • The in-memory job-submit mutation (create_job()) still runs before the commit barrier (persist_submit_wal_and_commit()), and a failed/timed-out commit does not roll it back — the exact optimistic-divergence window Requirement 2 targets is still live today, just narrowed by the bug-1306 fix (which only fixed the reply-ack race, not the apply-ordering).
  • ha-term-safety is fully done (20/20 tasks); on_commit_advanced / raft_advance_commit are wired and live on the replication-ACK path (advisory-only, Option A) — no conflict with this spec's keystone.
  • raft-drive-async already built vtask_enq_classified + a slow-lane executor + admission classes (VTASK_LIVENESS / VTASK_LIVE_BLOCK / VTASK_REQ_BLOCK) — this closes the "no general non-IO task-submit path" gap the brief flagged as missing, so the keystone conversion can reuse it instead of building a new primitive.
  • Online-apply and replay-apply are not unified today: job submit online uses raw create_job(); replay uses restore_job() (decode + idempotent -EEXIST guard + LSN tag). Fact state transitions online go through guarded mark_job_start/finish/fail/update() (monotonic-rank checks); replay applies facts via restore_job_state(), a raw j->job.state = state overwrite with no guard at all. This is concrete, evidenced proof of the divergence Requirement 3/7 exist to close.
  • WAL_MODE_RETAIN facts already register into the commit engine (commit_submit(ce, lsn, COMMIT_MODE_RETAIN, 0)) for durability bookkeeping, but nothing today reacts to a RETAIN entry's commit — the state mutation already happened synchronously at receipt, before the WAL append. The "apply pump" driven off commit-advance (Requirement 5/6) does not exist in any form yet.
  • raft-sim-conformance's harness structurally enforces apply-only-after-commit already (it calls raft_advance_commit, then applies via on_commit_advanced, then checks State Machine Safety) — Requirement 10's conformance assertions mostly compose with existing harness structure rather than requiring new harness machinery.

Research Log

Current submit/hook path (job.c, persist.c)

• Context: Brief claimed persist_job_hook_fun does WAL-append + commit-wait inside an AFTER_EVENT hook (persist.c:2532/2198, 2026-06-24 citation).
• Sources Consulted: src/controller/job.c:560-662 (handle_jobsubmit), src/controller/persist.c:3392-3478 (persist_submit_wal_and_commit, persist_job_hook_fun), git log (15be5d9).
• Findings:
  • handle_jobsubmit (job.c:560): BEFORE_EVENT hook (validation) → job_submit() → create_job() (in-memory insert into c->jobmgr, job.c:583) → explicit persist_submit_wal_and_commit() (job.c:626, persist.c:3406) reading the commit verdict directly → on failure, reply error and return (job.c:627-637) — the already-created job object is not removed from c->jobmgr → on success, AFTER_EVENT hook fires as notification-only (job.c:643, persist.c:3466-3478 confirms the WAIT branch inside persist_job_hook_fun is now a no-op) → reply success.
  • persist_job_hook_fun's WAL_MODE_WAIT branch is dead code by design (returns 0 unconditionally, persist.c:3466-3478) — kept as a documented no-op, not removed, so hook registration order doesn't matter for job submit anymore.
• Implications: Requirement 1 (extract raft step from hook) is done for the job-submit WAIT path. Requirement 2 (apply-after-commit ordering) is not done — create_job() still runs before the barrier, and there is no rollback on commit failure. Design must move the job object construction itself (not just a secondary mutation) into the post-commit apply step.

Commit engine and ha-term-safety completion (commit.c, ha.c)

• Context: The keystone prerequisite (repl_wait_majority_ack blocking-to-event-driven conversion) is claimed as this spec's first-landed item; needed to confirm nothing else already did it and that ha-term-safety's concurrent work on the same seam doesn't conflict.
• Sources Consulted: src/controller/repl.c:533-558, src/controller/commit.c:195-409, src/controller/ha.c:1680,1776,1829-1831,3764, .kiro/specs/ha-term-safety/tasks.md.
• Findings:
  • repl_wait_majority_ack (repl.c:557) unconditionally delegates to commit_submit(ce, lsn, COMMIT_MODE_WAIT, timeout_ms) (commit.c:358), which blocks via pthread_cond_timedwait (commit.c:409) under the COMMIT_WAIT_RESERVED admission cap. Unchanged — still a pooled-worker blocking wait, 500ms default timeout.
  • ha-term-safety: all 20 tasks done. on_commit_advanced (ha.c:1776) is wired and live, firing from raft_advance_commit (raft.c call site ha.c:3764) on the replication-ACK path; it feeds commit_set_raft_commit_index() as an advisory cap only (Option A — the durable-majority barrier remains authoritative; the library commit index does not gate anything today).
• Implications: No conflict — ha-term-safety supplies the library-side commit-index plumbing this spec's apply pump can key off of (on_commit_advanced), but the blocking-wait-to-resolve conversion itself is untouched and squarely this spec's keystone to build.

Reusable async primitives from raft-drive-async (vtaskmgr.c)

• Context: The brief identified a gap — no general (non-IO) task-submit path exists — as an API prerequisite for the keystone's async continuations.
• Sources Consulted: .kiro/specs/raft-drive-async/tasks.md, src/controller/vtaskmgr.c:467,497,528.
• Findings: Task 1.2 (slow-lane executor, non-blocking submit) and Task 1.6 (vtask_class_t + vtask_enq_classified, admission-gated by class: VTASK_LIVENESS / VTASK_LIVE_BLOCK / VTASK_REQ_BLOCK) are both done and live (default-on since 2026-07-02, bug-1376). vtask_enq_classified(mgr, task, class) is a general, non-packet-bound submit path.
• Implications (Build vs. Adopt): Adopt vtask_enq_classified with a VTASK_REQ_BLOCK-equivalent class for the resolved-commit continuation (reply dispatch / apply trigger) instead of building a new submission primitive. This directly resolves the brief's "API gap" concern at zero new-code cost.

Restart replay and the apply-seam divergence (persist.c, job.c)

• Context: Requirement 3 requires one shared apply_committed_entry() seam for online and replay; needed to establish exactly how much divergence exists today.
• Sources Consulted: src/controller/persist.c:2452-2562 (apply_wal_entry), src/controller/persist.c:2667-2723 (replay_wal CRC verification), src/controller/job.c:44-177 (create_job, restore_job, restore_job_state), src/controller/job.c:664-774 (mark_job_start, mark_job_finish).
• Findings:
  • apply_wal_entry dispatches WAL_JOB_SUBMIT → restore_job() (persist.c:2505) and WAL_JOB_{START,FINISH,FAIL,...} → restore_job_state() (persist.c:2520).
  • restore_job() (job.c:96) decodes the Cap'n Proto payload, checks for an existing job by jobid (idempotent -EEXIST guard, job.c:117-130), and calls the same create_job() (job.c:44, thin wrapper over makeJob) that the online submit path uses. So submit-side apply is half-unified: replay already funnels through a shared primitive; online does not (it calls create_job() directly, skipping the idempotency guard and LSN tagging that restore_job() provides).
  • restore_job_state() (job.c:164) is a raw j->job.state = (int)state overwrite — no job_state_is_backward/job_state_is_terminal check at all. The online path's mark_job_start/mark_job_finish/mark_job_update (job.c:664-850) do carry those monotonic-rank guards. This is a genuine, currently-live idempotency gap: a duplicate or out-of-order fact entry replayed today (segment-boundary restart replay, or a future retained-fact retry) can regress job state, because replay's mutation path bypasses the same guard the online path enforces.
• Implications: apply_committed_entry() should standardize on restore_job() for submits (already idempotent, already reusable) and on a guarded state-transition function for facts — meaning mark_job_start/finish/fail/update's guard logic must become reachable from both the post-commit online path and replay, and restore_job_state() must stop being a second, weaker mutation path. This is a Build-vs-Adopt case: adopt the existing guards, relocate their call site.

raft-sim-conformance harness readiness

• Context: Requirement 10 needs regression coverage for apply-ordering; checked whether the existing deterministic simulator can carry these assertions without new harness machinery.
• Sources Consulted: .kiro/specs/raft-sim-conformance/{spec.json,requirements.md,design.md}.
• Findings: The harness already asserts Election Safety, Log Matching, Leader Completeness, and State Machine Safety, and its commit/apply loop is structured as raft_advance_commit → on_commit_advanced → apply → State-Machine-Safety check — i.e., it is already apply-after-commit by construction.
• Implications: New assertions (no-reaction-before-commit, idempotent re-apply) are additive checks layered on the existing harness loop, not a new harness. The live-cluster kill-mid-fact-stream scenario (Requirement 10.4) is new and belongs to the /xce-test-ha-style soak tooling, not the simulator.

Architecture Pattern Evaluation

Option	Description	Strengths	Risks / Limitations	Notes
Single apply_committed_entry() dispatch function, keyed on WAL type	One function, switch on wal_type, called from both the post-commit online path and apply_wal_entry()	Matches steering's "library-owned, thin-adapter" pattern already used for raft callbacks; smallest change that satisfies Req 3	Must not become a god-function — keep per-type logic in the existing restore_*/mark_job_* helpers, apply_committed_entry() only dispatches	Selected
Separate apply seams per entry type (no unifying dispatcher)	Each WAL type keeps its own online-vs-replay pair, just guard-aligned	Less code to touch per type	Does not satisfy Req 3's "same seam" requirement literally; re-invites future divergence	Rejected
Event-sourced full replay-log-as-truth (rebuild all reactions from log on every apply)	Maximal purity	Over-engineered — Design Risk A in facts-in-raft.md already concluded reactions must NOT be replayed, only state; resync (existing) is the re-derivation mechanism	Rejected (already settled in brief.md)

Design Decisions

Decision: Keystone conversion reuses the existing per-LSN commit registry and vtask_enq_classified, not a new registry or submit path

• Context: repl_wait_majority_ack → commit_submit(..., COMMIT_MODE_WAIT, ...) blocks a pooled worker on pthread_cond_timedwait. Converting to event-driven resolve() needs (a) a way to register a continuation instead of parking a thread, and (b) a way to run that continuation without new infrastructure.
• Alternatives Considered:
  1. Build a new pending-request registry alongside the commit engine's existing per-LSN structure.
  2. Reuse the commit engine's existing per-LSN structure (ce), replacing cond_broadcast with a resolve(lsn, outcome) call that invokes a registered continuation.
• Selected Approach: (2). commit_submit keeps its per-LSN bookkeeping; COMMIT_MODE_WAIT callers register a continuation (function pointer + context) instead of blocking; commit_advance_locked's existing cond_broadcast call site becomes the single place that walks newly-committed/newly-failed LSNs and invokes resolve(), which dispatches the continuation via vtask_enq_classified (raft-drive-async) rather than a bespoke thread hop.
• Rationale: No new concurrent data structure to get wrong; reuses the already-verified admission-cap discipline (COMMIT_WAIT_RESERVED / VTASK_* classes) instead of inventing a second one.
• Trade-offs: The commit engine's internal contract changes from "callers block on ce->cond" to "callers register a callback" — every existing COMMIT_MODE_WAIT call site (job submit today; future command types) must be converted together, not incrementally, or the engine would need to support both calling conventions simultaneously.
• Follow-up: Verify during implementation that commit_advance_locked's lock-ordering discipline (documented at persist.c:3347-3350: never hold p->lock across ce->lock) still holds once resolution dispatches into vtask_enq_classified from inside the commit-advance path.

Decision: apply_committed_entry() standardizes on restore_job() / a guard-carrying state-transition function, not new mutation primitives

• Context: See Research Log finding above — replay already has an idempotent submit primitive (restore_job); online submit doesn't use it. Online facts have a guarded transition primitive (mark_job_*); replay doesn't use it.
• Alternatives Considered:
  1. Write new, from-scratch apply functions for both submit and facts.
  2. Adopt restore_job() as-is for submit; extract the guard logic embedded in mark_job_start/finish/fail/update into a form callable from both the post-commit path and apply_wal_entry, retiring restore_job_state()'s unguarded overwrite.
• Selected Approach: (2).
• Rationale: Both primitives already carry the exact idempotency semantics Requirement 7 asks for; the gap is call-site wiring, not missing logic. Writing new functions would duplicate logic that already exists and is already unit-tested.
• Trade-offs: mark_job_start/finish/fail/update today fire their own BEFORE_EVENT/AFTER_EVENT hooks inline (job.c:680-681, 699-700) as part of the online receipt path — that hook-firing must move to occur only when these functions are invoked from the post-commit apply step, not from the (now durable-append-only) receipt handler, or the ordering guarantee (Req 5) would be violated. This is a real refactor, not a pure reuse.
• Follow-up: Confirm no other caller of restore_job_state() (grep found only apply_wal_entry) depends on its current unguarded-overwrite behavior before retiring it.

Decision: egress.c's job_submit_committed() watermark gate becomes redundant for job submit once Req 2 lands, but is not removed by this spec

• Context: Requirement 4.3 says downstream consumers should not need to re-check a commit watermark once notifications only fire post-commit. job_submit_committed() (egress.c:47) exists precisely because today a job is visible in c->jobmgr before it's committed.
• Selected Approach: Design leaves job_submit_committed() in place as a defense-in-depth predicate (it becomes a tautology once Req 2 lands — a job is never in jobmgr before commit — so it's harmless to keep), and records its removal as a documented candidate cleanup, not an in-scope deletion.
• Rationale: Removing it requires confirming every caller of the scheduler-export path relies solely on this gate and not on some other assumption — that audit is a distraction from the core ordering fix and belongs in a follow-up cleanup pass, matching the Simplification lens's guidance not to over-scope "just in case."
• Follow-up: Revalidation trigger — if a future spec removes job_submit_committed(), it must re-verify no path still relies on it as a correctness gate rather than a no-op.

Risks & Mitigations

• Risk: Converting every COMMIT_MODE_WAIT call site to the continuation-based API in one change is a wide, mechanically risky diff (touches the commit engine's core contract). — Mitigation: today there is exactly one COMMIT_MODE_WAIT call site (repl_wait_majority_ack, used only by job submit); the conversion is narrow in practice even though the contract change is conceptually broad.
• Risk: Relocating mark_job_*'s hook-firing out of the receipt-time handler could silently change BEFORE_EVENT hook timing/semantics for subscribers that assume BEFORE fires at receipt. — Mitigation: audit all HOOK_DETAIL_JOB_START/_EXIT/etc. subscribers before relocating; BEFORE_EVENT for facts should fire at receipt (validation), only the AFTER_EVENT + state mutation defers to apply.
• Risk: worker-rejoin-resync (hard dependency for full fact-reaction correctness per facts-in-raft.md Design Risk A) does not exist yet. — Mitigation: accepted, documented gap, consistent with prior specs (ha-consistency-invariants, ha-commit-barrier-rollout) that already ship with this same accepted gap.
• Risk: The keystone conversion changes lock-ordering-sensitive code in the commit engine that several other specs depend on (ha-commit-durability, ha-term-safety). — Mitigation: preserve the documented lock-ordering rule (never hold p->lock across ce->lock) as an explicit invariant in design and tests; run the full existing HA soak suite (Req 11) before/after.

References

• .kiro/specs/raft-apply-decoupling/brief.md — original discovery brief (2026-06-24/25)
• .kiro/specs/raft-apply-decoupling/facts-in-raft.md — facts-through-raft candidate plan, D1-D3 decisions, keystone rationale
• .kiro/specs/ha-consistency-invariants/requirements.md criterion 2.9 — Facts-Apply invariant, ownership assignment
• .kiro/specs/ha-term-safety/tasks.md — confirms full completion, no conflict
• .kiro/specs/raft-drive-async/tasks.md — confirms vtask_enq_classified / slow-lane availability
• .kiro/specs/raft-sim-conformance/{requirements.md,design.md} — existing conformance harness structure
• Commit 15be5d9 — bug-1306 fix, partial pre-landing of Requirement 1