Add uint32_t dispatch_fail_count to the job_t wrapper struct (xce/types.h, alongside submit_lsn, same in-memory-only/reset-on-failover precedent), per Requirement 4.3. Data model only -- xce_job_t (wire type) untouched, job.c untouched; increment/reset wiring and the bounded-retry threshold check are tasks 3.1/3.3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
102 lines
13 KiB
Markdown
102 lines
13 KiB
Markdown
# Implementation Plan
|
|
|
|
- [ ] 1. Foundation: controller-side dispatch-failure classification and durable transition record
|
|
- [x] 1.1 (P) Define the dispatch-failure classification used to decide whether a failed dispatch attempt should retry or fail permanently
|
|
- Enumerate each distinct way a dispatch attempt to a worker can fail today (worker lookup, worker unavailable, worker not ready, invalid allocation, queue configuration lookup, the dispatch request itself failing), and classify each as either retryable (transient/environmental) or terminal (a locally invalid decision that retrying will not fix)
|
|
- The classification is a pure decision with no side effects, independently testable against every known failure case without needing a live dispatch attempt
|
|
- Observable completion: a standalone test exercises every classified failure case and asserts the expected retryable/terminal outcome
|
|
- _Requirements: 4.1, 4.2_
|
|
- _Boundary: Controller Dispatch-Failure Classification (site enum + classify function)_
|
|
|
|
- [x] 1.2 (P) Add a durable record of a job's dispatch-time transition to a running state
|
|
- Introduce a new durable fact type recording that a job has transitioned to running state at dispatch-commit time, reusing the existing shape used for other job-state facts
|
|
- Wire this new fact type into both the normal live-commit apply path and the recovery/restart replay path, so the transition is recovered identically whether observed live or replayed after a restart
|
|
- Observable completion: appending this fact and then replaying it (simulating a restart) leaves the job's state as running, without requiring the separate worker-confirmed-start fact to also be present
|
|
- _Requirements: 2.4_
|
|
- _Boundary: WAL Dispatch Fact_
|
|
|
|
- [x] 1.3 (P) Track how many consecutive times a job's dispatch has failed for a retryable reason
|
|
- Add an in-memory counter on the controller's job record that increments on each retryable dispatch failure and resets to zero on a successful dispatch
|
|
- This counter does not need to survive a controller failover — resetting to zero after a failover is an acceptable, safe degradation
|
|
- Observable completion: the counter can be incremented, read, and reset independently of any actual dispatch attempt
|
|
- _Requirements: 4.3_
|
|
- _Boundary: Controller Dispatch-Failure Classification (retry-counter data model)_
|
|
|
|
- [ ] 2. Core: scheduler transitions a job to running state at dispatch-commit time
|
|
- No dependency on Task 1 — scheduler-side files only, independent of the controller-side foundation work above.
|
|
- [ ] 2.1 (P) Scheduler marks a job as running in the same operation that commits its dispatch decision
|
|
- When the scheduler commits a decision to dispatch a job, the job's state becomes running immediately, in the same operation that already records the job's resource allocation — not waiting for any later confirmation
|
|
- The scheduler's existing resource-in-use accounting, which only counts jobs already in a running state, requires no changes — this task provides the input it was missing
|
|
- Observable completion: a job dispatched in one round appears in resource-in-use totals starting that same round, and continues to appear in every subsequent round regardless of how many rounds pass before any confirmation arrives
|
|
- _Requirements: 1.1, 1.2, 1.3_
|
|
- _Boundary: Scheduler Dispatch Commit_
|
|
|
|
- [ ] 2.2 (P) Scheduler processes a reported dispatch failure by returning the job to a pending state
|
|
- When the scheduler is told a job's dispatch failed, it returns that job to a pending state instead of silently ignoring the report as it does today
|
|
- Returning a job to pending correctly releases the resource-allocation data that was tentatively committed for it, reusing the scheduler's existing behavior for any other transition away from a running state
|
|
- Observable completion: a job whose dispatch failure is reported to the scheduler is again eligible for the scheduler's own job-selection process in a subsequent round, with no dangling resource-allocation data left behind
|
|
- _Requirements: 3.1, 3.2, 4.4_
|
|
- _Boundary: Scheduler Dispatch-Failure Handler_
|
|
|
|
- [ ] 3. Core: controller transitions a job to running state only once dispatch to the worker succeeds, and resolves failures
|
|
- Sub-tasks 3.1-3.3 all restructure the same function and share its per-job lock scope, so none of them are marked parallel to each other, regardless of how disjoint their individual concerns look.
|
|
- [ ] 3.1 Controller commits a job's resource allocation and running-state transition together, only on a successful dispatch
|
|
- The controller's existing (currently unconditional) commit of a job's resource-allocation data becomes conditional on the dispatch request to the worker actually succeeding; the job's state transition to running happens in that same successful outcome, protected against concurrent access to the job record
|
|
- Record the new durable dispatch-transition fact (Task 1.2) as part of this same successful outcome
|
|
- Reset the consecutive-retryable-failure counter (Task 1.3) to zero as part of this same successful outcome, so a job that later fails again starts counting from zero rather than retaining count from before its last success
|
|
- The worker's own later confirmation that the job actually started continues to be recorded exactly as it already is today, unaffected by this change
|
|
- Observable completion: querying a job's status immediately after a successful dispatch shows it as running, and that status is unaffected by whether the worker has separately confirmed the job started yet; a job that fails, succeeds, then fails again is evaluated against the bound as if it had only failed once
|
|
- _Requirements: 2.1, 2.2, 2.3, 2.4, 5.1_
|
|
- _Depends: 1.2, 1.3_
|
|
|
|
- [ ] 3.2 Controller resolves a terminal (locally-invalid) dispatch failure
|
|
- When a dispatch attempt fails for a reason the classification (Task 1.1) marks terminal, the controller resolves the job to a permanently failed state immediately, protected against concurrent access to the job record, and releases any resource-allocation data tentatively associated with the attempt
|
|
- This covers the two failure sites that already release the job reference they hold today — confirm that behavior is preserved, not duplicated, by this new resolution step
|
|
- Observable completion: a terminally-classified dispatch failure leaves the job permanently failed, with no partial allocation data left behind and the job's reference count unchanged from before the attempt
|
|
- _Requirements: 4.2, 4.5_
|
|
- _Depends: 1.1_
|
|
|
|
- [ ] 3.3 Controller resolves a retryable (transient/environmental) dispatch failure, with bounded escalation
|
|
- When a dispatch attempt fails for a reason the classification (Task 1.1) marks retryable, the controller increments the consecutive-failure counter (Task 1.3) under the same protected access; if the incremented count is still within a bounded limit, it reports the failure so the job can be retried, otherwise it resolves the job to a permanently failed state exactly as Task 3.2 does for a terminal failure
|
|
- This covers the four failure sites that do not currently release the job reference they hold — fix each to release it as part of this same resolution step (Task 3.2's two sites already release theirs correctly and are unaffected)
|
|
- Observable completion: a retryable dispatch failure below the bound leaves the job eligible for another attempt with its reference count restored to the pre-attempt value; a retryable failure that pushes the count past the bound resolves the job to permanently failed instead
|
|
- _Requirements: 4.1, 4.3, 4.4, 4.5_
|
|
- _Depends: 1.1, 1.3, 3.2_
|
|
|
|
- [ ] 4. Integration: caller no longer duplicates dispatch-outcome reporting
|
|
- [ ] 4.1 Remove the caller's own dispatch-outcome reporting now that resolution owns it
|
|
- The function that invokes a dispatch attempt for each job in a batch no longer separately reports the outcome itself — reporting is now entirely owned by the dispatch resolution added in Task 3, preventing a failed dispatch from ever producing two different, conflicting outcome reports for the same job
|
|
- Observable completion: a dispatch failure that resolves as permanent produces exactly one outcome report, not two conflicting ones
|
|
- _Requirements: 3.1, 4.1, 4.2_
|
|
- _Depends: 3.1, 3.2, 3.3_
|
|
|
|
- [ ] 5. Validation: end-to-end behavior and regression
|
|
- [ ] 5.1 End-to-end test: resource-in-use accounting survives a multi-round gap between dispatch and confirmation
|
|
- Reproduce bug-1624 directly: dispatch a job, let several rounds pass before the worker confirms it started, and confirm resource-in-use totals reflect the job continuously from the round it was dispatched, matching what a same-round confirmation would have produced
|
|
- Observable completion: the regression scenario passes, and fails against the pre-change code path if temporarily reverted
|
|
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.3_
|
|
- _Depends: 2.1, 3.1_
|
|
|
|
- [ ] 5.2 End-to-end test: retryable and bounded-retry dispatch failures resolve correctly
|
|
- Simulate a transient dispatch failure and confirm the job becomes eligible for another attempt; simulate enough consecutive transient failures to exceed the bound and confirm the job resolves to permanently failed instead of retrying indefinitely; simulate a failure, a success, then another failure and confirm the bound is evaluated as if only one failure had occurred
|
|
- Observable completion: all three scenarios produce the documented outcome, with the counter's bound value exercised at exactly the boundary (one under, exactly at, and one over)
|
|
- _Requirements: 3.1, 3.2, 4.1, 4.3, 4.4, 4.5_
|
|
- _Depends: 2.2, 3.3_
|
|
|
|
- [ ] 5.3 End-to-end test: terminal dispatch failure resolves without duplicate reporting or leaked references
|
|
- Simulate a locally-invalid dispatch decision and confirm the job resolves to permanently failed, that no retry-eligible report is also produced for the same failure, and that the job record's reference count returns to its pre-attempt value
|
|
- Observable completion: the assertions above all pass in the same test run
|
|
- _Requirements: 4.2, 4.5_
|
|
- _Depends: 3.2, 4.1_
|
|
|
|
- [ ] 5.4 Full regression: existing suites and HA soak
|
|
- Run the existing scheduler and controller test suites end to end, updating only the assertions that directly depended on the old confirmation-gated timing; run a full sequential and chaotic HA soak
|
|
- Observable completion: both test suites pass and the soak shows no new job loss, state divergence, or liveness stall attributable to this feature
|
|
- _Requirements: 5.2, 5.3, 5.4_
|
|
- _Depends: 5.1, 5.2, 5.3_
|
|
|
|
## Implementation Notes
|
|
- Task 1.1: design.md's "Controller Dispatch-Failure Classification" Service Interface Preconditions text claims a default-less `switch` over `xce_dispatch_fail_site_t` gives "compiler-enforced exhaustiveness... fails to compile" for an unhandled case — verified false for plain C without `-Werror` (this project sets none, confirmed in `src/meson.build`/`src/controller/meson.build`). Implemented instead with a `default:` arm that ERROR-logs and returns a safe `XCE_DISPATCH_FAIL_TERMINAL`, with `assert(0 && ...)` gated to `#ifdef XCE_TESTING` only — mirrors the established, verified-matching precedent `persist.c`'s `wal_classify()` already uses (`persist.c:6030-6037`) for the same "never risk silent misbehavior on an unrecognized case" concern. Any later task/spec touching this same design.md section should correct its wording rather than re-trusting the "fails to compile" claim.
|
|
- Tasks 3.2/3.3 (retryable/terminal resolution, both call `xce_classify_dispatch_fail()`) should follow the same fail-loud `default:` convention if they add their own exhaustive switches (e.g. over `xce_dispatch_fail_class_t`'s two values).
|
|
- Operational note (not spec-relevant): implementer/reviewer subagents in this run consistently ended their turn mid-build (after starting `xce-build.sh controller`, before it finished) rather than blocking on it, requiring one resume-and-nudge each time. Future task dispatches in this run should explicitly instruct agents to poll/block synchronously until the build genuinely completes before composing their status/verdict block, to save round-trips.
|
|
- Task 1.2: `apply_wal_entry()` (persist.c) is confirmed to have ZERO production call sites today — `persist_recover()`'s actual boot-time replay path (`replay_wal()`) and the standby catch-up path (`ha_standby_advance_committed_and_apply()`, post the 2026-07-10 `ha_apply_single_entry` refactor) both go through `apply_committed_entry()` exclusively. design.md's "WAL Dispatch Fact" component (its "Preconditions"-adjacent prose) still characterizes `apply_wal_entry()` as "the recovery/replay-time path reading WAL segments from disk after restart" — that's stale. Adding `WAL_JOB_DISPATCH` to both functions' fall-through groups was still correct (matches every sibling `WAL_JOB_*` type's identical dual-registration, harmless dead-code parity) and is NOT something to undo — but design.md's wording should be corrected before any later task cites `apply_wal_entry()`'s role from that doc without re-verifying.
|