chore(wolf): mark bug-1679 fixed; log bug-1680 (xcecli jobs list -a returns nothing, discovered via bug-1679 verification)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
parent
8d6f610ef9
commit
e787cead73
1 changed files with 27 additions and 3 deletions
|
|
@ -18583,7 +18583,7 @@
|
|||
"error_message": "A job's live dispatch/start/finish transitions are never pushed to the apiserver -- xcecli jobs list shows a job stuck at PEND forever (or, in one observed case, disappearing from the default query entirely) even though the controller's own WAL and in-memory state correctly progress the job all the way through RUN to EXIT/DONE. Discovered as a side effect while live-verifying bug-1678 (restart-recovery of job.allocs) -- this is a SEPARATE, unrelated, more severe bug: it reproduces with NO restart involved at all, on a single, freshly-submitted job, every time. Confirmed reproducible on the STOCK branch (git-stashed all of bug-1678's changes, rebuilt, redeployed fresh) -- NOT a regression introduced by bug-1678 or any other fix landed this session (bug-1673/1674/1675/1676/1677 touch only Go/apiserver code, a Python script, or job_submit_data_t's from_host field + jobj.c's dedup identity check -- none touch the state-transition/hook-firing logic implicated here).",
|
||||
"file": "src/controller/job.c (dispatch_a_job_, on_job_start, and likely other direct-mutation call sites), src/controller/persist.c (apply_committed_entry's WAL_JOB_START/.../DISPATCH shared case, apply_job_update_transition)",
|
||||
"root_cause": "dispatch_a_job_() (job.c, scheduler-dispatch-commit's task 3.1 restructuring) mutates j->job.state DIRECTLY in memory (job.c ~1355, `j->job.state = JOB_STAT_RUN;`) BEFORE calling persist_submit_job_dispatch_fact()/persist_submit_job_fact() to durably record the transition. On a live, single-node, self-committing controller, the commit-then-apply pipeline runs synchronously enough that apply_committed_entry()'s own WAL_JOB_DISPATCH handling (via apply_job_update_transition(j, JOB_STAT_RUN)) sees `cur == state == JOB_STAT_RUN` -- because the state was ALREADY set moments earlier by the same call, under the same lock -- and returns JOB_TRANSITION_NOOP_DUPLICATE. The shared case body's `if (result != JOB_TRANSITION_APPLIED) return 0; /* rejected-backward / no-op-duplicate -- no hook */` then unconditionally skips xce_run_hooks(HOOK_AFTER_EVENT, HOOK_DETAIL_JOB_UPDATE, ...) -- the ONLY mechanism (via apiserver_job_hook_fun/apiserver_push_joblist in apiserver.c, registered against XCE_CONTROLLER_HOOK_JOB) that pushes an updated job record to the apiserver. Live-confirmed via controller log: `apply_job_update_transition: job <id>: duplicate UPDATE to state 0x2 -- no-op` fires for BOTH the WAL_JOB_DISPATCH commit AND the subsequent WAL_JOB_START commit (on_job_start(), job.c) on the exact same job, back to back -- meaning the job NEVER gets a single HOOK_DETAIL_JOB_UPDATE fire through its entire dispatch. apiserver_push_joblist() itself only ever fires ONCE per job, at creation time (apiserver_job_new_fun, subscribed to HOOK_DETAIL_JOB_NEW, itself fired once by apply_committed_entry's WAL_JOB_SUBMIT case) -- there is no periodic/polling full-refresh fallback, confirmed by reading apiserver_push_joblist's only call site. So a job's apiserver-visible record is permanently frozen at whatever it looked like at creation (JOB_STAT_PEND, no exec_host) for its entire lifetime, regardless of what actually happens to it afterward, UNLESS some other code path elsewhere in job.c legitimately fires the hook directly (mutate-then-notify, like mark_job_update()/persist_job_hook_fun's 'still-legitimate append' pattern) rather than relying on the apply-time hook-fire -- not confirmed either way for on_job_exit/mark_job_fail/other terminal-transition call sites; only dispatch_a_job_() and on_job_start were directly traced. This is very likely a structural gap introduced by (or newly exposed by) the raft-apply-decoupling / scheduler-dispatch-commit redesigns moving job-state mutation to happen synchronously ahead of persistence for several call sites, while the apiserver-notification hook-fire mechanism was designed under an assumption (state hasn't already changed by the time apply runs) that held for the OLD architecture but no longer holds for these newer, direct-mutation call sites.",
|
||||
"fix": "NOT FIXED -- discovered and root-caused as a side effect of bug-1678 verification, out of that fix's scope (bug-1678 is specifically about restart/replay recovery of job.allocs, verified independently via unit tests that inspect controller-internal state directly, bypassing this broken notification path entirely). Needs its own investigation: (1) confirm the full list of live call sites with this same mutate-then-persist-then-rely-on-apply-hook shape (dispatch_a_job_, on_job_start confirmed; on_job_exit/mark_job_fail/others not yet checked), (2) decide the fix architecture -- either make each such call site fire HOOK_DETAIL_JOB_UPDATE directly/synchronously itself (mirroring mark_job_update()'s existing mutate-then-notify pattern, which the persist_job_hook_fun's Hazard-1 dual-origin guard already anticipates and handles), or change apply_committed_entry's shared case to fire the hook even on a same-value transition specifically for these WAL types (riskier -- could reintroduce duplicate-notification issues the NOOP_DUPLICATE check was presumably added to prevent elsewhere), (3) full regression + live E2E verification that a freshly-submitted job's status visibly progresses through xcecli jobs list without any restart involved.",
|
||||
"fix": "FIXED (2026-08-25). Root cause confirmed exactly as filed: dispatch_a_job_() (job.c) 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 and skips its would-be notify (JOB_TRANSITION_NOOP_DUPLICATE).\n\nNew src/controller/persist.c function persist_notify_job_updated_already_persisted(cluster_t*, job_t*) (declared in src/include/persist.h), called from dispatch_a_job_()'s success branch immediately after unlock(j). Internally guards with the exact same t_apply_seam_notifying flag apply_committed_entry()'s own shared WAL_JOB_START/.../DISPATCH case already uses around its identical hook fire (same guard, same hook -- HOOK_DETAIL_JOB_UPDATE's AFTER_EVENT -- same detail) -- without it, persist_job_hook_fun (also subscribed to this hook) would redundantly re-append the same transition as a second, duplicate WAL_JOB_UPDATE entry. The flag stays file-private to persist.c; job.c reaches it only through this one new function. dispatch_a_job_()'s own mutation/persistence logic is otherwise untouched.\n\nTDD: 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. RED->GREEN independently reproduced: temporarily disabled the new call (production code unchanged otherwise), rebuilt, confirmed FAIL (Expected 1 Was 0); restored, rebuilt, confirmed PASS. Verified the two existing tests that would catch a broken guard (double WAL append): 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, and the former's fixture genuinely wires persist.cluster with persist_job_hook_fun registered, so it actually exercises the guard. Full controller_unit_test: 1137/1137 (2 pre-existing test_config_repl.c failures -- the known overlayfs fdatasync errno=5 flake, bug-1561 -- confirmed identical via git-stash on stock code, not a regression).\n\nLive-verified on a fresh single-node deploy (rebuilt controller/scheduler/worker/xcecli): submitted a job, xcecli jobs list showed PEND at t+1s then RUN with EXEC_HOST=1*worker* from t+2s onward continuously -- no restart involved, matching the exact acceptance criterion. Also confirmed record_job_finish_fact's existing notification path needed no fix: on_job_finish fired correctly and xcecli jobs list -d showed DONE with EXEC_HOST intact for both test jobs, exactly as the root-cause analysis predicted (only dispatch_a_job_()'s broken hybrid pattern was ever the problem -- the durable-append-only pattern record_job_start_fact/record_job_finish_fact use was already correct).",
|
||||
"tags": [
|
||||
"controller",
|
||||
"job.c",
|
||||
|
|
@ -18597,10 +18597,34 @@
|
|||
"discovered-via-bug-1678"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1678"
|
||||
"bug-1678",
|
||||
"bug-1680"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-25T03:15:00Z"
|
||||
"last_seen": "2026-08-25T03:50:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1680",
|
||||
"timestamp": "2026-08-25T03:50:00Z",
|
||||
"error_message": "`xcecli jobs list -a` (\"list run,done,pending,suspended jobs\" -- i.e. show every job regardless of status) returns \"no job found\" even when jobs genuinely exist and are correctly stored with a real status. Discovered as a side effect of live-verifying bug-1679: two jobs that correctly showed PEND->RUN->DONE via the default query and via `-d` (done-only) both come back empty under `-a` specifically. Not a data-loss bug (the jobs ARE correctly stored and queryable through other flags -- `-d`, the default active-jobs view -- this is specifically `-a`'s own query construction that's broken) but a real, reproducible CLI defect: `-a` is documented as the way to see literally everything, and it shows nothing.",
|
||||
"file": "src/frontend/xcecli/filters/job_filter.go:58-60, src/frontend/apiserver/utils/types.go:262-322, src/frontend/apiserver/store/query.go:70-73",
|
||||
"root_cause": "xcecli's `-a` flag (list.go:347, `AllJobInfoFlag`) makes HandleJobFilter (job_filter.go:57-60) build the filter string `{job_status:notnull:}` -- a `notnull` operator on `job_status` with an EMPTY value token (no argument needed for null-checks, by design) -- instead of the normal `{job_status:in:(...)}`/`{job_status:notin:(...)}` shape every other status flag (-p/-r/-d/-s) produces. Server-side, `job_status`/`host_status` get SPECIAL bitmask handling in buildIntCondition (types.go:262-322), but that special-casing only triggers for the `in`/`notin`/`between` operators (the `if filter.Field == \"job_status\" || filter.Field == \"host_status\"` branch sits inside the `if filterOperator == \"in\" || filterOperator == \"notin\" || filterOperator == \"between\"` block, types.go:263-270) -- `notnull` falls through to the generic scalar path (types.go:316-322), which builds a plain `Condition{Column: \"job_status\", Operator: OpNOTNULL, Value: {TypeInt, 0}}` with no bitmask awareness at all. In isolation, store/query.go's own MatchesCondition evaluator (query.go:70-73) implements OpNOTNULL correctly (`fieldVal != nil`), which should trivially match every job (job_status is never actually absent/nil in a stored row) -- meaning the actual break is somewhere between condition construction and evaluation for this specific field/operator combination that was not fully traced (a debugger/print-tracing session would be needed to pin the exact line): plausible candidates are (a) the query engine routing job_status conditions through a bitmask-specific code path unconditionally regardless of operator, silently misinterpreting a non-bitmask NOTNULL Condition as a bitmask EQ/NE against value 0 (which would exclude every real job_status value, all of which are nonzero bitmask flags), or (b) some other job_status-specific special-casing elsewhere in the query pipeline that only handles the `in`/`notin`/`eq`/`ne` shapes types.go's own bitmask branch produces and never anticipated a bare NOTNULL condition on this field at all.",
|
||||
"fix": "NOT FIXED -- discovered and partially traced (client-side filter construction and the operator-to-condition-building split confirmed precisely; server-side evaluation/routing gap not fully pinned down) as a side effect of live-verifying bug-1679, out of that fix's scope. Needs: (1) instrument or step through the actual query path a `{job_status:notnull:}` filter takes from controllers/jobs.go's filter parsing through to whatever executes against store.Jobs, to find exactly where a job_status NOTNULL condition stops matching real rows: (2) once found, either extend job_status's bitmask special-casing (types.go:270) to also cover null/notnull (probably the right fix -- job_status is ALWAYS a bitmask int, so NOTNULL should just mean 'true, unconditionally' for this field, similar to how notin's bitmask branch already has an unconditional-match escape hatch) or fix whatever else is misrouting/misevaluating this specific condition shape; (3) add xcecli/apiserver test coverage for `-a` specifically (grep found none) so a regression here isn't invisible again.",
|
||||
"tags": [
|
||||
"xcecli",
|
||||
"apiserver",
|
||||
"job_filter.go",
|
||||
"types.go",
|
||||
"query.go",
|
||||
"job-status-filter",
|
||||
"cli-bug",
|
||||
"discovered-via-bug-1679"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1679"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-25T03:50:00Z"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue