clitest_spawn/send_line/send_partial/send_tab/send_signal/wait_for_text/
wait_for_exit/kill_and_reap launch the real compiled xce_test behind a
genuine pty (so isatty/g_interactive/libedit behave as a real terminal),
with stderr routed to a separate pipe for real stream-separation testing.
All waits are poll()-bounded with an explicit timeout so a future
regression can't hang the whole test suite. This is the shared
infrastructure tasks 2.3, 3.5, and all of Validation phase (4.1-4.4)
will build their black-box scenarios on top of. One smoke test proves
launch/send/capture/exit-code/stream-separation end to end.
1290/1290 controller suite passing (+1).
Task 1.7 of .kiro/specs/xce-test-cli-ux/tasks.md.
Foundation phase (1.1-1.7) now complete.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
argp already provided --help/-?/--version with exit-without-REPL
behavior, but had no -h alias (its default short help flag is -? per
GNU argp convention) despite Requirement 4.1 naming -h explicitly.
Added a hidden -h option triggering the same help path, and extended
the doc string to describe the in-session help command. Proven against
the real compiled binary via subprocess tests, not just argp's own
in-process behavior. 1289/1289 controller suite passing (+4).
Task 1.6 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 4.1, 4.2, 4.3
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
A registry of per-connection locks, acquired (interruptibly) before
spawning a worker and released by whichever side finishes last, so an
abandoned call's worker is never running concurrently with a new call
on the same connection. Uses a POSIX semaphore (sem_t), not a mutex:
review round 1 found the initial pthread_mutex_t implementation relied
on behavior POSIX classifies as undefined for a non-owning-thread
unlock (not just error-checking mutex types) -- this design's worker
thread legitimately releases a lock the caller thread acquired, which
sem_t supports with no ownership concept at all. bug-1791. Design.md
synced to reflect the semaphore choice. 1285/1285 controller suite
passing.
Task 1.5 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 1.1, 1.5
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
xcetest_run_interruptible() runs a blocking call on a detached worker
thread while the caller polls xcetest_interrupted() every 50ms. A single
atomic CAS (RUNNING -> DONE_UNCLAIMED | ABANDONED) decides which side
frees the heap-allocated call block, so an abandoned call's worker
thread never double-frees or leaks once it eventually finishes. Proven
under active ASAN with a real detached thread and a real SIGINT-forced
abandonment. 1282/1282 controller suite passing (+2).
Task 1.4 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 1.1, 1.2, 1.3, 1.4
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
Replaces the bare signal(SIGINT, sigbreak) install with sigaction() and
SA_RESTART cleared, so a blocking read reliably returns EINTR instead of
being silently restarted by glibc's BSD signal() semantics. Adds
xcetest_interrupted() as a read-only accessor over the existing break_out
flag for the interruptible-call adapter (tasks 1.4/1.5) to consume.
Proven by a real fork+SIGINT+blocked-read test that first demonstrates
the old bug, then the fix. 1280/1280 controller suite passing (+2).
Task 1.3 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 9.9
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
flagspec_reject() now names a likely-intended flag when an unrecognized
flag closely matches exactly one entry in the descriptor table (genuine
Levenshtein DP, silent on zero or ambiguous matches). flagspec_render_help()
generates the short/long/required-optional/default flag-listing block from
a descriptor table. 1278/1278 controller suite passing (+5).
Task 1.2 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 6.1, 6.2, 6.3, 6.4, 8.2, 10.1, 10.2, 10.3
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
Adds flagspec_t descriptor table, getopt_long option-table builder, and
the shared stderr-routed rejection path that every flag-accepting
command will migrate onto in later tasks. TDD'd via test_flagspec.c
(1273/1273 controller suite passing).
Task 1.1 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 2.1, 2.2, 2.3, 2.4, 3.1
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
Comment-only edits in 4 files: test_ha_bootstrap.c/test_ha_ctrlhello.c
rename their "make test-integration" pointers to reference the
regression suite (tests/regression/run.sh) directly; apiserver/xcecli
cmd/main.go drop the Makefile clause from their ldflags-injection
comments, keeping only src/tools/go_build. Zero production/test
logic changed -- verified via full controller unit-test suite rerun
(1261 tests, 0 failures) and xcecli build.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
Closes bug-1755: worker_send_resync_query() hardcoded pktlimit=0 in
its xce_job_state_query_req() call, so the controller's outbound
OPCode_jobStateQuery always sent every non-terminal job ID in one
unbounded page regardless of count - the query direction never
actually exercised the pagination mechanism task 1.4/7.4 proved
correct, unlike the report direction's real, wired
XCECD_RESYNC_REPORT_PKTLIMIT bound.
Add XCE_RESYNC_QUERY_PKTLIMIT=100, mirroring that exact precedent
(same value - a JobStateQuery entry is a bare jobid, smaller than a
JobStateReport entry), and thread it into the one call site. Existing
small-job-count behavior is unchanged; a rejoin exceeding the cap now
genuinely produces multiple wire pages, proven against the real
production call site (not a bypass) with a 250-job/3-page test.
This is the last task in worker-rejoin-resync's implementation plan.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add test_large_job_count_pagination_correctness, proving both
OPCode_jobStateQuery and OPCode_jobStateReport genuinely span many
wire pages (1042 jobs, 11 pages each direction, including a partial
final page) with zero jobs dropped/duplicated/truncated - exact-count
assertions throughout (page continuity, no-duplicate bitmap, exact WAL
delta, exact hook-fire count), extending task 6.2's 2-page proof by an
order of magnitude. Test-only, no production changes.
Logs bug-1755: the underlying query-pagination mechanism this test
proves correct is never exercised in production - worker_send_resync_
query() hardcodes pktlimit=0, so the controller's real query send
always produces one unbounded page regardless of job count, asymmetric
with the report direction's real XCECD_RESYNC_REPORT_PKTLIMIT bound.
No hard transport-layer size cap exists, so this isn't a demonstrated
failure mode, but it contradicts design.md's explicit "both paginated"
claim in this feature's own headline mechanism. Adds task 7.5 to close
it with a small, well-scoped fix before the feature-level gate, rather
than leaving a silently-false design claim in the completed feature.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add test_burst_recovery_unbounded_by_job_count, reproducing bug-1741's
exact original shape (7 jobs on one host, 6 lost finish reports) in
one reconciliation round and one wire page: 5 finished-success + 1
finished-failure recover correctly and distinctly, while an already-
DONE job re-reported as an unqueried extra (with a deliberately wrong
finish time) is proven structurally unchanged - exact +6 WAL LSN
delta, not >=, so a duplicate append is genuinely caught rather than
silently tolerated. Test-only, no production changes; deliberately
kept to a single page since page-boundary correctness is task 7.4's
separate concern.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add test_failover_during_job_recovers_lost_finish, a focused proof of
the feature's own original motivating scenario (bug-1741): a job
dispatched and RUN with no finish fact ever committed anywhere
(modeling a lost fire-and-forget finish notify), recovered to DONE
with the worker-reported finish time - not stuck in RUN - via the
real rejoin/query/report/apply chain. Test-only, no production
changes; reuses task 7.1's helper infrastructure rather than
duplicating its broader four-outcome coverage.
Updates the existing bug-924 (/tmp exhaustion / flaky teardown) entry
with a 5th occurrence hit during validation, unrelated to this task's
own change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add test_full_rejoin_reconciliation_round_trip proving the whole
chain composes end-to-end in one round, for a mix of still-running,
finished-success, finished-failure, and unrecognized jobs: real
rejoin_worker() -> worker_send_resync_query() (real wire query) ->
hand-built worker answer through the real report codec ->
resync_handle_report_page() -> resync_apply_report(), asserting
correct final state and scheduler notification for each. Test-only,
no production changes.
Logs bug-1754: a worker-reported job failure's exit code is never
durably recorded anywhere in this codebase (distinct from bug-1747's
finish-time gap) - discovered honestly asserted by the new test,
out of scope for this validation-only task.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add OPCode_jobStateReport dispatch in myreq_handler, reassembling a
report across multiple incoming pages before handing it to task 3.2's
resync_apply_report() exactly once - proven via a test-only call
counter, not just end-state equivalence (a double-apply could pass an
end-state check if idempotent).
Standby-redirect investigation (Req 8.1/8.3): confirmed
ha_dispatch_gate already redirects every rejected opcode uniformly, so
no opcode-specific code was needed for that. OPCode_jobStateQuery has
no controller-side receive path at all (only sent, never received)
so needed no gate change either.
That same investigation found a different, more consequential gap:
fixes bug-1753, ha_dispatch_gate's quorum-loss fact-admission allowlist
(OPCode_notify/update/setpreason/clrpreason) was missing
OPCode_jobStateReport, so a report arriving during a transient
quorum-loss window - this feature's own core failover scenario - would
be rejected outright with no retry (worker sends fire-and-forget).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Arm a reconciliation round in apply_join_state() the moment a worker
host becomes connected (first join, reconnect, or failover-reconnect -
all routed through the same shared call site, Req 1.1-1.3), then send
the query from on_worker_join() strictly after the join reply and
existing config push (Req 2.1-2.2), verified via a structural test
that reads real wire order off the endpoint queue.
Also closes Req 3.7's config-delivery seam end-to-end: worker_resync_
window_ms now rides the existing Conf4Worker push (clustername
precedent, no new opcode) and the worker applies it to its retention
window on receipt.
Fixes bug-1752: myreq_handler had no OPCode_push dispatch case at all,
so every config push - including the pre-existing clustername field -
was silently discarded. Confirmed harmless until now (clustername was
never read anywhere in src/worker/); closed narrowly for
worker_resync_window_ms only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add resync_apply_report()/resync_apply_running_entry()/
resync_apply_finished_entry()/resync_apply_unrecognized_entry() to
resync.c: routes each entry in a worker's job-state report to the
correct existing idempotent path (still-running -> clear from outstanding
set, no fact; finished-success/failure -> record_job_finish_fact_at/
on_job_fail; unrecognized -> record_job_reconcile_pending_fact;
unqueried extra -> record_job_reconcile_assign_fact or no-op), discards
reports for superseded rounds (Req 6.1/6.3), and resolves the round once
its outstanding set empties.
Also fixes bug-1747 (Req 4.2): adds record_job_finish_fact_at(),
additive alongside the unmodified record_job_finish_fact, so a
finished-success reconciliation entry records the WORKER-reported
finish time instead of reconciliation-processing time -- proven via a
real WAL-write + persist_recover() restart-replay test. The FAIL-side
half of the same gap (WAL_JOB_FAIL carries no fail-time wire slot at
all, for any caller) is confirmed pre-existing and systemic, correctly
left out of this task's scope.
Three review rounds surfaced and fixed two real, distinct bugs in the
unqueried-extra-job routing guard (see tasks.md's Implementation Notes
for the full account and the general lesson about re-deriving a full
truth table after a partial boolean-guard fix): round 1 caught the
wrong axis (terminal-vs-non-terminal instead of assignment-status,
Req 4.8); round 2 caught that the first fix's short-circuit clobbered
an already-terminal job's DONE/EXIT back to RUN (Req 4.4), including
confirming the implementer's own test had certified the bug as correct;
round 3 independently re-derived the final three-way guard from scratch
and found no further defect. bug-1748 logged for the guard fix.
Full controller suite: 1239 tests, 0 failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
New src/controller/resync.c/resync.h: resync_arm_round(cluster_t*,
worker_t*) enumerates every non-terminal job assigned to a host,
populates worker_t.resync (round_token, query job-ID set, deadline),
sets XCE_WORKER_RECONCILING, and schedules a non-blocking delaymgr
one-shot timeout. resync_resolve_round() clears the round; a fired
timeout resolves it too, via resync_round_is_current()'s by-value
round-token check -- delay.c/delay.h expose no true cancel/dequeue
primitive, so a stale timer from an already-superseded round becomes a
safe no-op instead of touching a newer round's live state (verified
safe against both misresolving the wrong round and use-after-free: the
timeout closure never holds a pointer into resync_state_t itself).
5 new tests: query-set correctness, fresh-token-per-arm, the
RECONCILING flag's set/clear lifecycle, and a direct (non-blocking)
timeout-fire proof via an XCE_TESTING-gated seam confirmed compiled out
of the production binary. Full suite: 1232 tests, 0 failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Split dispatch_a_job_()'s combined "(w->ep == NULL) || (w->host.status
!= 0)" check into two branches, checked in order: w->ep == NULL now
resolves directly via record_job_reconcile_pending_fact() (task 2.1) --
no dispatch_fail_count increment, no mark_job_fail(), never reaching
xce_classify_dispatch_fail() at all -- before the code ever tests
w->host.status != 0, which keeps its existing retry-then-terminal-fail
behavior completely unchanged (byte-identical body, confirmed against
HEAD).
Completes Task 2 (all controller job-table reconciliation outcomes:
2.1 reconcile-to-pending, 2.2 reconcile-assign, 2.3 this split).
RED phase caught a genuine pre-existing test bug: test_worker_notready_
resolves_retryable was seeding a NULL-ep worker while claiming to test
the host.status!=0 retry path -- it never actually exercised that path.
Corrected to use a connected-but-busy worker, with self-asserting
TEST_ASSERT_NOT_NULL(w->ep)/TEST_ASSERT_TRUE(status!=0) guards so this
can't silently drift again.
2 new tests, including a full JOB_TRANSITION_APPLIED path proof (WAL
LSN advance, hook-fires-exactly-once per bug-1679, allocs released per
bug-1631) through this exact call site, both using correctly-timed
canary references on both the job and worker objects to catch any
over-release under ASAN. Full controller suite: 1227 tests, 0 failures,
zero regressions across every other dispatch_a_job_ failure site.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add record_job_reconcile_assign_fact(cluster_t*, job_t*, worker_t*,
xce_job_state_entry_t*) to job.c: eagerly sets job->job.state to match
the worker's reported outcome (mirroring dispatch_a_job_'s inline
mutation, minus allocation fields -- there are none to set, job->job.allocs
stays NULL per Requirement 4.8's documented scope). Calls the existing
persist_submit_job_dispatch_fact(..., alloc=NULL) directly, then leaves
the post-unlock persist_notify_job_updated_already_persisted() call to
the caller, same convention as task 2.1.
Fixes bug-1746 along the way: capnp_encode_joballoc() (persist.c, part
of bug-1678's original fix, predates this feature) unconditionally
dereferenced a NULL alloc -- never triggered before since
dispatch_a_job_ (the only prior caller) always passes non-NULL. Fixed
with a minimal guard producing a true zero-length buffer, matching
apply_committed_entry's alloc_len > 0 decode gate exactly. This
directly contradicted design.md's "tested, backward-compatible on both
seams" claim (true only for decode, never actually exercised for
encode) -- corrected in 4 places.
2 tests: applies-running-no-allocs-and-notifies, and a real restart-
replay round trip (write to real disk, fresh persist_recover(), confirm
jobid/host/state/NULL-allocs survive) -- proving this task's own
"durably survives a restart replay" completion criterion literally, not
just the in-memory mutation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
Add record_job_reconcile_pending_fact(cluster_t*, job_t*) to job.c:
eagerly applies apply_job_update_transition(job, JOB_STAT_PEND) (real,
live mutation, not a deferred peek -- matching dispatch_a_job_'s
established pattern, not record_job_finish_fact's), releases
job->job.allocs on an applied (non-no-op) transition, then durably
appends via the generic persist_submit_job_fact(WAL_JOB_UPDATE). Caller
holds job's lock throughout and is responsible for calling
persist_notify_job_updated_already_persisted() after unlocking -- this
function deliberately does not call it itself.
Closes bug-1631 at this new call site: a RUN job with a real allocation
reconciled to PEND (Requirement 4.6, worker reports it unrecognized)
would otherwise carry a stale allocation into its next dispatch attempt,
exactly the gap bug-1631 documented as dormant until something re-admits
an already-dispatched job to the scheduler's PEND pool -- this feature
is that path. dispatch_a_job_ itself is unchanged (still correct for a
job's first-ever dispatch, the only case reachable there).
3 tests: allocs release + eager-mutation-before-unlock proof, no-op on
already-pending, and the Requirement 4.4 terminal-state-preservation
proof (rejects overwriting JOB_STAT_DONE).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
sched_job_exit_fun() (renamed sched_job_finish_fun, sched.c) subscribes
to HOOK_DETAIL_JOB_EXIT, which fires for EVERY job finish, both success
and failure -- record_job_finish_fact() (job.c) decides DONE vs EXIT
from finish->exit_code BEFORE firing this same hook detail either way
(mirrors persist.c's own HOOK_DETAIL_JOB_EXIT -> WAL_JOB_FINISH
mapping). This function discarded info->exit_code and sent only a bare
jobid, forcing sched_handle_notify() (scheduler/src/handler.c) to
unconditionally map EVERY finish -- including a clean, successful one
-- to JOB_STAT_EXIT. A subsequent HOOK_DETAIL_JOB_UPDATE notify usually
self-corrected this moments later via the full job state, but if that
second notify was dropped during a scheduler reconnect gap (the same
window bug-1731/bug-1734 already documented), the scheduler was left
permanently stuck showing EXIT for a job the controller correctly
recorded as DONE.
Live-reproduced on a real 3-node HA cluster + worker: job
0-8-1788143749-0[38] genuinely completed successfully (worker log:
"completed successfully", controller joblist: DONE) but the
scheduler's own replay showed EXIT.
Fix: carry {jobid, exit_code} across the wire via the existing
JobFinishInfo shape (the same struct apiserver_job_exit_fun already
reuses for this identical hook) instead of a bare UInt64, so
sched_handle_notify() can make the identical exit_code==0-vs-not
decision job.c itself makes. Renamed sched_job_exit_fun to
sched_job_finish_fun to reflect what it actually does.
Live-verified: a 51-job run (all exit 0) showed 100% DONE on the
scheduler side after the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
The oldest unresolved bug in this project's tracker (first sighted
2026-06-20): controller_unit_test intermittently hung for the full
300s meson timeout with no crash/ASAN report, never previously caught
live under gdb.
Live-reproduced this session (first attempt): thread 1 stuck forever
in commit_engine_fini() -> pthread_cond_destroy() (test_commit_barrier_
rollout.c's jap_teardown), while two fsync_worker threads sat parked
in pthread_cond_timedwait on the same persist_t. Root cause: several
test teardown functions called commit_engine_fini() BEFORE
persist_stop() -- the exact opposite of commit_engine_fini()'s own
documented contract. This let the live background fsync_worker thread
call back into commit_on_durable_advance() while commit_engine_fini()
was mid-destroy of the engine's lock/condvar -- a genuine TOCTOU race
with no synchronization between the two threads.
This is the same race already root-caused once for a SEGV symptom in
bug-1452 (2026-07-19, test_persist_truncate.c) and partially re-fixed
in bug-1687 (2026-08-25, cbrp_teardown) -- but never swept across all
affected files. Bug-1452's own remediation note explicitly flagged
test_demotion_convergence.c as a known, unfixed instance; that flag
went unactioned until now.
Fix: insert persist_stop() immediately before commit_engine_fini() at
21 confirmed-vulnerable sites across 12 files (test_apply_pump.c,
test_apply_pump_integration.c, test_commit_barrier_rollout.c,
test_commit_durability.c, test_commit_engine.c,
test_convergence_verification.c, test_demotion_convergence.c,
test_egress_filter.c, test_ha_role_lifecycle.c, test_job_array_fence.c,
test_job_array_replay_fence.c, test_p2_integration.c) -- the same
pattern bug-1452/1687 already established. persist_stop() is
idempotent (guarded by an atomic "already running" check), so each
fixture's own pre-existing persist_stop() call becomes a safe no-op.
No production code changed.
Known remaining exposure, documented but not fixed this session:
test_config_repl.c (~23 sites) and test_term_safety_adapter.c
(~5 sites) have the same ordering bug in their own teardown helpers,
flagged as follow-up.
Verified: full controller suite run 4 times total (once as part of
the fix, three further independent sequential runs) -- Ok:6 Fail:0
every time, 38-42s wall clock each run, no hang, nowhere near the
300s timeout. 1220/1220 individual controller_unit_test cases pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
The worker's rusage collector already measures and sends
"memory_swap"/"tasks_current" per job, but xce_host_runusage_t had no
field to receive them at all -- a complete dead end, not a naming
mismatch like bug-1735's cpu_time/mem_usage. Added mem_swap/
tasks_current fields (types.h), matching capnp fields on HostRunusage
(controller.capnp, new ordinals @3/@4) for WAL field-completeness, and
wired update_host_runusage() (misc.c) to parse them. Also fixed
merge_job_runusage()'s (job.c) existing-host merge branch, which only
copied cpu_time/mem_usage per host -- the two new fields would
otherwise have been silently dropped on WAL replay/catchup for a host
already seen.
Also computed peak_cpu_time as the running max of aggregate_job_
runusage()'s per-call cpu_time sum across every report a job has ever
received, guarding against a later report reducing it.
Deliberately NOT computed: peak_cpu_duration and the three efficiency
fields (mem_efficiency/avg_cpu_efficiency/peak_cpu_efficiency) -- these
need a "used vs. requested" ratio against a job's resource request (a
resreq expression tree, not a plain core/slot count) and/or an
unspecified duration semantic. A documented scope decision, not an
oversight -- see aggregate_job_runusage()'s doc comment.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
Two independent gaps in misc.c/job.c, both silently zeroing every real
job's resource-usage data:
- update_host_runusage() matched worker rusage report resnames
"cputime"/"mem", but the real worker's collector
(collect_job_rusages(), xcecd_cgroups.c) sends "cpu_time_sec"/
"memory_rss" -- the two never matched, so cpu_time/mem_usage stayed
at their calloc()'d zero for every non-simulated job. Aligned both
sides, including xce_test's own simulated-worker fixture.
- update_job_rusages() had a standing FIXME: xce_job_runusage_t's
job-level max_mem/avg_mem/used_mem/cpu_time were never derived from
the per-host host_runusage[] entries. Added aggregate_job_runusage()
(sum/max/avg across hosts) and wired it in right after the per-host
update.
Added test_host_runusage.c (standalone unit test, mirrors
resval_typeresolve_unit_test's isolation pattern) plus the
controller_unit_test stub aggregate_job_runusage() needs since that
target excludes the real misc.c.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
Two root causes, confirmed by direct code reading (previously misattributed
to an overlayfs sandbox fdatasync artifact):
conf_trim_old_versions() sorted config-version files by st_mtime (second
resolution). The test writes 3 versions 15ms apart -- well within one
second, so ties are the expected outcome, not a rare edge case. qsort()
isn't stable, so a tied ordering could push the protected current-pointer
file (skipped on deletion, never compensated) past keep_n, leaving one
extra file on disk ("Expected 2 Was 3"). Fixed by sorting on st_mtim
(nanosecond-resolution struct timespec) instead of st_mtime.
cr_setup() keyed its tmpdir on getpid()+time(NULL) (second resolution),
risking collision with a stale directory left by an earlier interrupted
run (ASAN abort, timeout, SIGKILL -- none reach cr_teardown()'s rm -rf)
reusing the same PID within the same second -- test_conf_get_saved_tag_
no_config then observed that directory as "not actually empty" and read
a leftover tag instead of the expected 0. Fixed with a monotonic counter
guaranteeing a unique directory per cr_setup() call.
TDD: RED reproduced on the first attempt (both tests failed with the
exact historical signature). GREEN confirmed across 16+ repeat runs
after the fix, zero other regressions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
Follow-up to ebe691c3 (HOOK_DETAIL_JOB_START/JOB_EXIT), same gap for job
failures: apply_committed_entry()'s WAL_JOB_FAIL case only ever fired
the generic HOOK_DETAIL_JOB_UPDATE, leaving sched_job_fail_fun (sched.c)
and apiserver_job_fail_fun (apiserver.c) dead -- the apiserver's
OnJobFinished was starved of data for job failures specifically (the
prior fix only closed this for clean finishes).
Fires HOOK_DETAIL_JOB_FAIL only for the WAL_JOB_FAIL_STAGE_FLAG branch,
which already constructs a real job_failinfo_t -- the legacy _pad==0
absolute-overwrite branch has no fail-specific data and must not
fabricate one, mirroring how the WAL_JOB_START fix only fires when
pidinfo was genuinely decoded. Unlike WAL_JOB_START, no gate-widening
was needed: apply_job_fail_transition() always returns
JOB_TRANSITION_APPLIED for a genuine new fail transition (confirmed by
reading it), so the existing result-gated early return already covers
this correctly.
TDD: RED confirmed (fix reverted, test failed "Expected 2 Was 1"),
GREEN after restoring it. Full regression: 1220 tests, only the known
bug-1561 flake.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
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
Chaotic HA soak (post-bug-1695) reported committed_job_loss every
round. raft_cb_become_leader() computed the promotion WAL-truncation
floor and evaluated the full promotion-eligibility gate (durable_floor/
recovery_truncated/apply_verdict) BEFORE cluster_init_phase2() -- the
only call that runs persist_recover() on this process's first-ever
boot. Those fields all come from persist_recover(); read before it
runs, a freshly persist_init()'d struct reports durable_lsn=0, making
the eligibility gate wrongly judge a genuinely corrupt/divergent node
ELIGIBLE. Unreachable before bug-1695's fix (a restarted node's stale
term previously always made it defer to a peer instead of re-electing
on the same boot with real data at stake) -- bug-1695 correctly
restored real re-election on restart and newly exposed this
pre-existing boot-ordering flaw.
Reordered raft_cb_become_leader() so cluster_init_phase2()/
persist_recover() runs before the eligibility gate and truncation-floor
computation. Split the pure vote-derived safe_committed_lsn computation
(reads only ha->elect_ctx, safe pre-recovery) from the truncation-floor
computation (needs post-recovery persist state) -- the former stays
early, the latter moves to run after phase2. New regression test
CRC-corrupts a real on-disk WAL and confirms first-boot promotion now
correctly refuses instead of destroying committed data.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
xceadmin scheduler record status/start/stop against a standby failed
with the bare "controller returned status -10014" (E_XCE_STANDBY) with
no redirect, forcing the caller to guess the primary manually.
bug-1700: myreq_handler's generic ha_dispatch_gate rejection path
(cluster.c) now calls fill_redirect_reply() when rc == -XCE_ERR_STANDBY,
mirroring the same helper OPCode_join's handler already uses --
-XCE_ERR_NOT_READY (CTRL_INIT/CTRL_ELECTING, no settled primary to
redirect to) is deliberately excluded. Landing this alone had zero
observable effect live: ha_state_t.primary_host/primary_port were never
written by any production path on a standby (only test fixtures set
them) -- fill_redirect_reply() always saw an empty/zero redirect target
in a real deployment.
bug-1701: closes that data gap with a pure local lookup, no new wire
field. handle_ctrlhello() (config_repl.c) now resolves the ctrlhello
sender's name against this node's own statically-known peers_cfg
(parsed once from --peers at boot) and populates primary_host/port at
the exact moment a standby first learns who primary is.
Live-verified end to end on a fresh 3-node cluster: xceadmin scheduler
record status against any standby now prints "redirected: <standby> ->
<primary>" and returns the real status, confirmed again after a
re-election changed which node was primary.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
dispatch_a_job_() synchronously sets state=JOB_STAT_RUN at dispatch time
(scheduler-dispatch-commit task 3.1's deliberate no-worker-ack-wait
semantic). By the time the worker's later WAL_JOB_START fact commits,
apply_job_update_transition() sees state already == RUN and returns
NOOP_DUPLICATE instead of APPLIED -- so bug-1687's start_time/pidinfo
decode/adopt block, gated on result==APPLIED, could never run for any
job. start_time (and consequently run_time) stayed 0 forever. Same bug
class already fixed for WAL_JOB_RUSAGE/CKPT/PREASON in this identical
apply function; WAL_JOB_START was missed because bug-1687 predated the
dispatch-time RUN semantic change.
Moves start_time to dispatch-owned: dispatch_a_job_() now stamps
job.start_time alongside state=RUN, under the same write lock.
WAL_JOB_DISPATCH gains a flag-gated (WAL_JOB_DISPATCH_HAS_START_TIME_FLAG)
start_time field so it survives replay/restart; both apply seams
(apply_committed_entry, apply_wal_entry) and restore_job_dispatch_state
decode it when present, falling through to the old alloc-only shape
otherwise. WAL_JOB_START's own now-permanently-dead start_time write is
removed; pidinfo adoption made unconditional to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Now that xce_test auto-loads a personal rc file at startup (this spec),
harden the three existing non-interactive xce_test callers so a future
developer's ~/.config/xce_test/rc can never silently affect a CI/soak run:
run-ha-soak.sh, run-resource-verify.sh, and clean_failover_preservation.sh
all now pass --norc to their xtest() invocation. No behavior change today
(no rc file exists in this environment yet, per xce-test-rc-file task 3.3's
own verification) -- this is forward-looking reproducibility hardening.
Independently re-verified end-to-end: run-ha-soak.sh (--sequential, 2
rounds, zero job loss) and run-resource-verify.sh (1 resource, 3 layers,
zero mismatches) both PASS unchanged with --norc added.
clean_failover_preservation.sh could not be run in this sandbox -- its
DATADIR precondition (<repo>/ha/data/cluster.bin + certs) was deleted by an
unrelated earlier commit (ee8473bd, admin-config-tool) and never restored;
confirmed via git history this is pre-existing and unrelated to this edit
(the precondition check runs before xtest() and fails identically on the
unmodified script). Logged as bug-1721, not fixed here -- out of this
spec's scope.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Extends test_rcfile_integration.sh with 2 scenarios proving Requirements
3.1-3.4: a failing rc-file command in piped/scripted mode aborts before
any of the piped script's own commands run, exiting nonzero; the same
failing command in an interactive session still reports the failure,
continues executing the rc file's remaining lines, and reaches the normal
prompt -- while still carrying the session's sticky nonzero exit-code
verdict (g_session_failed is never cleared, matching how any other
failing-command-then-quit interactive session already behaves).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Adds test_rcfile_integration.sh, mirroring clean_failover_preservation.sh's
shell-driven black-box pattern against the real built xce_test binary:
- a valid rc file's set masters override takes effect in both a piped run
and a genuine pty-driven interactive run, before the normal session begins
- --norc leaves the default masters value untouched with the same rc file
present
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Adds the integration call site in main(), between g_continue_on_error's
computation and the read loop: unless --norc is given, resolve the rc-file
path and load it through preprocess_and_run (the same command-execution
path used for typed/piped input). A NULL path (no XDG/HOME) or missing
file falls through silently to the read loop. A failing rc-file command
under scripted/piped abort-on-first-failure mode short-circuits straight
to the existing g_session_failed exit-code convention, never entering the
read loop -- so a piped script's own commands never run after an rc-file
failure. Verified end-to-end against the built binary, including a real
pty for the interactive path, in both the failure and success cases.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Adds struct arguments.norc, a --norc argp option, OPT_NORC (matching
main.c's OPT_PEERS/OPT_CONF_KEEP_N idiom), and its parse_opt case to
test.c. Parsing/recording only -- no wiring into main()'s control flow
or rcfile.c yet (task 2.2).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Adds rcfile_load(path, runner) to rcfile.c/rcfile.h: reads lines with
getline() (no fixed-size buffer, so a long line like a multi-peer
`set masters ...` can never be silently truncated or split), strips the
trailing newline, invokes the caller-supplied runner once per line, and
stops at the first nonzero runner return, propagating that value. A missing
rc file is a silent no-op; any other open failure prints one stderr warning
and returns 0.
Found during GREEN: fopen() succeeds on a directory path on this platform,
so the EISDIR failure only surfaces on the first getline() call (ferror(f)
set) -- added a post-loop ferror() check to catch it (bug-1716).
5 new unit tests (11 total with task 1.1): ENOENT-silent, per-line-in-order
with newline stripped, stop-on-nonzero with exact propagation, non-ENOENT
open-failure warning, and a >4KB single line proving no truncation/split.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Adds rcfile_resolve_path() (rcfile.c/rcfile.h): resolves $XDG_CONFIG_HOME/xce_test/rc
when set and non-empty, else $HOME/.config/xce_test/rc, else NULL. Standalone
libc-only module mirroring the configcompare.c pattern; wired into xce_test_src
and controller_unit_test_src, registered in controller_test.c's Unity runner.
6 unit tests in test_rcfile.c cover all XDG/HOME combinations plus a dedicated
env-var save/restore isolation test, addressing the cross-test leak risk found
during design review (controller_unit_test runs 80+ test files in one process).
Adds the xce-test-rc-file spec (requirements/design/tasks/research), including
design fixes from validation: getline()-based line reading (task 1.2, avoids
truncating long rc-file lines) and the env-isolation requirement above.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Found via the mandatory live HA chaotic soak for this session's controller
audit campaign -- unrelated to any of the 9 audit fixes (bug-1684/1687/
1689/1690/1691/1692/1693/1694), all in ha.c/persist.c/job.c/cluster.c/
sched.c/worker.c; this is in myapp.c, untouched by any of them.
myapp_run()'s bootstrap "is a leader already known" check had a fallback
`c.ha.raft.term > 0` meant to detect genuine new raft activity processed
during the startup-stagger nap. But ha_restore_term() loads ha->term from
disk BEFORE this check runs, on every boot -- and Raft terms only ever
increase, never reset. So `raft.term > 0` is trivially true on every
restart for the rest of the cluster's on-disk lifetime, once it has ever
held one election, regardless of whether any leader is currently
reachable. Live-reproduced: every node, on every restart after the
cluster's first election, unconditionally skipped ha_start_election_async
forever -- no node could ever re-assert leadership through a real
election again. bug-1477's existing cluster_init_phase2 retry loop kept
firing correctly (confirmed in the reproduction logs) but could never
succeed, since the deeper cause -- no election ever runs to produce a
live primary to push config from -- was never addressed by that fix.
Fixed by snapshotting `term_at_boot` right after ha_restore_term()/
ha_init_raft() seed the term, strictly before the stagger nap, and
comparing `raft.term > term_at_boot` instead of `raft.term > 0` -- only a
term that has genuinely ADVANCED during this boot cycle (real vote/
heartbeat traffic processed) counts as evidence a leader is known. The
decision was extracted into a new, unit-testable myapp_leader_already_known()
(startup_validation.c, since controller_unit_test deliberately excludes
myapp.c/main.c). voted_for_name/vft==cur and primary_ctrl_fd>=0 checks
are unchanged.
7 new tests in test_ha_startup.c; RED/GREEN independently verified.
Rigorous A/B live-soak proof (identical 6-round --chaotic run, fixed vs
git-stashed-back-to-original): baseline showed term frozen for the whole
run and zero election activity on any non-primary node; fixed run showed
term genuinely progressing and ctrl-2 achieving a real, log-verified
CTRL_PRIMARY transition via a won PreVote round -- proof a node other
than the original primary can now take over, which was structurally
impossible before. Full controller suite: 1191 tests, 2 failures -- both
the pre-existing, unrelated bug-1561 overlayfs sandbox flake.
Residual, NOT independently verified yet (flagged honestly, not fixed
here): the fixed run's chaotic verdict showed committed_job_loss=6
(baseline 0) -- traced via waldump to one jobid missing from every node's
WAL, at the cold-boot window before round 1. Resembles the already-fixed
bug-1487 false-positive class (apply-pump-lag racing verify_classify's
expect-job check) but the reported mechanism doesn't perfectly match
(bug-1487's job was present-in-WAL/not-yet-visible; this one is described
as absent from every WAL entirely) -- needs independent re-verification
before being dismissed, tracked as a follow-up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Last of the 9 confirmed findings from the /xce-audit-invariants controller
pass. checkpoint_task_fn was registered directly as a delaymgr entry and
ran its full blocking body inline: checkpoint marker pwrite+fdatasync,
write_snapshot()'s genuinely unbounded per-entity walk (scales with
cluster state size) held under apply_pump_lock, WAL trim, and snapshot
rotation/unlink -- all on the same thread that drives heartbeats/election
timeouts. Unlike every other delaymgr operation (snapshot send,
config-push, reconnect, catch-up), checkpoint had no offload mechanism
at all -- the one gap left from the raft-drive-async hardening pass.
Design: full offload via the SlowLane (slowlane_submit), default ON --
NOT the vtask pool. persist_apply_pump_run() already holds apply_pump_lock
across its own per-entry walk while running ON the vtask pool
(VTASK_LIVE_BLOCK); putting checkpoint's unbounded, lock-held
write_snapshot() on that same pool risked starving it, exactly what
ha-async-io.md's own thread-role table warns against. The SlowLane is
the actual established pattern for this I/O class (push_config_transport_only,
the snapshot-send path, catchup_lane_fn, demotion_lane_fn all dispatch
there) -- and persist_checkpoint() already runs on the slow lane in
production TODAY via ha_snapshot_do_send() -> snapshot_lane_fn, so
apply_pump_lock's acquisition from a slow-lane thread is proven-safe,
exercised daily, not new surface. Unlike catchup/demotion (which stay
flag-gated OFF for an unrelated, still-not-root-caused hang, bug-1429),
checkpoint's new flag defaults ON given this reuses an already-proven
code path. Inline-fallback + WARN only on a genuine slowlane_submit
failure (not -EAGAIN/-EEXIST, both non-failures) -- never silently drops
a scheduled checkpoint.
New test_persist_checkpoint_offload.c (3 tests). RED-phase forced the
pre-fix always-inline path: the suite never printed a single result and
had to be killed, proving the test genuinely detects the violation, not
a tautology. GREEN: all 3 pass. Full suite: 1184 tests, 2 failures --
both the pre-existing, unrelated bug-1561 overlayfs sandbox flake
(exact signature match). No live HA soak was run for this specific fix
in isolation (explicitly scoped out, not hidden) -- the mandatory
live-soak proof for this whole audit campaign covers it next.
This closes the last of the 9 confirmed findings from the controller
audit (bug-1684, bug-1687/1689/1690/1691/1692, bug-1693, bug-1694).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Violates this project's own recorded Decision Log entry: "No client
reply-wait timeout as a fix for a hung request -- fail loud at the real
transport/source instead of masking with a deadline backstop."
OPCode_schedAdmin (cluster.c) and on_fetchfile (job.c, the worker proxy
-- cluster.c's own comment says schedAdmin "mirrors on_fetchfile's proxy
pattern", confirming one shared design mistake, not two independent
bugs) both blocked on a multi-second pthread_cond_timedwait, replying
"timeout" on expiry. handle_scheduler_leave()/handle_worker_leave() (the
actual disconnect handlers) only nulled the endpoint pointer -- neither
ever signaled the pending waiter, so a mid-wait disconnect was invisible
until the deadline lapsed. The timeout was the ONLY failure-detection
mechanism, exactly what the decision forbids.
Both sites already had a working signal function on their success path
(schedadmin_signal(), fetchfile_signal()) -- just never wired to the
disconnect handlers. Fixed by calling them from
handle_scheduler_leave()/handle_worker_leave() with a distinguishable
-ECONNRESET outcome, so the waiter wakes immediately on disconnect
instead of only on the deadline. Timeouts kept as defensive backstops
for a genuinely wedged peer that never disconnects cleanly either --
this fix is about not relying SOLELY on the timeout, not removing it.
schedadmin_signal_disconnect() must be called BEFORE handle_scheduler_leave()
acquires c->sched.lock -- the dispatch wait holds that same lock for its
entire duration, so signaling after acquiring it would deadlock.
Found by an /xce-audit-invariants controller pass (Decision Log fidelity
dimension). New test_fetchfile_disconnect.c plus extensions to
test_sched_admin_dispatch.c/test_sched_worker_change.c; RED/GREEN
independently verified against real elapsed-time assertions (pre-fix:
5001ms full-timeout wait; post-fix: <1s immediate wake). Full controller
suite: 1181 tests, 2 failures -- both the pre-existing, unrelated
bug-1561 overlayfs sandbox flake, confirmed via a clean meson-driven run.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Last of the five field-parity gaps from the original audit (bug-1678,
bug-1687, bug-1689, bug-1690, bug-1691). set_preason()/clear_preason()
set job.pend_reasons (host/resource-based pending-job explanations), but
persist_job_hook_fun's default: arm only ever captured {jobid, state}.
Neither apply seam restored pend_reasons after a restart.
Reuses the ALREADY-EXISTING JobPendReason/HostBasedReason/ResourceBasedReason
capnp structs -- no schema change needed, unlike bug-1690's WAL_JOB_RUSAGE.
Safety-checked against bug-1690's lesson (RTTI/makeString()-backed fields
need merge-not-replace, not free-and-replace): pend_reasons is plain
malloc/strdup-backed throughout (erase_job_preason is FREEUP()-only,
add_host_preason/add_res_preason use strdup()/calloc()), so a
straightforward free-and-replace is safe here, matching bug-1678/1687's
pattern instead.
New wrinkle beyond the four prior fixes: pend_reasons has a genuine CLEAR
operation, and "trailing bytes present or absent" can't distinguish a
legitimate clear from an old-format payload -- added an explicit presence
byte (0=cleared/adopt NULL, 1=capnp bytes follow), verified by two
dedicated "clear survives restart" tests.
11 new tests in test_wal_job_preason_fact.c; RED/GREEN independently
verified (7 genuine pre-fix failures, one test's own wrong premise about
capnp's garbage-byte behavior caught and corrected during RED/GREEN
iteration). Full controller suite: 1178 tests, 2 failures -- both the
pre-existing, unrelated bug-1561 overlayfs sandbox flake. No ASAN findings.
This closes the field-complete apply parity invariant class the
xce-audit-invariants controller dimension 7 was added for.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Same class as bug-1678/1687/1689/1690: on_job_ckpt() sets job.ckptinfo
(a plain char*, xstring-backed via makeString()) but is purely hook-driven
-- persist_job_hook_fun's default: arm only ever captured {jobid, state}
for WAL_JOB_CKPT. Neither apply seam restored ckptinfo after a restart.
Simplest of the five field-parity fixes: ckptinfo is a bare string, so no
capnp struct is needed -- mirrors WAL_QUEUE_CTRL's existing NUL-terminated-
string-after-header wire shape instead. Adoption is unconditional (not
gated on a state transition), matching bug-1690's precedent: a checkpoint
report never changes job.state. restore_job_ckpt_state() releases the
prior value via dereference() (the correct RTTI verb for an xstring), not
free().
Found by an /xce-audit-invariants controller pass (field-complete apply
parity dimension) -- the last of five sibling gaps found together
(bug-1678, bug-1687, bug-1689, bug-1690, this one). 7 new tests in
test_wal_job_ckpt_fact.c; RED/GREEN independently verified (5 genuine
pre-fix failures, 2 backward-compat tests correctly passed unchanged).
Full controller suite: 1167 tests, 2 failures -- both the pre-existing,
unrelated bug-1561 overlayfs sandbox flake. No ASAN findings.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Same class as bug-1678/1687/1689: the WAL fact for WAL_JOB_RUSAGE only
carried {jobid, state}, so job.run_usage (client-reported per-host
CPU/mem usage) was never restored on either apply seam after a restart.
Unlike the prior three, xce_job_runusage_t/xce_host_runusage_t had no
existing Cap'n Proto mapping -- a genuine new schema addition
(HostRunusage/JobRunusage in controller.capnp, mirroring JobPendReason's
List(NestedStruct)+$C.maplistcount style), verified against the real
struct definitions in types.h, codegen confirmed via nm on the built
binary rather than assumed.
Two deliberate deviations from the established precedent, found by
reading the real code rather than copying the pattern blindly:
1. Adoption is unconditional, not gated on a state transition -- a
rusage report almost never changes job.state (repeat reports keep
state at RUN), so gating on JOB_TRANSITION_APPLIED like START/FINISH/
DISPATCH do would make the fix dead code for the realistic case.
2. Merge, not free-and-replace. update_host_runusage() builds run_usage
with host_name sourced from makeString() -- an RTTI-managed pointer,
not malloc() -- and the primary's own live apply pump re-applies every
fact it just wrote. A naive free_JobRunusage_ptr()-then-adopt (matching
the prior three fixes exactly) would free() a non-malloc'd pointer on
the very first live rusage report for a new host -- guaranteed heap
corruption. Fixed with merge_job_runusage(): existing hosts get
scalar-only in-place updates (host_name pointer never touched), only
genuinely new hosts adopt their freshly-decoded (safe, malloc'd)
host_name. Proved via a real makeString()-backed test asserting
pointer identity survives, not just value equality. job_destroy()
(src/lib/jobj.c) was found to have this identical latent mismatch,
pre-existing and independent of this fix -- flagged, not fixed here.
Found by an /xce-audit-invariants controller pass (field-complete apply
parity dimension). 8 new tests in test_wal_job_rusage_fact.c; RED/GREEN
independently verified. Full controller suite: 1160 tests, 2 failures --
both the pre-existing, unrelated bug-1561 overlayfs sandbox flake. No
ASAN/heap-corruption findings. Backward compatibility confirmed for
old-format bare-payload entries.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Same class as bug-1678/bug-1687: record_job_finish_fact() wrote only
{jobid, state} for WAL_JOB_FINISH. Both apply seams either fabricated
finish_time via time(NULL) or touched nothing beyond state.
Worse than "lost on restart": apply_job_finish_transition() -- the only
function that ever set job.exit_info -- is dead code on the real
production apply path. The raft-apply-decoupling redesign routes actual
mutation through the leaner apply_job_update_transition() instead, so
exit_info was never populated at all, restart or not, before this fix.
job_exitinfo_t ({int exitcode;}) has no existing capnp mapping, so this
uses a small native (non-capnp) trailing payload -- wal_jobstate_t header
+ uint64_t finish_time + int32_t exit_code -- mirroring wal_jobstate_t's
own native-struct style rather than inventing a capnp wrapper for one
field. run_time is never carried on the wire, always re-derived as
finish_time - start_time at apply time. Both apply seams updated;
old-format payloads (exactly sizeof(wal_jobstate_t)) decode unchanged
(fabricated finish_time, NULL exit_info) -- full backward compatibility.
Found by an /xce-audit-invariants controller pass (field-complete apply
parity dimension). 6 new tests in test_wal_job_finish_fact.c; RED/GREEN
independently verified (4 genuine pre-fix failures with real
expected-vs-fabricated mismatches, 2 backward-compat tests correctly
passed both before and after). Full controller suite: 1152 tests, 2
failures -- both the pre-existing, unrelated bug-1561 overlayfs sandbox
flake (confirmed flaky across repeated re-runs of the same binary).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
Same class as bug-1678: apply_job_start_transition() sets the real
job->job.pidinfo (worker-reported PIDs/PGIDs/exdata) and start_time, but
the WAL fact for WAL_JOB_START only carried the shared wal_jobstate_t
{jobid, state} header -- both apply seams (apply_committed_entry() for
primary boot replay, apply_wal_entry() for standby catch-up) restored
state only. pidinfo stayed permanently NULL after a restart; start_time
was fabricated with time(NULL) at replay instead of being carried,
drifting from the true value.
Mirrors bug-1678's exact fix shape: capnp_encode_pidinfo()/decode_pidinfo()
(persist.c, using the pre-existing JobPIDInfo capnp struct -- no schema
change needed) serialize job_pidinfo_t into the WAL payload, trailing a
new uint64_t start_time field appended after wal_jobstate_t.
persist_submit_job_start_fact() replaces the old state-only
persist_submit_job_fact() call for this type. Both apply seams updated;
a payload of exactly sizeof(wal_jobstate_t) (any entry written before
this fix) decodes with pidinfo left NULL and start_time fabricated via
time(NULL), exactly matching pre-fix behavior -- full backward
compatibility, verified by every pre-existing WAL_JOB_START test using
the old bare-struct shape.
Also fixed a genuine, unrelated deadlock this change's larger payload
exposed in test_commit_barrier_reply.c: cbrp_count_jobstate_entries_ex()
matched WAL entries by exact payload size (==), so once WAL_JOB_START
legitimately grew it silently undercounted to 0 and a downstream test
spun forever polling an uncounted target. Fixed the match to >= (jobid/
state always live at a fixed leading offset) -- bisected via /proc/*/wchan
futex inspection (no ptrace in this sandbox), confirmed as the actual
cause, not the teardown-ordering red herring first suspected.
Found by an /xce-audit-invariants controller pass (field-complete apply
parity dimension, added after bug-1678). 6 new tests in
test_wal_job_start_fact.c; RED/GREEN independently verified (4 tests
genuinely failed pre-fix with exact expected-vs-fabricated value
mismatches, 2 backward-compat tests correctly passed both before and
after). Full controller suite: 1146 tests, 2 failures -- both the
pre-existing, unrelated bug-1561 overlayfs sandbox flake.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
ha_persist_term() had a void return -- every open/write/fsync/rename
failure was silently swallowed ("best-effort; caller must not rely on
this for correctness"). Its sole vote-granting caller, raft_cb_persist(),
always returned 0 regardless, so the raft library always granted the
vote even when the term/voted_for record never reached disk -- a real
split-vote risk if the node then restarted and forgot it voted
(Election Safety requires durably-persisted votedFor).
ha_persist_term() now returns 0 on success (including the deliberate
NULL-workdir test-mode skip) or -errno on a genuine I/O failure
(fsync's previously-unchecked return is now checked too). raft_cb_persist()
propagates that failure to the raft library (denying the vote per its
own documented contract) and rolls back ha->voted_for_term/
ha->voted_for_name to their pre-call snapshot, so the adapter's
in-memory state stays consistent with the library's own "deny without
updating voted_for" behavior. ha->term is deliberately NOT rolled back:
the raft library adopts a higher observed term unconditionally at Step 2
of raft_on_vote_request, before and independent of the vote-grant/persist
outcome, so the adapter correctly mirrors that same semantics.
Also corrects a stale claim in controller/CLAUDE.md's Protocol
Verification Checklist (found by a sibling audit dimension the same
session): term+voted_for were never written "atomically under ha->lock"
-- term uses its own atomic store outside the lock.
Found by an /xce-audit-invariants controller pass (error-handling
dimension). 3 new tests in test_ha_durability.c drive the real
production path (ha_init_raft + raft_on_vote_request, the exact
function cluster.c's dispatch calls) against a forced ENOENT write
failure; RED/GREEN independently verified.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
New test_dc_success_notifies_job_update_hook_exactly_once
(test_dispatch_commit.c): a spy hook counting HOOK_AFTER_EVENT/
HOOK_DETAIL_JOB_UPDATE fires specifically -- the exact hook
apiserver_job_update_fun subscribes to in production. Asserts exactly
one fire per successful live dispatch.
Independently reproduced RED->GREEN: disabled the new notify call
with the test kept, rebuilt, confirmed FAIL (Expected 1 Was 0);
restored, rebuilt, confirmed PASS.
Also independently confirmed no double-WAL-append regression via two
existing tests whose fixtures genuinely wire persist.cluster with
persist_job_hook_fun registered: test_jobdispatch_receipt_ack_no_wal_append
(asserts exactly one WAL entry for a successful dispatch) and
test_handle_jobdispatch_matches_dispatch_a_job_notify_count -- both
pass unchanged. Full controller_unit_test: 1137/1137 (2 pre-existing
test_config_repl.c failures, confirmed identical on stock code via
git-stash -- the known overlayfs fdatasync errno=5 flake, bug-1561).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
dispatch_a_job_() mutates job.state/job.allocs directly then persists,
firing no hooks at all. apply_committed_entry()'s later apply of that
same WAL_JOB_DISPATCH entry sees state already matches the value it
just set moments earlier (under the same lock) and reports
JOB_TRANSITION_NOOP_DUPLICATE, silently skipping its own would-be
notify. Result: the apiserver is never told a job reached RUN -- xcecli
jobs list shows it stuck at PEND forever, no restart involved.
New persist_notify_job_updated_already_persisted(cluster_t*, job_t*)
(persist.c/persist.h), called from dispatch_a_job_()'s success branch
right after unlock(j). Mirrors apply_committed_entry()'s own identical
Hazard-1-guarded hook fire exactly -- same t_apply_seam_notifying
guard, same hook (HOOK_DETAIL_JOB_UPDATE's AFTER_EVENT), same detail --
without which persist_job_hook_fun (also subscribed to this hook)
would redundantly re-append the transition as a second, duplicate
WAL_JOB_UPDATE entry. The flag stays file-private to persist.c; this
is job.c's only sanctioned way to reach it.
record_job_start_fact()/record_job_finish_fact() needed no change --
their durable-append-only pattern (defer mutation+notify to
apply_committed_entry entirely) is already correct; only
dispatch_a_job_()'s broken hybrid (direct mutation, no notify at all)
was ever the problem.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
test_dispatch_commit.c's test_dc_success_appends_wal_job_dispatch_durably
(already drove the real dispatch_a_job_() production path through a
real WAL-write + persist_recover() restart cycle, previously only
checking recovered state) now also asserts the recovered job.allocs
matches what was dispatched -- host, resource name, value. This is
the primary regression test for the user's exact acceptance
criterion: "after recover the job should get the same as it's state
before shutdown."
Independently reproduced RED->GREEN (not just narrated): reverted the
production fix with the test extension kept, rebuilt, confirmed FAIL
at exactly this assertion; restored, rebuilt, confirmed PASS.
test_wal_job_dispatch_fact.c: added 3 new tests exercising the richer
payload directly on both apply_committed_entry and apply_wal_entry,
plus a full restart-replay variant. All 3 pre-existing tests (bare
wal_jobstate_t payloads, no trailing bytes) pass completely unchanged,
proving backward compatibility with any WAL_JOB_DISPATCH entry written
before this fix.
Full controller_unit_test suite independently re-run: 1136/1136 pass,
zero regressions.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
dispatch_a_job_() set j->job.allocs in memory at dispatch-commit time
but only ever persisted {jobid, state} via persist_submit_job_fact() --
the allocation itself was never written to the WAL. On a full restart,
job.state correctly recovered to RUN (both WAL_JOB_DISPATCH and the
later WAL_JOB_START fact carry state=RUN) but job.allocs stayed
permanently NULL, since apply_job_update_transition() only ever
touches state. EXEC_HOST displayed blank forever for a job that was
genuinely running before shutdown.
No capnp schema change needed -- JobAlloc (xce_job_alloc_t) was
already a full top-level struct (the existing Job.allocs field) with
generated encode/decode functions already in the build. New:
capnp_encode_joballoc()/decode_joballoc() (persist.c, mirroring the
existing capnp_encode_worker()/restore_job() patterns) and
persist_submit_job_dispatch_fact() -- WAL_JOB_DISPATCH's payload is
now a wal_jobstate_t header plus trailing capnp-encoded JobAlloc
bytes. dispatch_a_job_() now persists j->job.allocs (the canonical,
freshly-constructed struct, read under the same write lock) instead
of just the state.
Both apply seams updated: apply_committed_entry() (primary's own boot
replay) gets one added branch in its existing WAL_JOB_START/FINISH
stamp-chain; apply_wal_entry() (standby catch-up / replicated-apply --
also required, a promoted standby would hit the identical bug) has
WAL_JOB_DISPATCH split into its own case, calling new
restore_job_dispatch_state(). A payload of exactly sizeof(wal_jobstate_t)
(any entry written before this fix) decodes no allocation and behaves
exactly as before -- full backward compatibility.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
