feat(scheduler-dispatch-commit): controller commits alloc+RUN transition together on successful dispatch (task 3.1)
dispatch_a_job_()'s previously-unconditional job.allocs commit is now gated on xce_startjob_req() actually succeeding (rc >= 0). In that same successful branch, under a newly-acquired lock(j, RTTI_LOCK_WRITE) (a real pre-existing locking gap this call site had -- it took no lock on the job object itself before this), the job transitions to JOB_STAT_RUN, dispatch_fail_count resets to 0, and a WAL_JOB_DISPATCH fact is appended for durability. The failure path is otherwise unchanged -- classification and resolution of the six failure sites is tasks 3.2/3.3's job, not this one's. Adds an XCE_TESTING-only dispatch_a_job_test() wrapper (dispatch_a_job_() is static) and test_dispatch_commit.c, proving: state->RUN, allocs committed correctly, counter resets, and durability survives a real persist_recover() replay with no WAL_JOB_START present. Independently reviewed and approved. Two pre-existing, out-of-boundary findings surfaced and logged for later tasks: test_commit_barrier_reply.c's test_jobdispatch_receipt_ack_no_wal_append will need updating by task 5.4 (Req 5.3 anticipates this), and bug-1429's hang was reconfirmed a 3rd time (unrelated to this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
parent
d87f4bb333
commit
d9aedc4b34
5 changed files with 699 additions and 11 deletions
|
|
@ -40,7 +40,7 @@
|
|||
|
||||
- [ ] 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
|
||||
- [x] 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
|
||||
|
|
@ -100,3 +100,8 @@
|
|||
- 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.
|
||||
- Task 3.1: `xce_request()`/every `xce_*_req()` wrapper (`src/lib/request.c`) is genuinely non-blocking fire-and-forget (capnp-encode + `endpoint_enq()` onto `ep->outq`, returns immediately — never waits for a reply). `endpoint_enq()` succeeds unconditionally against a zeroed `endpoint_t` with `ep->iomgr == NULL`. Net: testing a controller-side `dispatch_a_job_()` success path needs no live peer/socketpair/round-trip thread — a zeroed static `endpoint_t` (`.fd = -1`, no iomgr) is sufficient, matching `test_commit_barrier_reply.c`'s own established `g_wkr_disp_ep` fixture precedent. Reused directly for `test_dispatch_commit.c`; any later task exercising `dispatch_a_job_()`/`xce_startjob_req()` in a test should use the same pattern rather than building real network mocking.
|
||||
- Task 3.1: **`test_commit_barrier_reply.c`'s `test_jobdispatch_receipt_ack_no_wal_append`** (a different, `ha-commit-barrier-rollout`-owned pinned test) asserts a successful dispatch cycle appends ZERO WAL entries — now false, since a successful dispatch appends exactly one `WAL_JOB_DISPATCH` fact (Req 2.4). Confirmed by both task 3.1's implementer and its independent reviewer; this is explicitly the kind of stale assertion Requirement 5.3 / task 5.4 exists to update, deliberately left unfixed here (out of 3.1's boundary, and that file's tests were unreachable in a full-suite run anyway due to the bug-1429 hang below, registered earlier in `controller_test.c`'s `main()`). **Task 5.4 must update this assertion** before claiming full-suite regression-clean.
|
||||
- Task 3.1: reconfirmed **bug-1429** (`controller_unit_test` TIMEOUT/hang, pre-existing, unrooted) a 3rd time, now hanging inside `test_commit_barrier_reply.c`'s `test_jobsubmit_repeated_truncation_forced_failures_no_phantom_jobs` — a 3rd distinct hang location across 3 sessions, confirmed unrelated to `dispatch_a_job_()` (that test never calls it) and independently reproduced twice more (standalone binary run past the meson harness's 300s cap, and a fully separate rebuild). Because this hang sits earlier in `controller_test.c`'s registration order than most of this feature's own new test files, a full un-filtered `meson test` run will NOT reach them — **task 5.4's full-suite validation must run with the hanging group filtered out (`controller_unit_test -t <group>`) or budget for the 300s timeout**, not treat a bare `meson test` TIMEOUT as this feature's own failure.
|
||||
- Task 3.1: confirmed pre-existing (not introduced by this task, not in this task's boundary to fix) — `dispatch_a_job_()`'s success path never calls `dereference(j)` on the reference taken at lookup; identical in the diff's before/after, and the sole caller `handle_jobdispatch()` never dereferences either. A candidate future bug-log entry, out of scope for this spec.
|
||||
- Task 3.1: when validating a build against this suite's ASAN leak output, always replicate the project's own `ASAN_OPTIONS=detect_leaks=0` test env (`meson.build:229`) — a standalone binary run without it produces spurious leak reports (a red herring, not a regression) that can waste review time.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue