feat(scheduler-dispatch-commit): handle_jobdispatch no longer duplicates dispatch-outcome reporting (task 4.1)
dispatch_a_job_() (tasks 3.1-3.3) now owns notification entirely for every outcome -- notify_dispatch() on a retryable failure, mark_job_fail()'s own hook chain on a terminal failure, nothing on success. handle_jobdispatch()'s loop still unconditionally called notify_dispatch() itself for every job regardless, producing two conflicting outcome reports for any failed dispatch. Removed that caller-side call; the loop is now a thin dispatch-and-free loop with no notification logic of its own. New test_jobdispatch_no_double_notify.c drives a 3-job batch (success, retryable, terminal) through handle_jobdispatch() and confirms exactly one notification fires (from the retryable job only), not four. Independently reviewed and approved. Confirmed handle_jobdispatch()'s sole caller never depended on its notification behavior (always returns 0 unconditionally). Flagged that test_commit_barrier_reply.c's test_jobdispatch_receipt_ack_no_wal_append is now stale for a second, independent reason (notify count, not just WAL count) -- both left for task 5.4 per Requirement 5.3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
parent
65390fb111
commit
7b3dfed56e
5 changed files with 374 additions and 10 deletions
|
|
@ -64,7 +64,7 @@
|
|||
- _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
|
||||
- [x] 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_
|
||||
|
|
@ -112,3 +112,5 @@
|
|||
- Task 3.3: capturing a site's natural error code (`rc`/`errno`) into a stable local must happen BEFORE any intervening call once more logic is inserted before the eventual `return` — `errno` is not guaranteed stable across `lock()`/`ERROR()`/`free_launchinfo()` etc. This bit the worker-null and worker-not-ready sites specifically (both originally read `errno` directly at the `return` statement with nothing in between; task 3.3 had to capture it immediately after `SET_ERROR()`/at branch entry, before inserting the new lock/classify/resolve sequence).
|
||||
- Task 3.3: `test_commit_barrier_reply.c` (a file otherwise owned by the `ha-commit-barrier-rollout` spec) is this binary's sole host of the `notify()` stub — any future task needing to observe `notify_dispatch()`'s calls in a controller unit test must add accessors there too (purely additive, read-only `_ext`-suffixed pass-throughs to its existing static capture globals — do not modify the stub body itself or its existing pinned tests). Confirmed as legitimate shared test infrastructure, not a boundary violation, by task 3.3's independent review.
|
||||
- Task 3.3: the bound constant is `XCE_DISPATCH_FAIL_RETRY_BOUND = 3` (design.md's stated default), defined once near job.c's top and used at all four retryable sites — no magic numbers. 3 consecutive failures still retry; the 4th resolves terminal.
|
||||
- Task 4.1: **`test_commit_barrier_reply.c`'s `test_jobdispatch_receipt_ack_no_wal_append` is now stale for a SECOND, independent reason**, on top of task 3.1's already-documented WAL-append-count staleness: its 2-job batch (one real-worker success, one `nhosts==0` terminal failure — no retryable job) now produces ZERO `notify_dispatch()` calls total post-task-4.1 (the success path never called it even before task 4.1, per task 3.1's design; the terminal path resolves via `mark_job_fail()`'s `schedFailJob` hook chain, not `notify_dispatch()`/`jobDispatchFail`, per task 3.2's design) — breaking its pinned `g_notify_count == 2` / both-`jobDispatchFail`-event assertions (lines ~1991-2003). **Task 5.4 must update BOTH the WAL-count assertion (task 3.1's finding) and the notify-count/event assertions (this finding) in the same test** before claiming full-suite regression-clean; the test's own scenario (1 success + 1 terminal, zero retryable jobs) may need to change too since it can no longer meaningfully exercise `notify_dispatch()` at all with today's cast of outcomes — consider adding a retryable-classified job to the batch if the intent is still to prove `notify_dispatch()`'s wiring through `handle_jobdispatch()`.
|
||||
- Task 4.1: confirmed `handle_jobdispatch()`'s only caller (`cluster.c:471`, inside `myreq_handler`) uses its return value only as the generic RPC-handler `rc`, and `handle_jobdispatch()` itself always returns 0 unconditionally regardless of any job's outcome — nothing depends on the removed per-job notification behavior at the caller level.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue