test(job-array-single-wal-record): full regression pass - GO

Final task of the spec. Ran the complete unit, integration, and soak
suite (controller_unit_test, all scheduler binaries, xcelib_test, Go
apiserver tests under both CGO_ENABLED=0/1, and every soak mode this
spec added) -- all clean, zero new job loss, divergence, or liveness
stall.

Designed and executed a genuine empirical byte-identical baseline
comparison for the non-array (array_size=1) path: built the commit
immediately preceding this spec's first commit in an isolated
worktree, deployed independent clusters from both the baseline and
current-tree binaries, and did a byte-for-byte comparison of the raw
on-disk WAL entries for an identical single-job submission.
Independently reproduced end-to-end by the reviewer with a consistent
result -- every differing byte maps to inherently run-specific fields
(timestamp/crc/jobid/sub_time), zero differences in term/lsn/type/
payload_len or the remaining ~6485 payload bytes. Corroborated by a
structural code-diff argument (the single-job WAL-encode path is
untouched, purely-additive diff only) and the pre-existing task 2.3
unit test asserting the same invariant.

Confirmed the three self-flagged residual gaps (task 3.1 bulk-insert
orphan prefix, task 4.2 boot-replay fence, task 5.3b bounded leak)
remain the complete and only set of open items across all 24
completed tasks.

Task 6.3 of job-array-single-wal-record (Requirements 8.1, 8.2, 8.4).
Overall verdict: GO. This completes the spec's implementation phase.
This commit is contained in:
Rongsong Shen 2026-07-16 07:42:21 +00:00
parent c6138496bd
commit 1cfdac47b6

View file

