myxce/.kiro/specs/scheduler-dispatch-commit/design.md
Rongsong Shen 8eea74909c
feat(scheduler-dispatch-commit): add durable WAL_JOB_DISPATCH fact type (task 1.2)
New wal_entry_type WAL_JOB_DISPATCH=12, reusing wal_jobstate_t verbatim
(_pad=0, absolute-value semantics), wired into both apply_wal_entry()'s
and apply_committed_entry()'s existing job-state fall-through groups per
Requirement 2.4. Restart-replay test writes only WAL_JOB_SUBMIT+
WAL_JOB_DISPATCH to a real on-disk WAL and confirms job.state==RUN after
a fresh persist_recover(), independent of WAL_JOB_START.

Review found apply_wal_entry() has zero production call sites today
(persist_recover() and standby catch-up both route through
apply_committed_entry() post the 2026-07-10 ha_apply_single_entry
refactor) -- design.md's stale characterization corrected; the dual
case-label registration itself is unaffected and correct.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-23 07:12:50 +00:00

46 KiB

Design Document

Overview

Purpose: Move a job's transition to a running state from worker-confirmation time to dispatch-decision-commit time, on both the scheduler and the controller, and build the dispatch-failure retry/terminal-resolution machinery this requires — closing bug-1624 (resource-in-use totals silently drop a dispatched-but-unconfirmed job at the next round boundary) and bug-1625 (the scheduler discards the controller's existing dispatch-failure notification).

Users: Cluster operators who rely on accurate live resource-in-use accounting and correct job status; scheduler/controller maintainers who need dispatch failures to resolve deterministically instead of leaking stale allocation state.

Impact: sched_commit_decision() (scheduler) and dispatch_a_job_() (controller) each gain a state transition alongside the resource-allocation commit they already perform. A new scheduler-side notify handler and a new controller-side classification step are added. No existing component is replaced; both integration points are extended in place.

Goals

  • A job's state becomes RUN at the moment its dispatch is committed, on both scheduler and controller, with no window where the two disagree.
  • sched_account_jobs()'s existing RUN-state gate — unmodified — naturally includes a dispatched job from the round it was dispatched.
  • A dispatch failure resolves deterministically: transient/environmental failures retry the job back to PEND (bounded), local/invalid-decision failures fail the job permanently. Both outcomes release the stale allocation/commitment data from the failed attempt, on both sides.
  • The worker's actual confirmed-start fact is still durably recorded, unchanged, for start_time and audit purposes.

Non-Goals

  • Changing simulation-mode's own dispatch/commit path — sched_commit_decision() is not called by the simulation driver at all (confirmed in Research Log); no code change is needed there.
  • Implementing preemption delivery or victim selection — scheduler-preemption owns that; this design only leaves its cross-reference note intact.
  • Changing scheduler-allocation's own allocation-plan logic — this design only changes when a job reaches RUN, not how a plan is chosen.
  • Redesigning the WAL entry header format or the raft-apply-decoupling deferred-apply mechanism — this design adds one new WAL fact type using the existing wal_jobstate_t shape and the existing synchronous-mutation discipline job.allocs already follows, not a new mechanism.

Boundary Commitments

This Spec Owns

  • The scheduler-side state transition inside sched_commit_decision()'s DISPATCH branch.
  • The controller-side state transition inside dispatch_a_job_(), gated on xce_startjob_req() success, correctly synchronized under RTTI_LOCK_WRITE, and the new WAL fact type recording it durably.
  • The controller-side dispatch-failure classification (retryable vs. terminal, including the bounded-retry check) and its two resolution paths.
  • The new scheduler-side NotifyData_data_jobDispatchFail handler (the actual fix for bug-1625) — a single, narrow case that applies JOB_STAT_PEND.

Out of Boundary

  • scheduler.c's sched_update()/sched_job_state_change() — confirmed dead in the live driver (Research Log); not modified, not reused, not removed (removal is a separate cleanup, not required for this feature to be correct).
  • core_events.c's XCE_CORE_EVENT_JOB_STATE_CHANGE case — already correct and already idempotent for a repeated RUN application; unmodified.
  • builtin_fcfs's picker hooks (on_dispatch, on_job_state_change) — unmodified; their existing idempotency is relied upon, not extended.
  • The limit plugin's own usage accounting — confirmed to not share bug-1624's class of gate (Research Log); not investigated further here.
  • mark_job_fail()'s own mutate-then-notify body and its existing HOOK_DETAIL_JOB_FAIL → schedFailJob chain — reused as-is for the terminal resolution path; not modified.
  • Any change to xcecli/REST API response shapes — job status already reflects job.state directly; no new field is added there.
  • Fixing bug-1626 at job_ctrl()'s own pre-existing (and unrelated to dispatch) call to mark_job_fail() (job.c:1083) — this design's new call sites correctly take RTTI_LOCK_WRITE themselves, but job_ctrl()'s own gap is a separate, pre-existing condition this spec does not fix as a drive-by patch.

Allowed Dependencies

  • sched_commit_decision() (scheduler.c) and sched_account_jobs()'s existing JOB_IS_RUN gate (scheduler.c) — read, not modified, except for the one new transition line.
  • mark_job_fail() (job.c) and its existing hook chain — called, not modified, under a caller-supplied RTTI_LOCK_WRITE (its own documented precondition).
  • notify_dispatch() (job.c) — called, not modified (its meaning narrows by convention, not by signature change).
  • persist_submit_job_fact_ex() (persist.c) and wal_jobstate_t (wal_types.h) — extended with one new WAL entry type, following the existing _pad-discriminated shape.
  • job_t's in-memory-only wrapper fields (xce/types.h) — extended with one new in-memory-only counter, following submit_lsn's existing precedent.
  • job_lock/job_unlock (src/lib/jobj.c) via the existing lock(j, RTTI_LOCK_WRITE)/unlock(j) calling convention already used correctly at most other job-mutation sites in job.c — this design's new mutation sites must follow the same convention, not the unlocked pattern dispatch_a_job_() and job_ctrl()'s existing mark_job_fail() call site currently use (bug-1626, pre-existing — see research.md's Addendum).

Revalidation Triggers

  • Any change to sched_account_jobs()'s RUN-state gate itself (not just what feeds it).
  • Any change to dispatch_a_job_()'s failure-point ordering or return codes — the classification table (Components, below) is keyed to today's exact failure points.
  • Any change to mark_job_fail()'s hook chain or to HOOK_DETAIL_JOB_STOP/HOOK_DETAIL_JOB_FAIL's wiring to sched.c's notify translation.
  • scheduler-preemption moving to design — its AC6/AC9 pending-revision note depends on this design's RUN-timing conclusion being final.
  • Any future fix to bug-1626 (the pre-existing job_ctrl()/mark_job_fail() unlocked call site, out of this spec's boundary) that changes mark_job_fail()'s locking contract itself, as opposed to fixing individual callers.

Architecture

Existing Architecture Analysis

Two independently-maintained job records exist for the same logical job: the scheduler's xce_sched_job_t (sched/types.h, decision-making working copy) and the controller's job_t/xce_job_t (xce/types.h, the durable, user-facing record). They are today synchronized by two one-directional channels: scheduler → controller decision publish (sched_commit_decision() → xcecli_jobdispatch()), and controller → scheduler notify (sched_handle_notify()'s switch, handler.c). This design adds a state transition to each side's own record at its own commit point, rather than introducing a third synchronization channel — the two sides reach the same conclusion (RUN) independently, from the same triggering event (the dispatch decision), with the notify channel used only for the asymmetric case where the controller's downstream attempt fails after the scheduler already committed.

A second, load-bearing discovery (Research Log): scheduler.c's notify-apply path (sched_update()/sched_job_state_change()) is dead code in the current driver — the live path is core_events.c's XCE_CORE_EVENT_JOB_STATE_CHANGE case, reached via xce_sched_driver_translate_pending_events(). All references below to "the existing RUN-confirmation path" mean this live path.

Architecture Pattern & Boundary Map

sequenceDiagram
    participant Picker as Scheduler picker/allocator
    participant SchedCommit as sched_commit_decision
    participant Ctrl as Controller dispatch_a_job_ (owns resolution + notify)
    participant Worker as Worker
    participant Notify as Controller notify (existing)
    participant SchedNotify as Scheduler sched_handle_notify

    Picker->>SchedCommit: DISPATCH decision
    SchedCommit->>SchedCommit: set job-accounts, job-allocs, state=RUN
    SchedCommit-->>Ctrl: publish decision
    Ctrl->>Worker: xce_startjob_req
    alt RPC succeeds
        Worker-->>Ctrl: ok
        Ctrl->>Ctrl: lock j write, commit job-allocs, state=RUN, append WAL_JOB_DISPATCH fact, unlock
        Note over Ctrl: worker later confirms actual start<br/>WAL_JOB_START still recorded separately, unchanged
    else RPC fails, transient
        Ctrl->>Ctrl: increment dispatch_fail_count, stay PEND
        Ctrl->>Notify: jobDispatchFail
        Notify->>SchedNotify: NotifyData_data_jobDispatchFail
        SchedNotify->>SchedNotify: apply JOB_STAT_PEND (new case)
    else RPC fails, terminal or retry bound exceeded
        Ctrl->>Ctrl: lock j write, mark_job_fail (existing), unlock
        Ctrl->>Notify: schedFailJob (existing, unmodified path)
        Notify->>SchedNotify: NotifyData_data_schedFailJob
        SchedNotify->>SchedNotify: apply JOB_STAT_EXIT (existing, unmodified)
    end

Architecture Integration:

  • Selected pattern: extend two existing, independent commit points rather than add a synchronization protocol. Reuses the existing asymmetric notify channel only for the failure case, where it is unavoidable (the scheduler cannot know the controller's downstream outcome any other way).
  • Domain/feature boundaries: scheduler-side accounting correctness and controller-side dispatch/failure handling remain separately owned, matching the existing component split; neither gains knowledge of the other's internals beyond the existing decision-publish/notify contracts.
  • Existing patterns preserved: sched_account_jobs()'s RUN-state gate, mark_job_fail()'s mutate-then-notify shape, wal_jobstate_t's _pad-discriminated WAL fact shape, job_t's in-memory-only wrapper-field convention.
  • New components rationale: a classification step and a bounded counter are new because no equivalent existed; a new WAL fact type is new because the transition now needs a durable record that job.allocs's own (non-durable) precedent doesn't provide.
  • Steering compliance: no controller self-termination path is introduced; the terminal-failure path stays entirely inside the existing "fail loud, mutate, notify" discipline mark_job_fail() already follows.

File Structure Plan

Modified Files

  • src/scheduler/src/scheduler.c — sched_commit_decision()'s DISPATCH branch gains job->state = JOB_STAT_RUN;, alongside the existing job->allocs/job->accounts assignment. No new field.
  • src/scheduler/src/handler.c — sched_handle_notify()'s switch gains one new case NotifyData_data_jobDispatchFail:, applying JOB_STAT_PEND through the existing XCE_SCHED_OP_JSTATE_CHANGE enqueue path (same shape as the adjacent schedExitJob/schedFailJob cases).
  • src/controller/job.c — two coupled changes:
    • dispatch_a_job_()'s success/failure handling is restructured: job.allocs's commit block (today unconditional, job.c:947-954, run regardless of xce_startjob_req()'s rc) moves inside an if (rc >= 0) branch. On success, the function takes lock(j, RTTI_LOCK_WRITE), commits job.allocs/job.state, appends the WAL_JOB_DISPATCH fact, then unlock(j) — closing bug-1626 for this call site as a direct consequence of restructuring it. On xce_startjob_req() failure (XCE_DISPATCH_FAIL_SITE_RPC_FAILURE, one of the six sites Controller Dispatch-Failure Classification enumerates), the function follows that component's same classify-resolve procedure. At each of the six post-lookup failure points, the function classifies and resolves inline (under its own lock(j, RTTI_LOCK_WRITE)): retryable (four sites, including RPC failure) → increment dispatch_fail_count, unlock(j), dereference(j) (closing bug-1627's leak on these four paths), call notify_dispatch(); terminal (two sites) → call mark_job_fail() under the write lock, unlock(j) after (these two already correctly dereference(j) today, unaffected by bug-1627).
    • handle_jobdispatch()'s loop drops its existing per-job notify_dispatch(c, ep, alloc->jobid, ...) calls (both the success-path and failure-path ones) — notification is now entirely owned by dispatch_a_job_(), preventing the double-notify race found in round 2 of this review (a terminal failure previously would have produced both jobDispatchFail and schedFailJob).
  • src/include/xce/types.h — xce_job_t's wrapper job_t (not the wire type itself) gains uint32_t dispatch_fail_count;, following submit_lsn's existing "in-memory only, reset on replay" precedent.
  • src/include/wal_types.h — one new wal_entry_type value, WAL_JOB_DISPATCH = 12, reusing the existing wal_jobstate_t{jobid, state, _pad=0} payload shape (the "legacy: state is absolute" discipline, matching job.allocs's own synchronous-commit precedent — see Data Models).
  • src/controller/persist.c — WAL_JOB_DISPATCH is added as a new case label to two existing fall-through groups: apply_wal_entry()'s (persist.c:3452-3466; currently has zero production call sites — see the Components section correction below) and apply_committed_entry()'s (the actual live-apply-and-recovery path, persist.c:3700-3706) — both already apply wal_jobstate_t.state as an absolute value for every type in their respective groups; no new function or case body.
  • src/controller/job.c (same file, no new header) — xce_classify_dispatch_fail(), xce_dispatch_fail_site_t, and xce_dispatch_fail_class_t are declared static/file-local next to dispatch_a_job_(), its only caller — not in a shared header, since nothing outside this file consumes them (see Controller Dispatch-Failure Classification's placement-correction note).

New/Modified Test Files

  • src/scheduler/testsuite/test_scheduler.c (or a new focused file) — sched_commit_decision() DISPATCH-branch RUN-transition.
  • src/scheduler/testsuite/test_handler.c (or equivalent) — new jobDispatchFail case applies JOB_STAT_PEND, frees stale accounts/allocs via the existing shared path.
  • src/controller/testsuite/test_job_dispatch.c (or equivalent existing dispatch test file) — gated commit on RPC success/failure for all seven failure returns (the jobid-lookup case's two variants unchanged, the six post-lookup cases newly classified); retryable vs. terminal classification; bounded-retry escalation; WAL fact durability across a simulated restart/replay.
  • HA soak (/xce-test-ha) — no new file; existing sequential/chaotic regression re-run per Requirement 5.4.

System Flows

See the sequence diagram above (Architecture Pattern & Boundary Map) for the end-to-end flow across both success and both failure branches. No additional flow diagrams are needed — the state-transition logic itself is a simple two-branch decision per side, not a multi-step process.

Requirements Traceability

Requirement Summary Components Interfaces Flows
1.1, 1.2, 1.3 Scheduler RUN transition at commit time Scheduler Dispatch Commit sched_commit_decision Sequence: SchedCommit branch
2.1, 2.3 Controller RUN transition at dispatch success Controller Dispatch Commit dispatch_a_job_ Sequence: RPC succeeds branch
2.2 Confirmed-start fact still recorded separately Controller Dispatch Commit (unmodified sibling) record_job_start_fact (unmodified) n/a
2.4 Durability of the new transition WAL Dispatch Fact persist_submit_job_fact_ex, WAL_JOB_DISPATCH apply cases (live + recovery) Sequence: RPC succeeds branch
3.1, 3.2 Scheduler processes dispatch-failure notify Scheduler Dispatch-Failure Handler sched_handle_notify new case Sequence: transient branch
4.1, 4.2 Retryable vs. terminal classification Controller Dispatch-Failure Classification classification table Sequence: both failure branches
4.3 Bounded retry count Controller Dispatch-Failure Classification dispatch_fail_count Sequence: transient branch
4.4, 4.5 Release stale allocation/commitment data Scheduler: Scheduler Dispatch-Failure Handler (retry), mark_job_fail (terminal, unmodified). Controller: satisfied vacuously — Controller Dispatch Commit never commits on any failure path, so there is nothing to release core_events.c's existing free-on-non-RUN logic (scheduler side) Sequence: both failure branches
5.1 Confirmed-start fact recorded unchanged Controller Dispatch Commit (unmodified sibling) n/a n/a
5.2 Simulation mode unaffected n/a (no simulation code touched) n/a Research Log
5.3, 5.4 Regression / HA soak n/a (test-only) n/a n/a

Components and Interfaces

Component Domain/Layer Intent Req Coverage Key Dependencies (P0/P1) Contracts
Scheduler Dispatch Commit Scheduler / scheduler.c Transition a job to RUN in the same operation that commits its dispatch decision 1.1-1.3 sched_get_job (P0), sched_charge_account (P0) State
Scheduler Dispatch-Failure Handler Scheduler / handler.c Apply PEND to a job whose dispatch was reported as retryably failed 3.1, 3.2, 4.4 XCE_SCHED_OP_JSTATE_CHANGE enqueue (P0), core_events.c's existing apply case (P0) Event
Controller Dispatch Commit Controller / job.c Transition a job to RUN, and durably record it, only once dispatch to the worker actually succeeds 2.1-2.4, 5.1 xce_startjob_req (P0), WAL Dispatch Fact (P0) State, Batch
Controller Dispatch-Failure Classification Controller / job.c Decide retryable-vs-terminal for a dispatch failure, including the bounded-retry escalation 4.1-4.3 job_t.dispatch_fail_count (P0), mark_job_fail (P0), notify_dispatch (P0) Service
WAL Dispatch Fact Controller / persist.c, wal_types.h Durable record of the dispatch-time RUN transition, replayed on recovery 2.4 persist_submit_job_fact_ex (P0), wal_jobstate_t (P0) Batch, State

Scheduler (scheduler.c, handler.c)

Scheduler Dispatch Commit

Field Detail
Intent Set job->state = JOB_STAT_RUN in the same operation that commits job->allocs/job->accounts
Requirements 1.1, 1.2, 1.3

Responsibilities & Constraints

  • Runs inside sched_commit_decision()'s existing XCE_DECISION_DISPATCH branch, after job->accounts is built and before sched_commit_job_alloc() charges it — ordering among these three lines is not load-bearing to each other, but all three must complete before this function returns.
  • Does not call plugin_job_state_change()/on_job_state_change — the picker is already notified of this dispatch via the existing, unchanged on_dispatch() call in sched_allocator(). Firing a second hook is unnecessary and was confirmed unneeded via builtin_fcfs_on_dispatch()'s own pendJobs-removal behavior (Research Log).
  • The later, now-redundant schedStartJob-triggered application of JOB_STAT_RUN (via the existing, unmodified core_events.c path) is a confirmed-safe no-op against an already-RUN job — no guard is added here to suppress it; suppressing it would be unnecessary defensive code against an already-idempotent path.

Contracts: State [x]

State Management
  • State model: job->state transitions PEND → RUN synchronously, in-process, no deferred-apply (the scheduler has no WAL/durability concept — xce_sched_job_t is a working copy, not a system of record).
  • Persistence & consistency: none — a scheduler restart already rebuilds xce_sched_job_t from a fresh controller push, which will already reflect whatever the controller's own (durable) state is by then.
  • Concurrency strategy: unchanged — sched_commit_decision() already runs single-threaded on the driver's decision-drain loop; no new locking is introduced.

Scheduler Dispatch-Failure Handler

Field Detail
Intent The actual fix for bug-1625: process NotifyData_data_jobDispatchFail instead of falling to default: -EINVAL
Requirements 3.1, 3.2, 4.4

Responsibilities & Constraints

  • New case NotifyData_data_jobDispatchFail: in sched_handle_notify()'s switch (handler.c), positioned alongside the existing schedStartJob/schedExitJob/schedFailJob cases.
  • Calls the same enqueue helper those cases already use (sched_job_state_change(sched, notify->data.job_dispatch_fail->jobid, JOB_STAT_PEND) — the handler.c-local queuing wrapper, not the dead scheduler.c mutator of the same name), so the actual state application flows through the live xce_sched_driver_translate_pending_events() → core_events.c path identically to every other notify-driven state change.
  • No new classification logic here — jobDispatchFail's meaning is narrowed (Research Log) to exclusively mean "return this job to pending"; the controller has already decided this is the right resolution before sending it.
  • The existing core_events.c XCE_CORE_EVENT_JOB_STATE_CHANGE case's !JOB_IS_RUN(new_state) branch frees job->accounts/job->allocs automatically — this component does not duplicate that logic (Requirement 4.4 is satisfied by the existing, unmodified free-on-non-RUN path, reused rather than reimplemented).

Contracts: Event [x]

Event Contract
  • Subscribed events: NotifyData_data_jobDispatchFail (existing capnp/notify type, already sent by notify_dispatch(), currently unhandled).
  • Ordering / delivery guarantees: same as every other notify — delivered in-order on the same connection, applied on the next round's xce_sched_driver_translate_pending_events() drain (same latency characteristics as schedStartJob/schedExitJob/schedFailJob today).

Controller (job.c, persist.c)

Controller Dispatch Commit

Field Detail
Intent Commit job.allocs and transition to RUN only once xce_startjob_req() succeeds; append a durable fact recording the transition
Requirements 2.1, 2.3, 2.4, 5.1

Responsibilities & Constraints

  • dispatch_a_job_()'s existing unconditional j->job.allocs = ... assignment (today placed after the xce_startjob_req() call but not gated on its return code) becomes conditional: only committed when rc >= 0. j->job.state = JOB_STAT_RUN is set in the same conditional block.
  • This ordering choice supersedes requirements.md 2.1's literal "before attempting to notify the worker" phrasing (Research Log: gating on RPC success removes an entire reversal-path category that gating on decision-receipt would require). The observable guarantee requirements.md 2.1/2.3 actually asks for — the transition and the allocation commit happen as one atomic operation, and status queries reflect RUN once that operation completes — is preserved exactly; only the specific instant within dispatch_a_job_() moves later, to right after a successful RPC instead of right before the RPC attempt.
  • On success, appends one WAL_JOB_DISPATCH fact (see WAL Dispatch Fact, below) recording {jobid, state=JOB_STAT_RUN}, in addition to the synchronous in-memory mutation — mirrors job.allocs's existing synchronous-commit style, not record_job_start_fact()'s deferred-apply style (Research Log: no same-job racing-fact concern exists for the first post-PEND transition).
  • Requirement 2.2's worker-confirmed-start fact (record_job_start_fact()/WAL_JOB_START) is entirely unmodified — it continues to fire later, from the existing worker-confirmation code path, and continues to set start_time.
  • Requirement 2.3 (status queries reflect RUN) requires no new code — job.state is read directly by every existing status/listing path today; committing it earlier is sufficient.

Contracts: State [x] / Batch [x]

State Management
  • State model: PEND → RUN, gated on RPC success; on RPC failure, state is untouched (still PEND — never entered RUN, so no reversal is needed, per the Research Log finding).
  • Persistence & consistency: the in-memory mutation and the WAL fact append happen back-to-back under lock(j, RTTI_LOCK_WRITE) (acquired explicitly by this component, held across both the job.allocs/job.state mutation and the persist_submit_job_fact_ex() call, released via unlock(j) immediately after) — _pad = 0 (legacy/absolute), matching wal_jobstate_t's documented convention for a mutation with no staleness window.
  • Concurrency strategy: corrects a pre-existing gap (bug-1626), does not merely reuse it. dispatch_a_job_()'s existing locking, traced directly (job.c:831-963), takes only RTTI_LOCK_READ on c->jobmgr/c->workermgr/the worker w — it never takes any lock on the job object j itself, despite j->job.allocs already being mutated today. RTTI_LOCK_READ is a genuine pthread-backed reader lock (my_rwlock_rlock(), confirmed in src/lib/jobj.c:708-725), not a liveness/reference guard, so this is a real gap, not a documentation inaccuracy. This component's mutation (and the pre-existing job.allocs mutation it sits beside, which this restructuring already touches) is brought under RTTI_LOCK_WRITE, matching the convention most other job-mutating call sites in job.c already correctly follow (e.g. job.c:768,1687,1795,2058). job_ctrl()'s own separate, unrelated call to mark_job_fail() (job.c:1083) has the same gap and is intentionally left unfixed here (Out of Boundary) — only this feature's own new/restructured call sites are corrected.
Batch / Job Contract
  • Trigger: successful xce_startjob_req() return within dispatch_a_job_().
  • Input / validation: jobid, target state (JOB_STAT_RUN) — no new validation beyond what dispatch_a_job_() already performs before reaching this point.
  • Output / destination: one WAL_JOB_DISPATCH WAL entry, replicated and applied through the existing commit/apply pipeline.
  • Idempotency & recovery: WAL_JOB_DISPATCH's case in both apply_committed_entry() (live) and apply_wal_entry() (recovery replay) applies state as an absolute value (matching every _pad == 0 job-state fact today) — replaying it any number of times is a no-op after the first, identical to how every other absolute job-state fact already behaves.

Controller Dispatch-Failure Classification

Field Detail
Intent Decide retryable vs. terminal for one of dispatch_a_job_()'s six post-lookup failure points, including the bounded-retry escalation, and route to the corresponding resolution mechanism — fully inside dispatch_a_job_(), not deferred to its caller
Requirements 4.1, 4.2, 4.3

Responsibilities & Constraints

  • A small classification table maps each of dispatch_a_job_()'s six post-lookup failure sites to one of two classes:
    • Retryable (transient/environmental): worker lookup failure, worker not found, worker-not-ready (E_XCE_BAD_WORKER), the xce_startjob_req() RPC call itself failing.
    • Terminal (local/invariant): invalid allocation (nc <= 0), queue-exec-info lookup failure.
    • The seventh failure point (jobid lookup failure, j never resolved) is not part of this table at all — see the dedicated bullet below.
  • Placement correction, found in the same round as the site-identity fix above: this table has no consumer outside job.c — nothing on the scheduler side classifies (that assumption was superseded when classification moved to be controller-authoritative). It is declared static in job.c, not in the shared src/include/xce/errnox.h — a shared header is for cross-component contracts, and this classification is an internal implementation detail of one function's restructuring.
  • xce_startjob_req()'s own RPC failure (job.c:956-960, the function's final failure return) is the sixth site, XCE_DISPATCH_FAIL_SITE_RPC_FAILURE — see Controller Dispatch Commit for the accompanying job.allocs-gating and dereference(j) fixes at that exact call site.
  • Ownership correction found during /kiro-validate-design round 2: classification and resolution happen inside dispatch_a_job_(), at each of its six post-lookup failure points, using the j/job_t* pointer already held there — not in its caller handle_jobdispatch(). Two independent reasons: (1) handle_jobdispatch() only has alloc->jobid after dispatch_a_job_() returns (alloc is freed immediately after), so resolving there would require a second key_find() lookup and reopen a TOCTOU gap between the two; (2) handle_jobdispatch()'s existing loop unconditionally calls notify_dispatch() for every dispatch_a_job_() return, success (code=0) or failure — notify_dispatch() itself always emits NotifyData_data_jobDispatchFail as the event type regardless of code (confirmed by reading its body, job.c:1108-1125). Left unchanged, a terminal failure would produce both a jobDispatchFail notify (from the existing unconditional call) and a schedFailJob notify (from mark_job_fail()'s hook chain) for the same event, racing to apply PEND and EXIT to the same job depending on arrival order.
  • Fix: handle_jobdispatch()'s existing per-job notify_dispatch() calls (both the success-path notify_dispatch(c, ep, alloc->jobid, 0) and the failure-path notify_dispatch(c, ep, alloc->jobid, rc1)) are removed from that loop. dispatch_a_job_() now owns notification as part of resolution: it calls notify_dispatch() itself, only on the retryable-failure path, and never on success or on a terminal failure (mark_job_fail()'s own hook chain is the sole notification for that case). handle_jobdispatch()'s loop becomes a thin dispatch-and-free loop with no notification logic of its own.
  • Every one of the six post-lookup failure points calls xce_classify_dispatch_fail(site) with its own xce_dispatch_fail_site_t identity (not the raw rc/errno value that point happened to return — see Service Interface for why) — classification is never implicit in which branch executed; the table is the single source of truth, actually called at every site, not merely documentation the branch structure happens to match. This also makes the function genuinely load-bearing (not dead weight the Unit Tests section would otherwise be testing in isolation).
  • Single critical section per failure point — deliberately structured to avoid an unlock/relock gap between the count check and the resolution it drives (a dispatch_fail_count read separated from its acting-upon by a lock release would be a fresh race, on top of the one this design already closes). At each of the six post-lookup failure points, inside dispatch_a_job_(), before that point's existing dereference(j)/return (or, for the four sites bug-1627 identifies, where that dereference(j) is being newly added):
    1. lock(j, RTTI_LOCK_WRITE).
    2. Call xce_classify_dispatch_fail(site) with this failure point's own xce_dispatch_fail_site_t value. If XCE_DISPATCH_FAIL_TERMINAL, skip to step 3's terminal branch immediately (no counter involved). If XCE_DISPATCH_FAIL_RETRYABLE, increment job.dispatch_fail_count and capture the resulting value into a local.
    3. Still under the same lock: for a retryable classification whose incremented count is within the configured bound (design default: 3 consecutive failures; exact value is a task-level config decision, not hardcoded here), nothing further needs the lock — this failure resolves retryable. For a retryable classification whose count exceeds the bound, or a classification that was terminal from step 2, this failure resolves terminal without releasing and reacquiring the lock — call mark_job_fail() directly here (already satisfying its RTTI_LOCK_WRITE-held precondition), with a job_failinfo_t describing the dispatch failure (jobid, code = the original error, detail = a short diagnostic string; .stage left at 0, which correctly routes apply_job_fail_transition()'s switch to its default: branch, job->job.state = JOB_STAT_EXIT — confirmed by reading that function directly; the two named .stage cases are for post-exec failures, not applicable to a job that never started).
    4. unlock(j), then dereference(j) (closing bug-1627's leak on the four failure points where it exists today, found during rounds 3-4 of this review — the other two, nc <= 0 and get_queue_exec() failure, already correctly dereference(j), unaffected by that bug — this step is uniform across all six regardless).
    5. Only now, entirely outside the lock: if step 3 resolved retryable, call notify_dispatch(c, ep, jobid, rc) — now the only caller of this function for the dispatch-failure case, so jobDispatchFail unambiguously means "retry." If step 3 resolved terminal (whichever of the two ways it got there), notify_dispatch() is not called — mark_job_fail()'s hook chain (already fired, under the lock, in step 3) is the sole notification.
  • job.dispatch_fail_count is reset to 0 by Controller Dispatch Commit's own success path (above, under that component's own RTTI_LOCK_WRITE) — a fresh successful dispatch clears any prior failure history for that job.
  • How the controller side satisfies Requirement 4.4/4.5's "release stale allocation/commitment data": not by cleaning up after a failed commit, but by never committing in the first place — job.allocs/job.state are only ever written inside the rc >= 0 success branch (Controller Dispatch Commit), so on any of the six classified failure paths there is nothing to release; the requirement is satisfied vacuously by construction, not by an explicit cleanup step. This is the direct benefit of the RPC-gated commit ordering decision in research.md.
  • dispatch_a_job_()'s earliest failure point (jobid lookup failure — two code variants, key_find() itself failing or returning a NULL j, before j is ever resolved) is a distinct case outside this table entirely: there is no job_t* to mark failed or count against. This is not a job-state resolution problem, it is "the job does not exist" — the function's existing return rc/return -errno is sufficient, unchanged. Requirement 4's classification and this component's locking/counting/notification logic apply only to the six failure points reached after j is successfully resolved and referenced.

Contracts: Service [x]

Service Interface
/*
 * Classifies one of dispatch_a_job_()'s six post-lookup failure points as
 * retryable or terminal. Deliberately keyed on WHICH failure point was hit
 * (an explicit site identity), not on the raw errno-style `int rc` that
 * point returns -- key_find() (workermgr) and get_queue_exec() are two
 * distinct, independently-implemented failure sources that are not
 * guaranteed to return disjoint error codes (e.g. both could plausibly
 * return a generic -ENOENT for unrelated reasons), so classifying by rc
 * value alone would be ambiguous whenever two sites happen to collide.
 * Does not itself mutate job state, take any lock, or send notifications --
 * called from within dispatch_a_job_() itself, at each site, which uses the
 * result to choose between notify_dispatch() (retry) and mark_job_fail()
 * (terminal).
 */
typedef enum {
    XCE_DISPATCH_FAIL_SITE_WORKER_LOOKUP    = 0, /* workermgr key_find() rc<0 */
    XCE_DISPATCH_FAIL_SITE_WORKER_NULL      = 1, /* worker not found (w==NULL) */
    XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY = 2, /* E_XCE_BAD_WORKER */
    XCE_DISPATCH_FAIL_SITE_INVALID_ALLOC    = 3, /* nc <= 0 */
    XCE_DISPATCH_FAIL_SITE_QUEUE_EXEC       = 4, /* get_queue_exec() failure */
    XCE_DISPATCH_FAIL_SITE_RPC_FAILURE      = 5, /* xce_startjob_req() rc<0 */
} xce_dispatch_fail_site_t;

typedef enum {
    XCE_DISPATCH_FAIL_RETRYABLE = 0,
    XCE_DISPATCH_FAIL_TERMINAL  = 1,
} xce_dispatch_fail_class_t;

xce_dispatch_fail_class_t xce_classify_dispatch_fail(xce_dispatch_fail_site_t site);
  • Preconditions: site is one of the six named enumerators. Corrected during task 1.1 implementation review: a default-less switch does NOT compel a compile error in plain C without -Werror (this project sets none) — it only emits a non-fatal -Wreturn-type warning and falls through to undefined behavior on an unhandled value. The actual implementation instead follows this codebase's established fail-loud convention (matching persist.c's wal_classify(), persist.c:6030-6037): a default: arm that ERROR-logs and returns a safe XCE_DISPATCH_FAIL_TERMINAL, with assert(0 && ...) gated to #ifdef XCE_TESTING only, so a future unhandled site fails loud in test builds and degrades safely (never silently misclassifies as retryable) in production.
  • Postconditions: pure function, no side effects; the caller separately applies the bounded-retry override. WORKER_LOOKUP, WORKER_NULL, WORKER_NOT_READY, RPC_FAILURE classify XCE_DISPATCH_FAIL_RETRYABLE; INVALID_ALLOC, QUEUE_EXEC classify XCE_DISPATCH_FAIL_TERMINAL.
  • Invariants: the same site always classifies the same way — no hidden state, no time-dependence, no dependence on the specific rc/errno value that accompanied it.

WAL Dispatch Fact (persist.c, wal_types.h)

WAL Dispatch Fact

Field Detail
Intent Durable record of the dispatch-time RUN transition, so a controller failover recovers the same state a live run would have reached
Requirements 2.4

Responsibilities & Constraints

  • New wal_entry_type value WAL_JOB_DISPATCH = 12 (next free value in the job-fact range, after WAL_JOB_ARRAY_SUBMIT = 11).
  • Reuses the existing wal_jobstate_t{jobid, state, _pad} payload verbatim — no new struct. _pad is always 0 for this type (the "legacy: state is absolute value" convention wal_jobstate_t's own header comment documents), matching the synchronous, non-deferred-apply commit style Controller Dispatch Commit uses.
  • Two separate functions, both needing the new case — found by tracing precisely rather than assuming a single switch: apply_wal_entry() (persist.c:3375+) has its own fall-through group at persist.c:3452-3466 (WAL_JOB_START through WAL_JOB_RUSAGE, including WAL_JOB_FAIL — applies every type as an absolute overwrite, unconditionally). apply_committed_entry() (persist.c:3570+, the live path invoked immediately when a WAL entry commits during normal operation — the one raft-apply-decoupling actually introduced the _pad-discriminated routing for) has its own separate fall-through group at persist.c:3700-3706 (WAL_JOB_START, WAL_JOB_FINISH, WAL_JOB_STOP, WAL_JOB_RESUME, WAL_JOB_RUSAGE, WAL_JOB_CKPT, WAL_JOB_PREASON — deliberately excluding WAL_JOB_FAIL/WAL_JOB_UPDATE, which get their own _pad-aware handling immediately after). That group's own comment states the exact rationale this design already relies on: these types' payloads are "deterministic/state-independent... an absolute state baked at receipt time never goes stale" — WAL_JOB_DISPATCH fits this group by the same reasoning (Research Log: no same-job racing-fact concern for the first post-PEND transition). WAL_JOB_DISPATCH joins both fall-through lists — no new apply-time function or standalone case body is introduced in either. Corrected during task 1.2 implementation review: apply_wal_entry() is NOT actually "the recovery/replay-time path" in the current architecture — persist_recover()'s real boot-time replay (replay_wal()) and the standby catch-up path (post the 2026-07-10 ha_apply_single_entry refactor) both call apply_committed_entry() exclusively; apply_wal_entry() has zero production call sites today. Adding the case to both functions is still correct (matches every sibling WAL_JOB_* type's identical dual-registration, harmless dead-code parity, and costs nothing if apply_wal_entry() is ever revived) — only the prose description of what apply_wal_entry() currently is was wrong.
  • Does not carry or restore job.allocs/job.accounts — those remain non-durable in-memory fields, unchanged from today's behavior (Boundary: Out of Boundary does not list a new durability guarantee for allocation data itself, only for the state transition).

Contracts: Batch [x] / State [x]

Batch / Job Contract
  • Trigger: Controller Dispatch Commit's success path.
  • Input / validation: jobid, state = JOB_STAT_RUN.
  • Output / destination: standard WAL append → replication → commit pipeline, identical to every other job-state fact.
  • Idempotency & recovery: absolute-value apply, safe to replay any number of times.

Data Models

Domain Model

  • No new domain entities. job_t.dispatch_fail_count (via xce_job_t's wrapper, not the wire type) is a new scalar field on an existing entity, not a new aggregate. This counter is controller-side only — the scheduler has no equivalent field, since only the controller's copy is read for classification (see Controller Dispatch-Failure Classification); a scheduler-side mirror would be dead weight with no consumer.

Logical Data Model

  • job_t (xce/types.h, the {xce_job_t job; uint64_t submit_lsn;} wrapper — not xce_job_t itself): + uint32_t dispatch_fail_count — in-memory only, following submit_lsn's exact precedent ("recomputed... never serialized"). A controller failover resets this to 0, which is a safe, conservative degradation (more retries allowed post-failover, never fewer, and never a misrouted terminal/retry decision).
  • wal_entry_type (wal_types.h): + WAL_JOB_DISPATCH = 12, payload = existing wal_jobstate_t.

Error Handling

Error Strategy

Two independent failure surfaces, each already governed by an existing discipline this design does not change: the scheduler-side notify handler follows the same -ENOENT/-EINVAL conventions every sibling case in sched_handle_notify() already uses; the controller-side classification is keyed on an exhaustive, compiler-checked failure-site enum (no default/unrecognized branch is reachable) rather than on a possibly-ambiguous raw return code.

Error Categories and Responses

  • Unrecognized dispatch-fail notify jobid (scheduler side): same as every existing case — sched_job_state_change()'s enqueue wrapper does not validate jobid existence at enqueue time; the live apply path's sched_get_job() == NULL branch (already present in core_events.c) logs a WARN and no-ops, unchanged.
  • A new post-lookup failure point added to dispatch_a_job_() in the future without updating the classification (controller side): fails to compile — xce_classify_dispatch_fail()'s switch over xce_dispatch_fail_site_t has no default case, so a new enumerator with no matching case is a compiler error, not a silent runtime default.
  • Bounded-retry exhaustion: not an error path — a deliberate, expected transition to terminal, using the exact same mark_job_fail() call as any other terminal classification.

Monitoring

No new logging infrastructure — mark_job_fail()'s existing hook-fired logging and sched_handle_notify()'s existing per-case TRACE/WARN conventions already cover both new code paths; only the log lines' content changes to reference the new case names.

Testing Strategy

Unit Tests

  • sched_commit_decision()'s DISPATCH branch sets job->state == JOB_STAT_RUN, in addition to the existing accounts/allocs assertions.
  • sched_handle_notify()'s new jobDispatchFail case enqueues an XCE_SCHED_OP_JSTATE_CHANGE targeting JOB_STAT_PEND; a follow-up drain via xce_sched_driver_translate_pending_events()/core_events.c confirms job->accounts/job->allocs are freed (reusing the existing free-on-non-RUN assertion pattern from other state-change tests).
  • dispatch_a_job_(): for each of the seven failure returns (jobid lookup's two variants, plus the six post-lookup sites), confirm job.state remains PEND and job.allocs is not committed; for a successful xce_startjob_req(), confirm both are committed together with one WAL_JOB_DISPATCH fact appended. For the four retryable-class paths, additionally assert j's reference count returns to its pre-call value (regression test for bug-1627's leak).
  • xce_classify_dispatch_fail(): table-driven test covering all six xce_dispatch_fail_site_t enumerators against their documented classification (four retryable, two terminal).
  • Bounded-retry escalation: three consecutive retryable failures for the same job route to notify_dispatch(); the fourth (exceeding the design-default bound of 3) routes to mark_job_fail() instead.

Integration Tests

  • End-to-end: scheduler commits a DISPATCH decision → controller processes it successfully → scheduler's sched_account_jobs() includes the job's resource consumption from that same round onward, across a simulated multi-round gap before the worker's actual confirmation arrives (this is bug-1624's regression test).
  • End-to-end retryable failure: controller's xce_startjob_req() fails (simulated worker-unreachable) → scheduler receives jobDispatchFail → job returns to PEND → scheduler's picker reconsiders it in a later round.
  • End-to-end terminal failure: controller's jobid/allocation lookup fails → mark_job_fail() fires → scheduler receives the existing schedFailJob notify (unmodified path) → job resolves to EXIT with accounts/allocs freed. Assert the scheduler does not also receive a jobDispatchFail notify for this same failure (regression test for the round-2 double-notify finding) — handle_jobdispatch()'s loop must not call notify_dispatch() for a failure dispatch_a_job_() already resolved as terminal.
  • WAL durability: append a WAL_JOB_DISPATCH fact, simulate a controller restart/replay, confirm job.state == JOB_STAT_RUN is recovered without requiring the (separate, unmodified) WAL_JOB_START fact to also be present.

Performance/Load

  • Not applicable — this feature adds O(1) work per dispatch decision and per notify; no new per-round scan or O(n) pass is introduced (sched_account_jobs() itself is unmodified).

Supporting References

  • .kiro/specs/scheduler-dispatch-commit/research.md — full discovery findings, the three design decisions this document builds on (dead-code path identification, RPC-gated commit ordering, controller-authoritative classification), and risk mitigations.