Closes the msg-gap G-8 test gap the research flagged ("handler→dequeue ROUTING
is verified by code review, NOT an automated integration test"). The
remediation (UPDATE_JOB_FIELDS op routed to sched_update_existing_job) is
already in the code (93124c5); these tests lock the routing:
- test_update_fields_routes_to_inplace_path: driving the real sched_update()
dequeue loop, an UPDATE_JOB_FIELDS request reaches sched_update_existing_job
and mutates the existing job in place (same object, runtime state kept).
- test_bulk_update_existing_job_no_damage: a re-pushed dataset row for an
existing jobid is EEXIST-dropped by key_add (via the real sched_add_job) —
the live job keeps identity/state/runtime state.
10/10 sched_redispatch_test PASS; full scheduler suite 6/6 OK. Live cluster:
job PEND→RUN→DONE via notify updates, no WARN, no duplicate instance.
Co-Authored-By: Claude <noreply@anthropic.com>
The controller acknowledges EVERY dispatched job through
NotifyData_data_jobDispatchFail (controller/job.c handle_jobdispatch):
a negative code is a real rejection, code 0 is the success ack (the else
branch calls notify_dispatch(jobid, 0)). sched_handle_notify treated any
jobDispatchFail as a failure and returned the job to PEND, so every
successfully dispatched job was immediately re-PENDed, re-elected, and
re-dispatched while the original instance kept running — spawning
duplicate instances cluster-wide. On a 3-worker cluster 24 submitted jobs
ran 38 instances and piled onto the first worker (t1) because its slots
were counted for each duplicate.
Fix: only sched_requeue_pend_job() when fail->code < 0; consume the
success ack (code 0) as a no-op — the job is already RUN
(sched_commit_decision marks it at commit). This was a regression of the
bug-1494 re-PEND logic (bug-1510 fix) that lacked the code guard.
Verified live on t1/t2/t3: 24x sleep-300 jobs distributed exactly 8/8/8,
zero duplicates, all DONE. Add white-box test
test_handle_dispatch_fail_requeues_only_on_negative_code.
Co-Authored-By: Claude <noreply@anthropic.com>
When a worker is re-pushed (e.g. it rejoined with fresh resvals after the
controller/worker had been down), sched_update_workers built a new worker and
sched_add_worker's key_add returned EEXIST — the fresh worker was dropped and
the scheduler kept the stale state (slots=0 from the dead/absent period).
PEND jobs never dispatched even after the worker returned with full capacity
(bug-1494, controller-restart self-heal).
sched_update_workers now updates an already-known worker in place: swap the
fresh resvals/resInUse/status into the existing worker (the stale arrays are
freed when the discarded object is torn down) and adopt its host_eval_ctx.
Also NULL-guard make_sched_worker (it can return NULL).
Verified on the t1 cluster: scheduler started while the worker was dead
(slots=0), worker rejoined (slots=8), previously-PEND jobs dispatched and
completed, no ASAN.
Co-Authored-By: Claude <noreply@anthropic.com>
The controller rejects a dispatch when the chosen worker is not in a
dispatchable state (UNAVAIL after a worker death, or still joining) and
replies with a jobDispatchFail notify. The scheduler had no handler for that
event, so a job it had just committed (state=RUN) was never returned to PEND
— it was never re-elected and stayed PEND cluster-wide even after the worker
recovered.
Handle NotifyData_data_jobDispatchFail: log the failure and call the new
sched_requeue_pend_job(), which frees the dispatch bookkeeping, resets the
job to PEND, and re-enqueues it in the fcfs pendJobs queue so the next
election session re-dispatches it. White-box test TUs that #include
handler.c get a sched_requeue_pend_job stub.
Co-Authored-By: Claude <noreply@anthropic.com>
bug-1498 (re-dispatch): sched_job_is_ready() was a stub returning 1, so any
job lingering in the fcfs pendJobs queue was always elected. A dispatched job
was never marked non-PEND (the controller defers its schedStartJob notify to
the not-yet-implemented apply pump, controller/job.c), so fcfs_update_job
re-enqueued it on every job update and the election thread re-dispatched the
running job — the worker got conflicting signals and the job was SIGTERM'd.
- sched_job_is_ready: elect only PEND, non-suspended jobs
- sched_commit_decision (dispatch): mark the job JOB_STAT_RUN on dispatch so
it leaves the PEND set and is no longer re-electable
- fcfs_update_job: dedup via ordq_peek before enqueue
bug-1497 (double-free): sched_update's resval paths transferred ownership of
rv->val to sharedInst/w->resvals but left the request pointer set, so
sched_pend_update_killer double-freed it -> use-after-free in
build_host_eval_ctx. Clear rv->val = NULL on transfer.
Tests: new scheduler/testsuite/test_redispatch.c (8 cases): sched_job_is_ready
PEND gating + dispatch-commit marks the job RUN. Full scheduler meson suite
green. Live-verified: a `sleep 60` job is elected exactly once, runs the full
duration and exits 0 (`completed successfully`); before the fix it was
re-dispatched at ~11s and killed (SIGTERM 143).
Co-Authored-By: Claude <noreply@anthropic.com>
put_resvals (resource.c) used `idxs[i] <= 0` to skip "invalid" resource
indexes, but index 0 IS valid — it's the first resource in the table
(e.g. "slots"). For idx 0 the val was never allocated (xvals[i] stayed
NULL) and the write loop then SEGV'd. bug-1495.
Fix: skip only negative indexes (`idxs[i] < 0`), and NULL-guard the
write loop defensively.
Verified end-to-end: a locally-built scheduler now connects to the t1
controller and dispatches all 5 test jobs to node t1 (exec_hosts
populated, jobs run and exit). This was the final blocker for job
scheduling (bug-1494: all jobs PEND).
Also: add the t1 host definition to hosts.yaml (worker join was rejected
with -EINVAL because no matching hostconfig existed for hostname "t1").
Co-Authored-By: Claude <noreply@anthropic.com>
controller.capnp declared `struct Data $C.mapname("xce_data_t")` (an
OPCode_get query-result tagged union) at file scope, which shadows
Cap'n Proto's builtin Data blob type for every later `: Data` field in
the same file. This silently miscompiled 7 fields across 6 structs as
a struct-pointer type instead of a raw byte blob: UpgradeConfigRequest
.sha256/.bytes, ReplConfigReply.sha256/.bytes, ReplConfigPushRequest
.sha256/.bytes, ReplEnvelope.payload, SnapshotTransfer's chunk
payload, WalCatchupEntry.payload/.batchPayloads, and CtrlHelloRequest
.key. Production C code had already routed around it for years via
hand-rolled raw capn_ptr manipulation (see request.c's own comments);
nothing before now needed the generated codec/accessors to actually
work for these fields.
Renamed the local struct to XceData (capnp-level name only; the
$C.mapname("xce_data_t") C typedef is unchanged) and explicitly
repointed getdata/pushmsg at XceData by name, since those two fields
correctly want the local tagged union. Updated every derived
generated-identifier reference across 19 hand-written files spanning
controller, lib, scheduler, and the Go apiserver/xcecli CGo bindings.
Fixed 5 call sites in lib/request.c that had been typed against the
old workaround alias but now legitimately receive the correct
capn_data type (a pure type-name swap -- Data_ptr/XceData_ptr and
capn_data share the identical {capn_ptr p} layout).
Verified via full rebuild+test: lib (1/1), controller (3/3, incl. the
job-array push regression tests), scheduler (15/15), Go apiserver +
xcecli (all packages green), and a direct pycapnp round-trip proving
UpgradeConfigRequest.sha256/.bytes now encode/decode correctly.
Bug: bug-1461 (.wolf/buglog.json). Unblocks admin-config-tool task 3.3.
bug-1453: remove unused client-side CA-certificate plumbing. C-side:
drop ca_cert_path from xce_cluster_config_t, do_connect()'s ca_cert
param, --ca-cert flags on worker/scheduler, and controller's cert
population (TLS is server-side only, SSL_VERIFY_NONE everywhere on
the client side already, verified zero SSL_CTX_load_verify_locations
calls in the C tree). Go-side: delete xcecli's self-signed cert
TOFU-pinning flow (cmd/auth/cert/*), CACert field, SetRootCAs/
loadRootCAs, and every call site.
bug-1454: fix xcecli_ctx_t leak in scheduler.c sched_init() — the
unconditional initial ctx_init was never freed before the HA
candidate loop re-initialized it on the first iteration.
bug-1455: sched_init()'s memset(sched, 0, ...) was silently wiping
sched->cluster_cfg that main.c had populated from --peers just before
calling sched_init(), so --peers was always ignored and the scheduler
fell back to the hardcoded localhost:9990 single-host default.
Restructured sched_init() to take cluster_cfg as an explicit parameter
(assigned after the memset) instead of relying on caller-side
ordering with a separate sched_set_cluster_cfg() setter, which is now
removed.
bug-1456: revert an unsafe stack-local xce_client_config_t introduced
while fixing bug-1455 — xcecli_ctx_init() stores that pointer as
ctx->conf and client_failover.c dereferences it for the ctx's entire
life (runtime reconnect), so a per-loop-iteration stack copy would
dangle the moment sched_init() returned. Back to mutating the
caller-owned, process-lifetime config in place.
Verified: full ASAN build (all components), meson test suite 9/9 OK,
manual scheduler testsuite 15/15, Go build/vet/test all pass.
sched_handle_dataset()'s Data_data_jobArray case never freed
dataset->rows (the decoded xce_job_submit_t*) after sched_on_job_array()
consumed it -- free_Dataset() is an empty no-op for every table type, so
nothing else owned this call. One bounded leak per delivered array push.
Free it via the existing free_JobSubmit_ptr() teardown on both the
success and error path, mirroring the sibling list-shaped cases'
ownership-transfer convention.
Rewrote test_job_array_expand.c's init_job_array() fixture to
heap-allocate + strdup its fields (matching the real wire-decode shape)
since freeing the old stack/string-literal fixture crashed. Redelivery
test now builds a fresh descriptor per delivery, matching how a real
redelivery is always a freshly-decoded wire message.
Closes task 7.3 (job-array-single-wal-record), the last of the three
post-GO residual gaps reopened 2026-07-16.
Cross-daemon integration test, test-only, spanning the scheduler (C)
and API-server (Go/CGO) receive paths built in tasks 5.1-5.4/5.3b.
Drives real xce_encode()/xce_decode() wire bytes through the actual
production dispatch chain on both daemons -- extract_data() ->
scheduler_req_handler() -> sched_handle_dataset() ->
sched_on_job_array() and go_push_handler() -> handleDataset() ->
cJobArrayToModels()/dispatchJobArray() -- skipping only vtask_entry()'s
~10 lines of static glue, mirrored verbatim in test code (the
if (callnext) treats-any-nonzero-as-proceed behavior that caused
5.3b's bug is reproduced exactly).
Redelivery is tested via genuinely independent fresh decodes of
separately-encoded bytes, not a decode-once-call-twice shortcut.
Bug-catching capability proven by temporarily reverting task 5.3b's
fix and confirming both the C and Go tests fail meaningfully, then
restoring to green -- confirmed independently by both the implementer
and reviewer.
Task 5.5 of job-array-single-wal-record (Requirements 6.1-6.5, 8.4).
This completes Phase 5 (Compact Scheduler/API-Server Push).
extract_data() (src/lib/request.c), the shared reassembly step every
inbound push passes through inside vtask_entry() before any receiver's
handler runs, had no case for Data_data_jobArray -- it hit
default: rc=-EINVAL. Because vtask_entry()'s callnext check treats any
nonzero return (including a negative errno) as "reassembly complete,
proceed," this never surfaced as an error: the real payload was freed
and the receiver silently handed an empty, zeroed dataset_t, with no
log anywhere in the chain. Tasks 5.1-5.3 were each individually
correct and tested in isolation, but the feature was dead on the real
wire end-to-end.
Adds case Data_data_jobArray: passing the single, unchunked
descriptor straight through (no pagination needed, unlike the
existing LIST-shaped cases) and nulls the source pointer after
transfer -- verified this is not cosmetic, since free_Request()'s
pushmsg case would otherwise double-free it via free_Data()'s live
Data_data_jobArray branch.
Proven with a real xce_encode()/xce_decode() round-trip through the
fixed extract_data() -> sched_handle_dataset() -> sched_on_job_array()
(task 5.3, unmodified) chain, not just a unit-level check on
extract_data() alone. Also confirms task 5.4's future API-server
receive path shares this exact fix, since both go through the same
xcecli_ctx_init() -> vtask_entry() -> extract_data() pipeline.
One self-flagged, non-blocking residual gap (pre-existing since task
5.3, out of this task's request.c-only boundary): dataset->rows for
the jobArray case is never deep-freed downstream, leaking one bounded
xce_job_submit_t per successfully-delivered array push commit.
Task 5.3b of job-array-single-wal-record (Requirements 6.1, 6.2, 6.4, 8.4).
Inserted mid-run after independent review confirmed the gap; user
confirmed sequencing (fix now, before task 5.4).
Adds sched_on_job_array() (handler.c), a sibling case to
Data_data_schedjobs decoding the new Data.jobArray variant and
expanding it locally into N ordinary schedulable tasks via
base_jobid + i, routed through the existing unmodified per-job
pipeline (sched_on_jobs()). Each element gets its own deep clone
(clone_job_submit_data/clone_job_startinfo) of the shared submit
spec, since per-element teardown (free_Job_ptr) frees it
independently and a shared pointer would double-free.
Dedup reuses sched_get_job() against the same kvtab sched_add_job()
guards, rather than a new mechanism. A confirmation log line fires
once per successful expansion (skipped on a fully-duplicate
redelivery).
Adds a new task (5.3b) discovered during this task's review: the
shared wire dispatcher extract_data() (src/lib/request.c) has no case
for Data_data_jobArray and silently drops every real array push
before any receiver's handler runs -- outside every current task's
boundary, confirmed to affect the future API-server receive path
(task 5.4) identically. design.md's File Structure Plan corrected to
record this. User confirmed sequencing: fix it now, before 5.4.
Task 5.3 of job-array-single-wal-record (Requirements 6.3, 6.4, 6.5, 8.4).
The Task 9 tests were marked [x] but were stub/absent. Convert the
correctness-critical ones to real white-box tests, registered as meson tests
under --suite scheduler (leaks-off per convention):
- test_handlers.c (new) — #include ../src/handler.c; covers 9.2/9.3: each
notify/dataset handler enqueues the correct op / skips NULL-empty / -EINVAL.
Asserts the 5.6 fix (update_job -> UPDATE_JOB_FIELDS, not bulk UPDATE_JOB).
- test_job_update.c — add 9.5 (real sched_update_existing_queue, replacing the
former local stub); already covers 9.4 (job) and 9.7 (rusage).
- meson.build — build + register all three scheduler test executables.
Suite GREEN: sched_test 15/15, sched_job_update_test 10/10,
sched_handlers_test 6/6; ASAN-clean.
Honest remaining: 9.6 dequeue worker-resvals APPLY is PARTIAL (enqueue tested;
apply is a dormant path, R-3); 9.8 is a documented compile-time absence check.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5.5 (Req 6 / G-5): the JOB_RUSAGE notify path called the plugin but never
stored the usage data — xce_sched_job_t had no rusage field (UB-6.1 unmet).
- Add xce_job_rusages_t* rusages to xce_sched_job_t.
- New sched_apply_job_rusage() stores the report ON the job (ownership
transferred), replacing any prior via free_JobRusages_ptr; -ENOENT unknown,
-EINVAL NULL. Dequeue NULLs the request pointer on success; kill_sched_job
releases job->rusages.
5.8 (G-3): sched_pend_update_killer left five new-op payloads on empty
break cases — a per-message leak since ownership is transferred on enqueue.
- Free each with the exact generated deep-freer: free_QueueConf_ptr,
free_Resource_ptr[], free_ResourceVal_ptr[], free_ResValList_ptr,
free_JobRusages_ptr. Safe: handlers nullify on enqueue, makeResource copies
via makeInstance, dequeue copies values (no aliasing).
Tests: sched_job_update_test 7/7 (3 update + 4 rusage), regression
sched_test 15/15, ASAN clean (no bad-free/double-free).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The notify schedUpdateJob path shared XCE_SCHED_OP_UPDATE_JOB with the
dataset bulk-load, so the dequeue routed both to sched_update_jobs()
(create/replace) — a notify update to an existing job corrupted/duplicated
live job state and never returned -ENOENT (Req 3 UB-3.1/UB-3.2 unmet;
sched_update_existing_job was dead code copying only state).
- Add distinct op XCE_SCHED_OP_UPDATE_JOB_FIELDS (+ jobfieldsupdate union),
enqueued only by the notify handler, dequeued to sched_update_existing_job.
- Move sched_update_existing_job to schedobj.c; update all mutable fields
(state, priority, queue, resreq) in place; -ENOENT unknown, -EINVAL NULL.
- Dataset bulk-create path unchanged. Killer frees the new payload.
- New white-box test sched_job_update_test (3/3); regression sched_test 15/15.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire up 6 previously-stubbed message handlers in the scheduler:
resource dataset, resval dataset, job update notify, queue update
notify, resval update notify, and job rusage notify. Remove the
unreachable pushmsg branch (vtask_entry rewrites all pushmsg to
datamsg before the handler fires).
Changes by component:
- helper.h/helper.c: add xce_notify_event_name() for symbolic log names
- scheduler.h: add 5 new xce_sched_op_t values and matching union fields
- plugin.h/plugin.c: add plugin_job_rusage_hook_t and plugin_job_rusage_update()
- schedobj.c: add sched_update_existing_queue() with NULL/-ENOENT guards
- scheduler.c: wire 5 new dequeue cases; add sched_update_existing_job()
and static sched_op_name(); emit WARN log on handler failure
- handler.c: implement sched_handle_resources/resvals (dataset) and
sched_handle_sched_update_{job,queue,resvals}/job_rusage (notify);
remove dead sched_handle_jobpush/workerpush and pushmsg branch;
update TRACE logs to use symbolic name helpers
- test.c: 5 new unit tests covering notify event names, enum values,
and queue update NULL guard (15/15 pass)
Three identical static copies (controller, worker, scheduler) replaced
with a single xce_parse_peers() in src/lib/cluster.c.
The libxce version is the canonical implementation: stores the logical
name from 'name=host:port' syntax so daemons on the same dev host can
identify themselves by name rather than IP address.
Worker (xcecd):
- Add xce_cluster_config_t cluster_cfg field to controller_connector_ctx_t
(controller_connector.h includes xce/cluster.h)
- Populate cc_ctx->cluster_cfg from parsed cluster_cfg after xcecd_start()
sets cc_ctx (main.c, removes the P3 placeholder)
- Replace xcecd_join_cluster() single-shot connect with a candidate-iterating
loop: tries each candidate, calls xcecli_stop() before the next attempt,
falls back to the original single-host path when candidates_count == 0
Scheduler (xce_scheduler):
- Add xce_cluster_config_t cluster_cfg field to xce_scheduler_t
(scheduler.h includes xce/cluster.h)
- Add sched_set_cluster_cfg() setter
- Replace the hardcoded xcecli_start/xcecli_join pair in sched_init() with
the same candidate-iterating pattern; original single-host path preserved
- main.c calls sched_set_cluster_cfg() before sched_init() when --peers given
(removes the P3 placeholder comment)
checking_connection() already calls xcecd_join_cluster() on retry, giving
the worker free reconnect-after-failover behaviour for no additional code.
Worker (P2.5-5):
- Added parse_peers() static helper to src/worker/src/main.c
- Switched getopt to getopt_long to support --peers and --ca-cert long options
- When --peers is given: parses into xce_cluster_config_t, calls
xce_cluster_config_validate(), logs INFO with candidate count/identity/ca_cert
- If validation fails: prints clear error and exits(1)
- WARN logged when candidates_count > 3
- Backward compatible: no --peers falls back to existing single-controller path
Scheduler (P2.5-6):
- Added parse_peers() static helper to src/scheduler/src/main.c
- Extended argp struct with --peers (-P) and --ca-cert (-C) options
- Same validation/logging pattern as worker
- Backward compatible: no --peers falls back to existing single-controller path
Tests (new):
- src/lib/testsuite/test_cluster_parse.c: 8 unit tests for parse_peers logic
(single entry, multiple entries, no-name format, empty/null/missing-port errors,
name-stripped correctly, MAX_HA_CANDIDATES cap respected)
- Registered in test.c and meson.build
- All 8 tests GREEN; 3 pre-existing failures in unrelated GC/CSE tests unchanged
Two correctness fixes found during final Phase 6 code review:
F1 (CRITICAL): GC permanently disabled — staging_merge_complete bootstrapping deadlock.
sched_begin_session reset staging_merge_complete to 0 but never set it back to 1,
so sched_end_session's guard (!staging_merge_complete) always fired, preventing GC
from ever running. select-node memory leaked indefinitely across scheduling sessions.
Fix: set staging_merge_complete=1 at the end of sched_begin_session after all
staging work (sched_account_jobs etc.) completes.
F2 (HIGH): recompact_node_ids() omitted from GC protocol in sched_end_session.
The protocol in resreq.h requires gc_sweep → recompact_node_ids. Without
recompaction, sel_node_next_id grows monotonically across sessions and
select_eval_ctx_t arrays (sized by max node_id) grow without bound.
Fix: call recompact_node_ids() after gc_sweep_select_nodes.
Also adds test_staging_merge_complete_gc_guard to document the bootstrap
invariant: sched_begin_session must leave staging_merge_complete=1 so the
GC guard in sched_end_session does not fire.
- simple_num/simple_str: guard h->numvals/strvals NULL and idx out-of-range before array access
- eval_select_simple SEL_RESNAME: guard host->numvals NULL and idx bounds before deref
- eval_select_inner SEL_RESNAME (memoizing path): same NULL/bounds guards for numvals and strvals
- build_host_eval_ctx: return valid empty context (num_res=0, NULL arrays) instead of NULL when num_res<=0
- sched_meet_host_selection: remove empty_ctx stack hack; always call build_host_eval_ctx which now handles the zero-resource case cleanly
Replace the always-return-1 stub with a real implementation that:
- Returns 1 immediately when s == NULL or s->root == NULL (no constraint)
- Builds a host_eval_ctx_t from w->resvals and calls eval_select_simple
- Uses a zero-resource stack context when resvals.caps == 0 so constant
sub-expressions (SEL_TRUE, SEL_FALSE, SEL_ANY) still evaluate correctly
- On OOM (build_host_eval_ctx returns NULL), conservatively passes the host
- Maps eval_select_simple result: >0 → 1 (match), <=0 → 0 (no-match)
Add #define UNIT_TEST 1 to test.c, remove redundant #ifdef guard, and
add three new TDD tests covering NULL select, SEL_TRUE, and SEL_FALSE.
All 6 tests pass.