@ -180,7 +180,7 @@
- _Requirements: 8.2, 8.3_
- _Boundary: Non-Regression & Testing_
- [ ] 6.3 Full regression pass
- [x] 6.3 Full regression pass
- Run the complete existing unit, integration, and soak suite (sequential and chaotic) and confirm zero new job loss, divergence, or liveness stall attributable to this feature
- Confirm the non-array (array size 1) WAL format, commit-barrier count, and reply are byte-identical to the pre-feature baseline
- Observable: a clean full-suite run with the byte-identical single-job baseline check passing alongside it
@ -207,3 +207,4 @@
- **Task 6.0a**: Fixed `replay_wal()`'s job-count tally (persist.c ~3902) to add a `WAL_JOB_ARRAY_SUBMIT` branch that tallies `apply_rc` (the genuine per-record element count `apply_committed_entry()`'s array case already returns) when `apply_rc > 0`. Independently traced the FULL return-value space of `restore_job_array()`/`apply_committed_entry()`'s array case (6 distinct cases: 3 negative pre-mutation failures, `-EEXIST`→`apply_rc=0` idempotent no-op, the task-3.1 bulk-insert partial-failure case (stays negative, correctly still NOT tallied — a pre-existing, out-of-boundary undercount that still surfaces as a detectable anomaly rather than masking one), and positive `created` on genuine success) and confirmed every positive `apply_rc` this fix tallies is PROVABLY equal to real jobmgr growth for that record — the fix cannot overcount, and therefore cannot weaken `persist_verify_apply_correctness()`'s ability to catch a genuine divergence. Verified end-to-end via a from-scratch live 3-node HA soak re-run (`--array-soak`, both by the implementer and independently by the reviewer): zero `APPLY_DIVERGENT` across all three of the original bug's trigger paths (restart-mid-small-array, standby election-path recovery fallback, restart-mid-N=10000-array), one-WAL-record/one-commit-barrier confirmed for both N=2 and N=10000, per-element post-failover status/cancel/completion confirmed. This soak run also directly satisfies task 6.1's own Observable — its results should be reused rather than re-run from scratch. **New non-blocking finding (pre-existing, NOT introduced by this task)**: `controller_unit_test`'s single outer `UNITY_BEGIN()` is silently reset by ~19 of its called test-group functions' OWN nested `UNITY_BEGIN()/UNITY_END()` calls, making the process exit code / meson `Ok:/Fail:` summary line unreliable for detecting a regression in any group running before the last such nested call — both the implementer and reviewer worked around this by grepping raw `testlog.txt` for `:FAIL:` lines directly rather than trusting the summary, and this should be the standard practice going forward for this binary until a dedicated follow-up fixes the harness itself.
- **Task 6.1**: Extended `.claude/skills/xce-test-ha/run-ha-soak.sh`/`soak-verdict.sh` with a new `--array-soak` mode (small N=2 + large N=10000 sub-rounds, one clean kill/failover per sub-round — deliberately single-kill, not chaotic, matching this task's correctness-not-chaos scope; chaos-under-array-submission is task 6.2's separate job). `waldump.c` gained a `WAL_JOB_ARRAY_SUBMIT` case in `wal_type_name()` so the scenario can grep an exact type name for its live before/after WAL-record-count delta check. **Round 1 REJECTED**: the replay-time measurement used whole-second (`date +%s`) resolution — both sub-rounds' timings collapsed to `0s`/`n/a` on a real run — and, more fundamentally, nothing ever asserted on the comparison (`verdict_emit_arraysoak()` always returned PASS regardless of measured values), leaving Requirement 4.3 ("replay time governed by record size, not element count") with zero actual evidence and unable to catch a genuine O(N) regression. **Round 2 fixed**: switched to `$EPOCHREALTIME` (millisecond resolution; discovered and worked around a genuine environment trap — this sandbox's BusyBox `date` precedes GNU coreutils on `$PATH` and silently passes `%N`/`%3N` through as a literal string suffix rather than expanding or erroring, which would have poisoned any `date +%s%3N`-based approach silently); added `verdict_check_arraysoak_replay_bound()` with a real `bound = max(t_small*ratio_max, floor_ms) + margin_ms` assertion that `run_array_soak_round()` hard-`fail()`s on when exceeded — independently verified to have real teeth (both the implementer's and reviewer's own hand-picked regression cases correctly trip `BOUND_EXCEEDED`). Live-reproduced 3 times total (once round 1, twice round 2) with genuinely discriminating millisecond numbers each time (final: small=210ms, large=163ms — large array replay was NOT slower, live evidence of the feature's O(1) claim). Also fixed opportunistically: status/cancel/completion checks spread across low/mid/high indices (previously clustered at 1,2,3) matching the already-good basic-addressability spread; selftest extended 17→28 cases covering the new bound-check logic; `SKILL.md` documented. "One commit barrier" remains an inference from the WAL-record-count delta (not independently observed live) — defensible given task 2.1/2.3's unit-tested 1:1 append↔continuation coupling, but flagged as an inference, not a direct check, should a future task want to close that gap.
- **Task 6.2**: Extended the same harness with `--array-chaos-retry` (`submit_array_bg()`, `run_array_chaos_retry_round()`, `verdict_emit_arrayretry()`). Traced the full cross-primary dedup chain end-to-end (bug-1419's mechanism): client-side `idgen()` mints the base jobid once, `xcecli_jobsubmit()`'s `XCECLI_FAILOVER_CALL` retry loop resends the IDENTICAL submit (same jobid, never re-minted); server-side `handle_jobsubmit()` checks `jobid_range_exists()`/`inflight_range_try_add()` — both genuinely RANGE-shaped over `[base,base+array_size)`, run BEFORE the array-append branch, so the whole array is dedup-checked as one unit, not per-element; `xce_failover_classify()`'s "idempotent-replay rule" reclassifies a resulting `-EEXIST` as success on retry. Reviewer went further and confirmed a subtle apply-lag race (freshly-promoted primary's process-local in-flight registry starting empty) is closed by a PRE-EXISTING, unrelated mechanism (`raft_cb_become_leader`'s synchronous apply-pump drain before the promotion callback returns, added 2026-07-10 for an unrelated bug) — not something this task needed to build, just confirmed already-sound. No production code touched or needed — the existing single-job dedup mechanism generalizes correctly to arrays with zero changes. No mutation-test RED phase was performed against production code (correctly declined per this task's explicit constraint); reviewer compensated by extracting the harness's own assertion logic into a standalone mutation-harness with stubbed collaborators and confirming all four scenarios (clean/silent-loss/double-submit/partial-duplication) correctly trigger or don't trigger `fail()` as expected. Live-verified across 4 independent fresh cluster deployments total (2 by implementer, 2 by reviewer) spanning array sizes 6, 10, and 15, 17 total chaotic kill-during-submit rounds, zero double-submits, zero silent losses, zero partial duplications, genuine election/term diversity confirmed each time. `walcheck_count`'s divergence check is informational-only here, matching task 6.1's own established convention exactly (not a new decision) — judged acceptable since `wait_for_full_convergence` already requires matching `committedLsn` across all 3 nodes before this scenario's checks run.
- **Task 6.3**: Pure verification task, zero file changes. Ran the complete suite: `controller_unit_test` (299 tests, 0 failures, verified via raw-log `:FAIL:` grep, not the exit code/summary — per task 6.0a's nested-`UNITY_BEGIN`/`END` counter-reset finding), all scheduler binaries, `xcelib_test` (0 failures with the registered `detect_leaks=0` config; the one pre-existing task-1.3 rtti leak still reproduces under leak-detection-on, unchanged), Go apiserver tests under both `CGO_ENABLED=0` and `=1`, and every soak mode this spec added (`--selftest` 28/28, sequential baseline, `--chaotic`, `--array-soak`, `--array-chaos-retry`) — all clean. Designed and executed a genuine empirical byte-identical baseline comparison for the non-array path (Requirement 8.1): built commit `895a204` (immediately pre-dating this spec's first commit) in an isolated, cleanly-removed `git worktree add --detach`, deployed independent clusters from both the baseline and current-tree binaries, submitted one identical single-job request to each, and did a byte-for-byte `cmp -l` of the raw on-disk WAL entries. Independently reproduced end-to-end by the reviewer (separate build, separate clusters, separate comparison) with a consistent result: every differing byte mapped cleanly to `timestamp`/`crc`/`prev_crc` (the WAL header's inherently run-specific fields) or the payload's `jobid`/`sub_time`/`queue_time` fields — zero differences anywhere in `term`/`lsn`/`type`/`payload_len` or the remaining ~6485 payload bytes. This was further corroborated by a structural code-diff argument (the entire single-job WAL-encode path is untouched between `895a204` and HEAD, purely-additive diff only) and by the pre-existing task 2.3 unit test asserting the same invariant. Confirmed the three self-flagged residual gaps (task 3.1 bulk-insert orphan prefix, task 4.2 boot-replay fence, task 5.3b bounded leak) are the complete and only set of open items across all 24 completed tasks — no new regression, job loss, divergence, or liveness stall found anywhere. **Overall verdict: GO.**