test(scheduler-dispatch-commit): bug-1624 multi-round regression coverage (task 5.1)
Extends task 2.1's test_scheduler_dispatch_commit.c with two tests proving resource-in-use accounting survives more than a single round boundary between dispatch and worker confirmation, per task 5.1's "several rounds" / "matching a same-round confirmation" language: - test_dispatch_commit_survives_several_rounds_before_confirmation: loops 5 round-boundary cycles, asserting resInUse after EVERY round, not just once. - test_dispatch_commit_multi_round_gap_matches_same_round_confirmation: compares resInUse after 1 round vs after 5 rounds and asserts equality -- a same-round confirmation is modeled as nrounds==1 since design.md documents re-applying JOB_STAT_RUN to an already-RUN job as a confirmed no-op, so resInUse depends only on job->state, never on confirmation timing. Test-only; scheduler.c is untouched. RED phase independently verified by both implementer and reviewer via an actual revert-rebuild-rerun cycle (4/5 tests fail without the task 2.1 fix). Independently reviewed and approved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
parent
7b3dfed56e
commit
01c8306694
2 changed files with 188 additions and 1 deletions
|
|
@ -71,7 +71,7 @@
|
|||
- _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
|
||||
- [x] 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_
|
||||
|
|
@ -114,3 +114,5 @@
|
|||
- 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.
|
||||
- Task 5.1: a "same-round confirmation" scenario for resource-in-use accounting is legitimately modeled as `nrounds==1` rather than by simulating a real `schedStartJob` notify event — design.md documents (and the reviewer independently re-verified against `core_events.c`) that re-applying `JOB_STAT_RUN` to an already-RUN job via the confirmation path is a confirmed no-op, so `resInUse` depends solely on `JOB_IS_RUN(job->state)`, never on a separate confirmation flag. Reviewer's own finding: the resulting same-round-vs-multi-round comparison test is honest and correctly fails on revert, but adds little independent detection power beyond a per-round-asserting loop test (which is the one genuinely load-bearing proof) — worth knowing if a future task considers deduplicating these two tests, but not something to act on now.
|
||||
- Task 5.1: no controller-side test was added for this task — Requirements 2.1/2.3 (controller transitions to RUN independent of confirmation) are already covered by task 3.1's `test_dc_success_commits_alloc_state_and_resets_counter`. No live scheduler+controller wire-protocol integration harness exists anywhere in this codebase; building one was judged disproportionate to what this task verifies, and the reviewer agreed.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue