test(scheduler-dispatch-commit): fix last stale pinned assertion, full regression clean (task 5.4, final)

Fixes test_commit_barrier_reply.c's test_jobdispatch_receipt_ack_no_wal_append
(owned by ha-commit-barrier-rollout), the last remaining assertion this
feature's earlier tasks left stale:

- WAL-count delta corrected from "must be zero" to lsn_before + 1 -- the
  batch's successful dispatch now appends one WAL_JOB_DISPATCH fact
  (task 3.1); mark_job_fail() was traced and confirmed to append
  nothing via any path.
- The batch's terminal-failure job was replaced with a genuinely
  retryable one (WORKER_NULL site, unregistered host, new job id)
  rather than degrading the notify-count assertion to zero, which
  would also have silently killed the test's own reply-before-notify
  ordering pin (nothing left to order against). This keeps the test
  meaningfully exercising notify_dispatch()'s wiring through
  handle_jobdispatch().

Full regression, independently re-verified by review:
- Scheduler suite at documented baseline (51/52, 1 pre-existing
  unrelated failure). Investigated and logged bug-1632, a new sibling
  of the already-known bug-1598 stack-lifetime pattern in a different
  test file -- confirmed pre-existing, untouched by this feature, and
  non-deterministic.
- Controller suite clean: 1133 tests, only the 2 pre-existing bug-1561
  failures.
- Sequential HA soak: PASS (10 rounds, 36 jobs, zero loss, all 3 nodes
  led).
- Chaotic HA soak: PASS (10 rounds, committed_divergence=0
  committed_job_loss=0).

This completes the scheduler-dispatch-commit spec: bug-1624
(resource-in-use round-boundary loss) and bug-1625 (scheduler discards
dispatch-failure notify) are both fixed and regression-tested.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cxa1PmqAB5PyXfJaNvj8TN
This commit is contained in:
Rongsong Shen 2026-08-23 21:39:01 +00:00
parent 5591f3603f
commit a9cfee5fef
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
2 changed files with 112 additions and 37 deletions

View file

@ -89,7 +89,7 @@
- _Requirements: 4.2, 4.5_
- _Depends: 3.2, 4.1_
- [ ] 5.4 Full regression: existing suites and HA soak
- [x] 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_
@ -119,3 +119,7 @@
- Task 5.2: **bug-1631 (logged, NOT fixed, deliberately deferred)** — `dispatch_a_job_()`'s four retryable-failure branches (job.c, tasks 3.1/3.3) never revert `job.state`/`job.allocs` when a retryable failure occurs while the job's controller-side state is CURRENTLY RUN (they were only ever designed for a job whose every dispatch attempt starts from PEND, which is all that's reachable today). A kiro-debug investigation (HIGH confidence), independently re-verified even more exhaustively by this task's own reviewer (full trace of every `pendJobs`-reinsertion and every controller-side `job.state`-mutation call site), confirmed this scenario is genuinely UNREACHABLE via any real caller in the current, fully-implemented system: the scheduler's picker permanently removes a job from its PEND candidate pool the instant it dispatches it (task 2.1), and nothing already-implemented ever re-adds a successfully-dispatched (RUN) job back to that pool. The only mechanism design.md itself anticipates for that is `scheduler-preemption`, confirmed still unimplemented (`requirements-generated`, no approvals). **Revisit bug-1631 and add the state/allocs reversal to `dispatch_a_job_()`'s four retryable branches only once `scheduler-preemption` (or an equivalent mechanism) actually creates a real RUN→retryable-redispatch path** — not before, since there's no reachable caller to regression-test against today. `test_rpc_failure_bound_resets_after_intervening_success` (`test_dispatch_fail_retry.c`) proves Requirement 4.3's actual, literal ask (the counter genuinely resets on an intervening success) without asserting the unreachable state/allocs behavior.
- Task 5.2: design.md's "vacuously by construction" reasoning (`Controller Dispatch Commit` and `Controller Dispatch-Failure Classification` sections — "on any of the six classified failure paths there is nothing to release") silently assumes every `dispatch_a_job_()` invocation starts from a job that's never had anything committed (i.e., a first-ever dispatch attempt) — true today (see bug-1631 above) but not stated as a scoped assumption in the doc text itself. Worth correcting design.md's wording if it's ever revisited for the `scheduler-preemption` follow-up.
- Task 5.3: `test_dispatch_fail_terminal.c` (task 3.2) and `test_jobdispatch_no_double_notify.c` (task 4.1) each covered PART of this task but not the full combination in one run — 3.2's test bypasses `handle_jobdispatch()` entirely (via `dispatch_a_job_test()`), so it can't exercise the double-notify race at all; 4.1's test only proves an *aggregate* batch notify count, not a *direct* per-job zero-notify proof. `test_handle_jobdispatch_terminal_no_notify_no_leak` (extending 4.1's file, the one with the right entry point) closes this by combining EXIT resolution + a direct single-job zero-notify assertion + the canary-reference no-leak proof, all through the real `handle_jobdispatch()` entry point in one test. Pattern worth remembering: a batch-aggregate test and a bypassed-entry-point test can each look like they cover a scenario while neither actually does, in combination, what a later task's literal wording asks for ("all pass IN THE SAME TEST RUN").
- Task 5.4 (final task — spec complete): fixed `test_commit_barrier_reply.c`'s `test_jobdispatch_receipt_ack_no_wal_append` (owned by `ha-commit-barrier-rollout`), the last remaining stale assertion from this feature's own changes. WAL-count delta corrected from "must be zero" to `lsn_before + 1` (the batch's successful dispatch now appends one `WAL_JOB_DISPATCH` fact; `mark_job_fail()` was traced and confirmed to append nothing via any path). Notify-count: rather than degrading to "must be zero" (which would also silently kill the test's own reply-before-notify ordering assertion, since there'd be nothing left to order against), the batch's terminal-failure job was replaced with a genuinely retryable one (`WORKER_NULL` site, unregistered host, new job id) — preserves the test's ability to meaningfully exercise `notify_dispatch()`'s wiring through `handle_jobdispatch()`. Independently reviewed and approved, including scrutiny of this specific choice.
- Task 5.4: discovered and logged **bug-1632** (scheduler suite) — a new sibling instance of the already-known bug-1598 pattern (unjoined `sim_picker_thread_fn` background thread reading a stack-local fixture after frame reuse), this time in `test_sim_verify.c`'s own separate `run_sim_and_capture_stdout()` helper (bug-1598's original fix only covered 3 stack-locals in `test_sim_event_loop.c`, not this one). Confirmed via `git log`/`git diff` as untouched by this feature and confirmed non-deterministic (multiple clean re-runs). Pre-existing, unrelated, NOT fixed — a candidate for whoever next touches `test_sim_verify.c`'s fixture pattern.
- Task 5.4: bug-1429's hang did NOT reproduce during this task's full-suite runs (multiple clean 1133-test passes) — consistent with its own buglog history describing it as non-deterministic/timing-dependent, not evidence it's resolved. Still treat any future `meson test` TIMEOUT on `test_commit_barrier_reply`/`test_commit_barrier_rollout` as this known, pre-existing, unrooted issue rather than a new regression, per all of tasks 3.1-5.3's own prior confirmations.
- **Feature complete**: all tasks (1.1-1.3, 2.1-2.2, 3.1-3.3, 4.1, 5.1-5.4) implemented, independently reviewed and approved, and committed. Final regression: scheduler suite at documented baseline (51/52, 1 pre-existing unrelated failure), controller suite clean (1133 tests, only 2 pre-existing bug-1561 failures), sequential HA soak PASS (10 rounds, zero job loss, all 3 nodes led), chaotic HA soak PASS (10 rounds, `committed_divergence=0 committed_job_loss=0`) — independently re-verified by task 5.4's reviewer via its own rebuild plus a 5-round spot-check of both soak modes. bug-1624 and bug-1625 (this spec's originating bugs) are fixed by tasks 3.1/2.1 and 2.2 respectively; `.wolf/buglog.json` should be updated to reflect FIXED status as a follow-up.