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>
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_timeand 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-preemptionowns 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_tshape and the existing synchronous-mutation disciplinejob.allocsalready 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 onxce_startjob_req()success, correctly synchronized underRTTI_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_jobDispatchFailhandler (the actual fix for bug-1625) — a single, narrow case that appliesJOB_STAT_PEND.
Out of Boundary
scheduler.c'ssched_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'sXCE_CORE_EVENT_JOB_STATE_CHANGEcase — 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
limitplugin'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 existingHOOK_DETAIL_JOB_FAIL→schedFailJobchain — reused as-is for the terminal resolution path; not modified.- Any change to
xcecli/REST API response shapes — job status already reflectsjob.statedirectly; no new field is added there. - Fixing bug-1626 at
job_ctrl()'s own pre-existing (and unrelated to dispatch) call tomark_job_fail()(job.c:1083) — this design's new call sites correctly takeRTTI_LOCK_WRITEthemselves, butjob_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) andsched_account_jobs()'s existingJOB_IS_RUNgate (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-suppliedRTTI_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) andwal_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, followingsubmit_lsn's existing precedent.job_lock/job_unlock(src/lib/jobj.c) via the existinglock(j, RTTI_LOCK_WRITE)/unlock(j)calling convention already used correctly at most other job-mutation sites injob.c— this design's new mutation sites must follow the same convention, not the unlocked patterndispatch_a_job_()andjob_ctrl()'s existingmark_job_fail()call site currently use (bug-1626, pre-existing — seeresearch.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 toHOOK_DETAIL_JOB_STOP/HOOK_DETAIL_JOB_FAIL's wiring tosched.c's notify translation. scheduler-preemptionmoving 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 changesmark_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 gainsjob->state = JOB_STAT_RUN;, alongside the existingjob->allocs/job->accountsassignment. No new field.src/scheduler/src/handler.c—sched_handle_notify()'s switch gains one newcase NotifyData_data_jobDispatchFail:, applyingJOB_STAT_PENDthrough the existingXCE_SCHED_OP_JSTATE_CHANGEenqueue path (same shape as the adjacentschedExitJob/schedFailJobcases).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 ofxce_startjob_req()'src) moves inside anif (rc >= 0)branch. On success, the function takeslock(j, RTTI_LOCK_WRITE), commitsjob.allocs/job.state, appends theWAL_JOB_DISPATCHfact, thenunlock(j)— closing bug-1626 for this call site as a direct consequence of restructuring it. Onxce_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 ownlock(j, RTTI_LOCK_WRITE)): retryable (four sites, including RPC failure) → incrementdispatch_fail_count,unlock(j),dereference(j)(closing bug-1627's leak on these four paths), callnotify_dispatch(); terminal (two sites) → callmark_job_fail()under the write lock,unlock(j)after (these two already correctlydereference(j)today, unaffected by bug-1627).handle_jobdispatch()'s loop drops its existing per-jobnotify_dispatch(c, ep, alloc->jobid, ...)calls (both the success-path and failure-path ones) — notification is now entirely owned bydispatch_a_job_(), preventing the double-notify race found in round 2 of this review (a terminal failure previously would have produced bothjobDispatchFailandschedFailJob).
src/include/xce/types.h—xce_job_t's wrapperjob_t(not the wire type itself) gainsuint32_t dispatch_fail_count;, followingsubmit_lsn's existing "in-memory only, reset on replay" precedent.src/include/wal_types.h— one newwal_entry_typevalue,WAL_JOB_DISPATCH = 12, reusing the existingwal_jobstate_t{jobid, state, _pad=0}payload shape (the "legacy: state is absolute" discipline, matchingjob.allocs's own synchronous-commit precedent — see Data Models).src/controller/persist.c—WAL_JOB_DISPATCHis added as a newcaselabel 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) andapply_committed_entry()'s (the actual live-apply-and-recovery path,persist.c:3700-3706) — both already applywal_jobstate_t.stateas 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, andxce_dispatch_fail_class_tare declaredstatic/file-local next todispatch_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) — newjobDispatchFailcase appliesJOB_STAT_PEND, frees staleaccounts/allocsvia 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 existingXCE_DECISION_DISPATCHbranch, afterjob->accountsis built and beforesched_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, unchangedon_dispatch()call insched_allocator(). Firing a second hook is unnecessary and was confirmed unneeded viabuiltin_fcfs_on_dispatch()'s own pendJobs-removal behavior (Research Log). - The later, now-redundant
schedStartJob-triggered application ofJOB_STAT_RUN(via the existing, unmodifiedcore_events.cpath) 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->statetransitions PEND → RUN synchronously, in-process, no deferred-apply (the scheduler has no WAL/durability concept —xce_sched_job_tis a working copy, not a system of record). - Persistence & consistency: none — a scheduler restart already rebuilds
xce_sched_job_tfrom 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:insched_handle_notify()'s switch (handler.c), positioned alongside the existingschedStartJob/schedExitJob/schedFailJobcases. - Calls the same enqueue helper those cases already use (
sched_job_state_change(sched, notify->data.job_dispatch_fail->jobid, JOB_STAT_PEND)— thehandler.c-local queuing wrapper, not the deadscheduler.cmutator of the same name), so the actual state application flows through the livexce_sched_driver_translate_pending_events()→core_events.cpath 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.cXCE_CORE_EVENT_JOB_STATE_CHANGEcase's!JOB_IS_RUN(new_state)branch freesjob->accounts/job->allocsautomatically — 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 bynotify_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 asschedStartJob/schedExitJob/schedFailJobtoday).
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 unconditionalj->job.allocs = ...assignment (today placed after thexce_startjob_req()call but not gated on its return code) becomes conditional: only committed whenrc >= 0.j->job.state = JOB_STAT_RUNis 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_DISPATCHfact (see WAL Dispatch Fact, below) recording{jobid, state=JOB_STAT_RUN}, in addition to the synchronous in-memory mutation — mirrorsjob.allocs's existing synchronous-commit style, notrecord_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 setstart_time. - Requirement 2.3 (status queries reflect RUN) requires no new code —
job.stateis 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 thejob.allocs/job.statemutation and thepersist_submit_job_fact_ex()call, released viaunlock(j)immediately after) —_pad = 0(legacy/absolute), matchingwal_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 onlyRTTI_LOCK_READonc->jobmgr/c->workermgr/the workerw— it never takes any lock on the job objectjitself, despitej->job.allocsalready being mutated today.RTTI_LOCK_READis a genuinepthread-backed reader lock (my_rwlock_rlock(), confirmed insrc/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-existingjob.allocsmutation it sits beside, which this restructuring already touches) is brought underRTTI_LOCK_WRITE, matching the convention most other job-mutating call sites injob.calready correctly follow (e.g.job.c:768,1687,1795,2058).job_ctrl()'s own separate, unrelated call tomark_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 withindispatch_a_job_(). - Input / validation:
jobid, target state (JOB_STAT_RUN) — no new validation beyond whatdispatch_a_job_()already performs before reaching this point. - Output / destination: one
WAL_JOB_DISPATCHWAL entry, replicated and applied through the existing commit/apply pipeline. - Idempotency & recovery:
WAL_JOB_DISPATCH's case in bothapply_committed_entry()(live) andapply_wal_entry()(recovery replay) appliesstateas an absolute value (matching every_pad == 0job-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), thexce_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,
jnever resolved) is not part of this table at all — see the dedicated bullet below.
- Retryable (transient/environmental): worker lookup failure, worker not found, worker-not-ready (
- 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 declaredstaticinjob.c, not in the sharedsrc/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 accompanyingjob.allocs-gating anddereference(j)fixes at that exact call site.- Ownership correction found during
/kiro-validate-designround 2: classification and resolution happen insidedispatch_a_job_(), at each of its six post-lookup failure points, using thej/job_t*pointer already held there — not in its callerhandle_jobdispatch(). Two independent reasons: (1)handle_jobdispatch()only hasalloc->jobidafterdispatch_a_job_()returns (allocis freed immediately after), so resolving there would require a secondkey_find()lookup and reopen a TOCTOU gap between the two; (2)handle_jobdispatch()'s existing loop unconditionally callsnotify_dispatch()for everydispatch_a_job_()return, success (code=0) or failure —notify_dispatch()itself always emitsNotifyData_data_jobDispatchFailas the event type regardless ofcode(confirmed by reading its body,job.c:1108-1125). Left unchanged, a terminal failure would produce both ajobDispatchFailnotify (from the existing unconditional call) and aschedFailJobnotify (frommark_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-jobnotify_dispatch()calls (both the success-pathnotify_dispatch(c, ep, alloc->jobid, 0)and the failure-pathnotify_dispatch(c, ep, alloc->jobid, rc1)) are removed from that loop.dispatch_a_job_()now owns notification as part of resolution: it callsnotify_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 ownxce_dispatch_fail_site_tidentity (not the rawrc/errnovalue 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_countread 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, insidedispatch_a_job_(), before that point's existingdereference(j)/return (or, for the four sites bug-1627 identifies, where thatdereference(j)is being newly added):lock(j, RTTI_LOCK_WRITE).- Call
xce_classify_dispatch_fail(site)with this failure point's ownxce_dispatch_fail_site_tvalue. IfXCE_DISPATCH_FAIL_TERMINAL, skip to step 3's terminal branch immediately (no counter involved). IfXCE_DISPATCH_FAIL_RETRYABLE, incrementjob.dispatch_fail_countand capture the resulting value into a local. - 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 itsRTTI_LOCK_WRITE-held precondition), with ajob_failinfo_tdescribing the dispatch failure (jobid,code= the original error,detail= a short diagnostic string;.stageleft at 0, which correctly routesapply_job_fail_transition()'s switch to itsdefault:branch,job->job.state = JOB_STAT_EXIT— confirmed by reading that function directly; the two named.stagecases are for post-exec failures, not applicable to a job that never started). unlock(j), thendereference(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 <= 0andget_queue_exec()failure, already correctlydereference(j), unaffected by that bug — this step is uniform across all six regardless).- 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, sojobDispatchFailunambiguously 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_countis reset to 0 by Controller Dispatch Commit's own success path (above, under that component's ownRTTI_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.stateare only ever written inside therc >= 0success 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 inresearch.md. dispatch_a_job_()'s earliest failure point (jobid lookup failure — two code variants,key_find()itself failing or returning a NULLj, beforejis ever resolved) is a distinct case outside this table entirely: there is nojob_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 existingreturn rc/return -errnois sufficient, unchanged. Requirement 4's classification and this component's locking/counting/notification logic apply only to the six failure points reached afterjis 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:
siteis one of the six named enumerators. Corrected during task 1.1 implementation review: a default-lessswitchdoes NOT compel a compile error in plain C without-Werror(this project sets none) — it only emits a non-fatal-Wreturn-typewarning and falls through to undefined behavior on an unhandled value. The actual implementation instead follows this codebase's established fail-loud convention (matchingpersist.c'swal_classify(),persist.c:6030-6037): adefault:arm that ERROR-logs and returns a safeXCE_DISPATCH_FAIL_TERMINAL, withassert(0 && ...)gated to#ifdef XCE_TESTINGonly, 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_FAILUREclassifyXCE_DISPATCH_FAIL_RETRYABLE;INVALID_ALLOC,QUEUE_EXECclassifyXCE_DISPATCH_FAIL_TERMINAL. - Invariants: the same
sitealways classifies the same way — no hidden state, no time-dependence, no dependence on the specificrc/errnovalue 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_typevalueWAL_JOB_DISPATCH = 12(next free value in the job-fact range, afterWAL_JOB_ARRAY_SUBMIT = 11). - Reuses the existing
wal_jobstate_t{jobid, state, _pad}payload verbatim — no new struct._padis always0for this type (the "legacy: state is absolute value" conventionwal_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 atpersist.c:3452-3466(WAL_JOB_STARTthroughWAL_JOB_RUSAGE, includingWAL_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 oneraft-apply-decouplingactually introduced the_pad-discriminated routing for) has its own separate fall-through group atpersist.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 excludingWAL_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 absolutestatebaked at receipt time never goes stale" —WAL_JOB_DISPATCHfits this group by the same reasoning (Research Log: no same-job racing-fact concern for the first post-PEND transition).WAL_JOB_DISPATCHjoins 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-10ha_apply_single_entryrefactor) both callapply_committed_entry()exclusively;apply_wal_entry()has zero production call sites today. Adding the case to both functions is still correct (matches every siblingWAL_JOB_*type's identical dual-registration, harmless dead-code parity, and costs nothing ifapply_wal_entry()is ever revived) — only the prose description of whatapply_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(viaxce_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 — notxce_job_titself):+ uint32_t dispatch_fail_count— in-memory only, followingsubmit_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 = existingwal_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'ssched_get_job() == NULLbranch (already present incore_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()'sswitchoverxce_dispatch_fail_site_thas nodefaultcase, so a new enumerator with no matchingcaseis 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 setsjob->state == JOB_STAT_RUN, in addition to the existingaccounts/allocsassertions.sched_handle_notify()'s newjobDispatchFailcase enqueues anXCE_SCHED_OP_JSTATE_CHANGEtargetingJOB_STAT_PEND; a follow-up drain viaxce_sched_driver_translate_pending_events()/core_events.cconfirmsjob->accounts/job->allocsare 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), confirmjob.stateremains PEND andjob.allocsis not committed; for a successfulxce_startjob_req(), confirm both are committed together with oneWAL_JOB_DISPATCHfact appended. For the four retryable-class paths, additionally assertj'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 sixxce_dispatch_fail_site_tenumerators 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 tomark_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 receivesjobDispatchFail→ 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 existingschedFailJobnotify (unmodified path) → job resolves to EXIT withaccounts/allocsfreed. Assert the scheduler does not also receive ajobDispatchFailnotify for this same failure (regression test for the round-2 double-notify finding) —handle_jobdispatch()'s loop must not callnotify_dispatch()for a failuredispatch_a_job_()already resolved as terminal. - WAL durability: append a
WAL_JOB_DISPATCHfact, simulate a controller restart/replay, confirmjob.state == JOB_STAT_RUNis recovered without requiring the (separate, unmodified)WAL_JOB_STARTfact 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.