lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(scheduler-dispatch-commit): add durable WAL_JOB_DISPATCH fact type (task 1.2)

Browse source 
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>
This commit is contained in:
Rongsong Shen 2026-08-23 07:12:50 +00:00
parent 5724d1e90a
commit 8eea74909c
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
7 changed files with 367 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

4
.kiro/specs/scheduler-dispatch-commit/design.md
View file

@ -109,7 +109,7 @@ sequenceDiagram
- `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 (recovery-replay path, `persist.c:3452-3466`) and `apply_committed_entry()`'s (live-apply 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/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
@ -289,7 +289,7 @@ xce_dispatch_fail_class_t xce_classify_dispatch_fail(xce_dispatch_fail_site_t si
**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+`, the recovery/replay-time path reading WAL segments from disk after restart) has its own fall-through group at `persist.c:3452-3466` (`WAL_JOB_START` through `WAL_JOB_RUSAGE`, including `WAL_JOB_FAIL` — this recovery path 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.
- **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]