apply_committed_entry() (persist.c), the single seam that applies every
committed WAL entry, only ever fired the generic HOOK_DETAIL_JOB_UPDATE
for job-state changes -- HOOK_DETAIL_JOB_START/JOB_EXIT were
permanently dead code, since job.c only ever fires them at
HOOK_BEFORE_EVENT (for WAL-append purposes). This left sched_job_start_
fun/sched_job_exit_fun (sched.c) and apiserver_job_start_fun/
apiserver_job_exit_fun (apiserver.c) unreachable, and the Go apiserver's
already-wired OnJobFinished handler permanently starved of data -- a
finished job's state could stay stuck (e.g. showing RUN) in the
scheduler's own view and the apiserver never learned a job finished.
The wire format was already enriched by bug-1697/bug-1689 with the
pidinfo/exit_info data these hooks need, so this closes the gap rather
than requiring new wire work. WAL_JOB_FINISH now fires
HOOK_DETAIL_JOB_EXIT; WAL_JOB_START now fires HOOK_DETAIL_JOB_START,
gated on a call-scoped decode flag rather than the transition result
(WAL_JOB_START applies as a no-op duplicate for every real job in
production per bug-1697, so gating on the transition result would make
it dead code again). WAL_JOB_FAIL -> HOOK_DETAIL_JOB_FAIL has the same
gap and is left as a follow-up.
Also includes this investigation's supporting changes: a DEBUG3 log-
retention bump (main.c, both controller and scheduler) so race-condition
evidence survives a live burst without rotating out, and DEBUG3-gated
diagnostic trace logging at the two points a job-update notification
can fire or silently drop (job.c, sched.c).
Live-verified end to end on a real deployed cluster: scheduler-side
sched_job_state_change now fires for RUN and EXIT states, and the
apiserver's OnJobFinished -- previously never invoked in production --
now correctly updates job status on completion.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
Only 21 of 86 test_*.c group functions called their own nested
UNITY_BEGIN()/UNITY_END(). Each nested call reset Unity's global
NumberOfTests/TestFailures counters, so controller_test.c's final
`return UNITY_END();` -- the value meson test actually gates on --
only ever reflected whichever small window of tests ran since the
LAST nested reset, silently discarding pass/fail evidence for
everything before it (measured: 16 of 972 total tests in one run).
The same nested-BEGIN calls also caused PASS/FAIL banners to show
the wrong source file for tests in files that don't reset it
themselves (Unity.TestFile just carries over from whichever group
last called UNITY_BEGIN()).
Fix: replace every nested UNITY_BEGIN() with UnitySetTestFile(__FILE__)
(sets file attribution only, no counter reset) and drop the matching
UNITY_END(); add the same UnitySetTestFile(__FILE__) as the first
statement to the other group functions that had neither. The 2
genuinely standalone binaries (test_job_array_sched_push.c,
test_job_array_apiserver_push.c -- each has its own int main(), not
registered via controller_test.c's TEST() macro) are untouched.
Verified: single consolidated summary now reads
"972 Tests 0 Failures 0 Ignored OK", exactly matching the
ground-truth RUN_TEST call-site count across all 83 registered
group files (976 total minus 4 belonging to the 2 standalone
binaries). Every PASS/FAIL banner now names its own true source
file. Full xce-build.sh all regression pass clean (Ok: 9, Fail: 0).
No production code touched -- test-harness plumbing only.
Root cause (confirmed via targeted DIAG1419 logging on a live chaotic soak):
a client's commit-timeout retry during a genuine quorum-loss window appends
a second WAL_JOB_SUBMIT for the same jobid; the timed-out entry is never
retracted, so if it later achieves majority commit anyway both entries end
up committed. This is a legitimate, harmless duplicate — restore_job()'s own
-EEXIST correctly no-ops the second one — but replay_wal() tallied every raw
WAL_JOB_SUBMIT occurrence instead of genuine creations, so the duplicate
made new_job_submits disagree with jobmgr's real growth, firing a false
APPLY_DIVERGENT and deadlocking promotion.
Reverts the layer-3 claim/release machinery (job_wal_commit_claim/release,
claim_committed_job_submits, and their wiring): three attempts at preventing
the duplicate at submit/dispatch/commit time all failed to close the gap,
because the duplicate isn't a bug to prevent — a WAL entry can't be
retracted once appended, so any submit-time guard is structurally the wrong
layer.
Real fix: apply_committed_entry()'s WAL_JOB_SUBMIT case now returns 1 for a
genuine creation vs 0 for an idempotent no-op (restore_job() -EEXIST), and
replay_wal() only tallies new_job_submits_out on rc==1. Updates the affected
apply-seam/reapply-idempotency tests for the new return contract, and
reverses two apply-correctness tests whose premise (duplicate-jobid replay
must diverge) is now understood to be the actual bug rather than the
expected behavior — they now force divergence directly via the same
production fencing seam instead of relying on that trigger.
Verified: two consecutive 10-round chaotic soaks, zero divergence, zero job
loss, no election stall (previously reproduced within ~5 rounds).
Add test_apply_reapply_idempotency.c (3 tests, no production changes):
triple-reapplication of an identical job submit with hook-count and
pointer-identity assertions across every call, the NOOP_DUPLICATE
guard branch (untested by task 2.2), and the task's named scenario --
real SUBMIT then real FINISH then a stale out-of-order START, all
through the real apply_committed_entry() dispatch -- confirming the
job stays terminal with no spurious notification.
Closes out task-group 2 (shared apply seam for committed entries).
