merge: sync Rongsong's scheduler-framework (88 commits, f01ae353..c5b172c7)
Two-sided divergence from f01ae353 (2026-08-30): theirs +88 (worker
rejoin-resync reconciliation, terminal-state bug-1737/1738/1739/1764,
scheduler-sim verbosity, hermetic offline build, go mod vendor of Go deps
in apiserver -- 3375 pure-adds), ours +89 (job-finish robustness bug-1509
B1/B3, PEND-reason, worker reconnect resend, apiserver job control).
Conflict resolutions (kept both sides' fixes):
- controller_connector.c checking_connection(): combine theirs
xcecd_resync_is_pending() rejoin deferral gate with ours
resend_job_status() actual-state re-report on rejoin success.
- job.c: keep both unknown-finish registry (ours, bug-1509) and
record_job_finish_fact_at (theirs, bug-1747); keep both
mark_jobs_unknown_core family (ours) and record_job_reconcile_*
(theirs). No double-notify: on_job_finish unchanged, theirs is
resync-path-only.
- scheduler handler.c/test_handlers.c/meson: theirs schedExitJob
exit_code branch (bug-1737) + ours jobDispatchFail success-ack no-op
(bug-1512) + configpush enq_pend_update (bug-1513) both present; both
test suites and REPL/sim targets wired.
- apiserver jobs.go: keep ours normalizeJobInt64/jobArrayIdx superset +
JobIDHuman; drop theirs strict IntOrInt64 early-reject (superseded --
ours tolerates float64/json.Number, no panic path).
- jobs_test.go: keep both regression suites (ours bug-1490, theirs
bug-1742).
- .kiro/specs scheduler-sim-*/worker-rejoin-resync + .wolf/*: keep our
deletions (established convention -- drop Rongsong bookkeeping on merge).
Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
commit
5e2985542f
3469 changed files with 1466599 additions and 382 deletions
501
.kiro/specs/worker-rejoin-resync/design.md
Normal file
501
.kiro/specs/worker-rejoin-resync/design.md
Normal file
|
|
@ -0,0 +1,501 @@
|
|||
# Design Document: worker-rejoin-resync
|
||||
|
||||
## Overview
|
||||
|
||||
On every worker (re)join — a fresh network reconnect after a disconnect, or a reconnect to a new primary after controller failover — this feature has the controller ask the worker directly what really happened to every job the controller still believes is running there, and reconciles the worker's answer into the job table through the existing idempotent commit-then-apply path. This closes the gap left by fire-and-forget job start/finish/fail notifications (`XCECLI_FF_CALL`): a report lost to a disconnect or failover window today leaves a job stuck in `RUN` forever, with nothing in the cluster ever correcting it (bug-1741, bug-396).
|
||||
|
||||
**Users**: Internal controller/worker/scheduler subsystems — there is no end-user-facing surface. Cluster operators benefit indirectly: jobs self-heal out of a stuck-`RUN` state without manual intervention.
|
||||
|
||||
**Impact**: Extends `rejoin_worker`/`apply_join_state` (`worker-join-refactor`, implemented) with a job-state reconciliation round; adds one new opcode pair to the wire protocol; adds a short-lived finished-job retention to the worker daemon (`xcecd`), which retains none today; adds two narrow new fact-recording functions to the controller's job table, reusing its existing idempotency guards for everything else; adds two new shared-config fields; adds one line to the scheduler's existing candidate-host filter; and, because Requirement 4.6 is the first path in this codebase that re-admits an already-dispatched job back to the scheduler's PEND pool, closes a previously-dormant gap (bug-1631) in `dispatch_a_job_`'s retryable-failure handling that this feature would otherwise make reachable.
|
||||
|
||||
### Goals
|
||||
- Trigger reconciliation on every worker join where the controller holds non-terminal jobs for that host, without needing to distinguish first-join from reconnect (Requirement 1).
|
||||
- Query only the jobs in question; let the worker answer definitively for each, plus report anything running/recently-finished the controller didn't ask about (Requirements 2, 3).
|
||||
- Apply every reconciled outcome through the existing idempotent commit-then-apply path — no second mutation mechanism (Requirement 4).
|
||||
- Withhold new dispatch to a worker while its reconciliation is outstanding, and fail fast (not queue) a dispatch to a disconnected worker (Requirement 5).
|
||||
- Stay safe under repeated/interrupted rejoin (Requirement 6) and reach the scheduler through the same notification path as any ordinary job-state change (Requirement 7).
|
||||
- Work regardless of how many jobs a host is running, via a bounded/paginated exchange (Requirement 8).
|
||||
|
||||
### Non-Goals
|
||||
- apiserver/scheduler cluster/queue/job-*list* resync on reconnect (`component-rejoin-resync`, a disjoint sibling spec — different data shape, not merged here).
|
||||
- Any change to the primary-side pending-facts queue (the commit engine) itself — this feature is an additional caller into it, never a modification of it.
|
||||
- Delta-based / session-token re-attach optimization for rejoin in general (separate, deferred, explicit YAGNI).
|
||||
- Coupling the Requirement 3.7 reporting window to `commit-batch-window`'s (separately parked) commit-urgency tuning — disclaimed in requirements.md's Boundary Context and not referenced anywhere in this design.
|
||||
- Recovering a reconciliation-assigned job's original resource allocation (see Data Models — this is a real, explicitly-documented limitation, not a silent gap).
|
||||
|
||||
---
|
||||
|
||||
## Boundary Commitments
|
||||
|
||||
### This Spec Owns
|
||||
- The wire protocol for the reconciliation exchange: two new opcodes, `OPCode_jobStateQuery` (controller → worker) and `OPCode_jobStateReport` (worker → controller), both paginated.
|
||||
- Controller-side: deciding when to trigger reconciliation (Requirement 1); tracking one reconciliation round per worker (queried job IDs, round token, deadline); applying every reported outcome via the job table's existing idempotent fact path, plus two new narrow fact-recording functions for the two outcomes with no existing analog (reconcile-to-pending, reconcile-assign) — **Correction (2026-09-01, bug-1747, task 3.2): a third new function, `record_job_finish_fact_at`, was added for the finished-success outcome's own Requirement 4.2 fix; see the Persistence Field-Completeness Review section's own correction note for the full rationale** — gating new dispatch on reconciliation-outstanding state; notifying the scheduler through the existing inbound notify path. Reconciliation is modeled as extra steps that run strictly *after* the worker-join RPC (`xce_join`/`xcecli_join`) has already completed — never as an extension of that call's own blocking wait (see Out of Boundary).
|
||||
- Worker-side: a new short-lived retention of recently-finished job outcomes (jobid, exit status, real finish time), bounded by a configurable window; answering the controller's query from that retention plus the still-running job list; reporting extra jobs outside the query; a new local gate flag that delays the worker's own resumption of normal post-join operation until its report has finished sending (Requirement 3.8).
|
||||
- Scheduler-side: one narrow addition — the existing candidate-host filter (`scheduler.c:1211`, today `IS_WORKER_UNAVAIL`-only) also excludes a host with `XCE_WORKER_RECONCILING` set, so a RECONCILING host is never proposed as a dispatch target in the first place (Requirement 5.1). No other scheduler-side change; the worker-status mirror this reads from already exists (`core_events.c:527`/`:595`) and needs no new wire content for this bit.
|
||||
- Two new shared-config fields (the Requirement 3.7 reporting window; the Requirement 5.3 reconciliation timeout).
|
||||
- **Addendum (2026-09-01, added during implementation):** xceadmin's `config build` YAML→`ClusterConfig` pipeline, scoped narrowly to making the two new fields operator-settable through it, consistent with how their own cited precedent (`heartbeatInterval`/`inactiveHeartbeatCount`) is already wired. This was missed by the original design review — design.md's Configuration Model section says these fields are distributed "via the existing shared-config push mechanism," which is only true end-to-end if xceadmin can actually produce a `ClusterConfig` binary containing operator-supplied values for them; as originally scoped, neither field had a path from YAML into the binary at all. Concretely, in scope: `src/configs/schema/xce.schema.json` (add both fields, same shape/validation style as `heartbeatInterval`), `src/xceadmin/src/xcecommon/mapping.py` (add both to the YAML-field-name → capnp-field-name tuple list at line ~179, immediately following the `heartbeat_interval`/`inactive_heartbeat_count` pair), and the checked-in golden-fixture binaries under `src/xceadmin/tests/fixtures/{minimal,full}/expected.bin` (regenerate — task 1.1 already changed `ClusterConfig`'s wire layout, which these fixtures predate; regenerating them before this field is wired would need redoing once it lands, so do both together). Out of scope, still: any change to `xceadmin`'s validation/business-rule logic beyond the two-field addition, and any question of whether these two fields *should* be operator-facing at all versus hardcoded-only (assumed yes, matching their cited precedent, not re-litigated here).
|
||||
|
||||
### Out of Boundary
|
||||
- apiserver/scheduler resync-on-reconnect content (`component-rejoin-resync`).
|
||||
- The commit engine / pending-facts queue mechanism itself (`commit.c`) — used, never modified.
|
||||
- Delta/session-token rejoin optimization.
|
||||
- `commit-batch-window`'s commit-urgency tuning — no dependency, no reference.
|
||||
- Recovering `job->allocs` (the scheduler's resource allocation) for a job reconciled via Requirement 4.8 that the controller never held a dispatch record for — the data was never known to this controller and cannot be reconstructed from the worker's report. Converges via the existing reserved/actual accounting policy on the job's next rusage report (see Data Models), same as any legitimately-unreserved job today.
|
||||
- A parallel WAL fact/decode path for the reconcile-assign case: bug-1678 (fixed 2026-08-25, predates this design) already gave `WAL_JOB_DISPATCH` a `persist_submit_job_dispatch_fact(persist_t*, jobid, state, ts, alloc)` primitive with both apply seams (`apply_committed_entry`, `apply_wal_entry`) decoding an *optional* trailing `JobAlloc` on the **decode** side, and `record_job_reconcile_assign_fact` calls this primitive directly with `alloc=NULL` (see Data Models) rather than inventing a second payload shape — out of boundary is building anything new here, not reusing what already exists. **Correction (2026-09-01, found live during task 2.2 implementation, bug-1746):** the *encode* side of "NULL alloc" was never actually exercised or tested before this feature — `dispatch_a_job_` (bug-1678's only caller) always passes a real, non-NULL allocation, so a latent NULL-pointer dereference in `capnp_encode_joballoc()` (`persist.c`, calls the unsafe `encode_JobAlloc()` instead of the already-existing NULL-safe `encode_JobAlloc_ptr()` used elsewhere in the same generated file) was never triggered until this feature's `record_job_reconcile_assign_fact` became the first genuinely-NULL caller. Fixed as a minimal guard in `capnp_encode_joballoc` (produces a true zero-length buffer for NULL input, matching exactly what the decode side's `if (alloc_len > 0)` check already expects) — in boundary for task 2.2, since it's the direct, minimal companion fix blocking this call, not new machinery.
|
||||
- Extending `xce_join()`/`xcecli_join()`'s own blocking wait (`cbuf_deadline_10s`, `xceapi.c`) to also cover config push or job-state reconciliation. That call is a shared, role-agnostic API (worker, apiserver, scheduler, and `xce_test`/`xceadmin` clients all use it) and already returns as soon as the join reply arrives — well before `on_worker_join` (config push) even runs, unchanged by this feature. Reconciliation has its own, independent, server-side timeout (`resync.c`'s `delaymgr` trigger against `workerResyncTimeoutMs`) and is not gated on, or gating of, the join RPC's own deadline in any way.
|
||||
|
||||
### Allowed Dependencies
|
||||
- `rejoin_worker` / `apply_join_state` (`worker-join-refactor`, implemented) — the plug-in point for triggering reconciliation; this design must not regress its existing attribute/endpoint-update/admin-bit-preservation behavior.
|
||||
- The job table's commit-then-apply seam (`record_job_*_fact`, `apply_job_*_transition`, `job_state_is_terminal`, `job_state_is_backward`, `apply_committed_entry`) — reused, never duplicated.
|
||||
- `XCE_CONTROLLER_HOOK_JOB` fan-out, `sched.c`'s hook callbacks, and `notify()` (`dataex.c:399`) — the existing controller→scheduler notify path.
|
||||
- `xce_reply_get`/`xce_push_data_req`'s `{pos, count, n_total}` pagination idiom (`request.c`) — the template for the new opcode pair.
|
||||
- `commit.c`'s `COMMIT_WAIT_RESERVED` admission-reservation pattern — the template for any bounded wait on the worker's report.
|
||||
- `config-design.md`'s shared-config classification rule, applied to the two new config fields.
|
||||
|
||||
### Revalidation Triggers
|
||||
- Any change to `worker_t.host.status` bitmask semantics or `dispatch_a_job_`'s connected-state check (job.c:1511-1512) — the dispatch-gating reuse (Requirement 5.1) depends on both staying exactly as observed. **This trigger fired**: `scheduler-dispatch-commit` (implemented 2026-08-24/25, after this design's original research) added `xce_classify_dispatch_fail()`'s retry/terminal-fail machinery behind this exact check, and exposed bug-1631 (job.allocs never released on a retryable-failure path, deliberately left unfixed pending a re-admission path this design provides). Re-validated 2026-08-31/09-01 — see Architecture, Components, Error Handling for the resulting design changes (scheduler-side candidate exclusion, split disconnected-worker branch, explicit allocs release in `record_job_reconcile_pending_fact`).
|
||||
- Any change to `apply_job_update_transition` / `job_state_is_backward`'s RUN↔PEND handling.
|
||||
- Any change to the `XCE_CONTROLLER_HOOK_JOB` fan-out shape or the `NotifyData` capnp union.
|
||||
- Any change to `JoinData`'s capnp union arms (this design deliberately does not touch it, but a future change there should re-check this design's choice to use a separate opcode pair instead).
|
||||
- `component-rejoin-resync`'s eventual design, if it ever proposes touching the join/rejoin wire exchange mechanics this design also touches.
|
||||
|
||||
---
|
||||
|
||||
## Architecture
|
||||
|
||||
### Existing Architecture Analysis
|
||||
|
||||
`rejoin_worker`/`apply_join_state` (`worker.c:403`, `:321`) already run under the per-worker write lock at the exact moment `w->ep` is set to the new live endpoint and `XCE_WORKER_UNAVAIL` is cleared — this is the moment the host becomes "connected" again, and the natural trigger point for Requirement 1's "does the controller hold non-terminal jobs for this host" check. `handle_worker_join` (join.c:48) commits the join durably (`persist_commit_barrier`) before `on_worker_join` runs — a second, later point, after the join itself can no longer be rolled back, where the reconciliation query can be sent.
|
||||
|
||||
The job table's existing commit-then-apply split is the load-bearing idempotency mechanism this feature must reuse (per requirements.md's Boundary Context): `record_job_*_fact()` functions append a WAL fact (durable, no mutation), and `apply_job_*_transition()` functions are the guarded mutation core, invoked both as a peek at receipt time and later by `apply_committed_entry` once the fact commits. `job_state_is_terminal`/`job_state_is_backward` reject duplicate or backward (terminal→non-terminal) transitions — this is the exact "no duplicate facts, no clobbering newer state" guarantee Requirement 4 asks for, already built and tested (`test_guard_replay_finish_equals_single_application`).
|
||||
|
||||
Dispatch gating has two layers today, and both matter to this design. `dispatch_a_job_` (job.c:1278) refuses to dispatch when `w->ep == NULL || w->host.status != 0` (job.c:1511-1512) — but since `scheduler-dispatch-commit` (implemented 2026-08-24/25, after this design's original research), hitting that check no longer just returns an error: it routes through `xce_classify_dispatch_fail()` into a bounded-retry-then-`mark_job_fail()` outcome (job.c:1538-1569, site `XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY`) for `w->ep == NULL` and any nonzero status bit alike, with no distinction between them. Separately, bug-1631 (confirmed live, deliberately left unfixed) documents that none of `dispatch_a_job_`'s retryable-failure branches release an already-committed `job->job.allocs` — scoped as unreachable *only* because nothing in the current system re-admits an already-`RUN` job back into the scheduler's PEND pool. Requirement 4.6 is exactly such a re-admission path, so this design cannot treat that gap as pre-existing/out-of-scope (see Components, Error Handling).
|
||||
|
||||
Given both findings, `XCE_WORKER_RECONCILING` dispatch-gating is primarily enforced one layer up, at scheduler candidate selection (`scheduler.c:1211`'s existing `IS_WORKER_UNAVAIL`-style filter, extended — see Components), so the scheduler simply never proposes a RECONCILING host as a dispatch target. `dispatch_a_job_`'s status check remains an unchanged defensive backstop for the rare race where a decision predates the bit being observed, correctly falling into the same retry-then-terminal-fail path every other non-UNAVAIL status bit already takes today — unmodified. Requirement 5.4 (a truly disconnected worker, `w->ep == NULL`) needs a genuinely new branch in `dispatch_a_job_`, split out from `XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY`.
|
||||
|
||||
The controller→scheduler notification path is a hook→notify→wire→dequeue pipeline: job-state-change code fires `XCE_CONTROLLER_HOOK_JOB` (job.c), `sched.c`'s registered callback (`sched_job_hook_fun`) dispatches by `HOOK_DETAIL_*` to `sched_job_finish_fun`/`sched_job_fail_fun`/`sched_job_update_fun`, each of which calls the shared `notify()` (dataex.c:399) with a distinct `NotifyData_data_sched*` event over `c->sched.ep`. On the scheduler side, `sched_handle_notify()` (handler.c:525) is the **inbound** receipt switch that calls `sched_job_state_change()`; this is architecturally distinct from `scheduler.c`/`driver.c`'s own **outbound** decision-publish direction, which a standing test (`test_no_worker_resync_symbols_in_scheduler_publish_path`) mechanically guards against ever referencing "resync." This fixes the only permitted integration seam: `handler.c`'s inbound switch.
|
||||
|
||||
The worker daemon retains no finished-job state today: `cleanup_worker_list()` (xcecd_worker_utils.c:385) deletes both the in-memory entry and its sqlite row on the pass immediately after a job exits, and neither `xcecd_worker_t` nor `xcecd_job_t` carries a timestamp field. `JoinData`'s capnp union (`nodata`/`attrs`) carries no job content and cannot be extended in place without forcing the exchange through `xce_join()`'s single-shot synchronous reply, which cannot paginate.
|
||||
|
||||
### Architecture Pattern & Boundary Map
|
||||
|
||||
```mermaid
|
||||
graph TB
|
||||
subgraph Controller
|
||||
JC[handle_worker_join / join.c]
|
||||
RW[rejoin_worker / apply_join_state / worker.c]
|
||||
RS[Reconciliation state: worker_t.resync]
|
||||
RQ[Reconciliation query+timeout / new resync.c]
|
||||
JOBC[job.c: record_job_finish_fact / record_job_fail_fact<br/>record_job_reconcile_pending_fact NEW<br/>record_job_reconcile_assign_fact NEW<br/>record_job_finish_fact_at NEW - bug-1747]
|
||||
COMMIT[commit.c / apply_committed_entry — unchanged, reused]
|
||||
HOOK[XCE_CONTROLLER_HOOK_JOB fan-out]
|
||||
SCHEDC[sched.c hook callbacks]
|
||||
NOTIFY[notify / dataex.c]
|
||||
end
|
||||
subgraph Worker[xcecd]
|
||||
WJOIN[Join / rejoin]
|
||||
RETAIN[NEW: finished-job retention]
|
||||
RUNNING[Existing: xcecd_worker_t running list]
|
||||
QHANDLE[NEW: job-state query handler]
|
||||
end
|
||||
subgraph Scheduler
|
||||
HANDLER[handler.c sched_handle_notify calls sched_job_state_change]
|
||||
CANDFILT[scheduler.c:1211 candidate filter<br/>NEW: also skips IS_WORKER_RECONCILING<br/>same shape as existing IS_WORKER_UNAVAIL check]
|
||||
PUBLISH[scheduler.c / driver.c: outbound decision publish — NOT touched otherwise]
|
||||
end
|
||||
|
||||
JC --> RW --> RS
|
||||
RS -->|non-terminal jobs for host?| RQ
|
||||
RQ -->|OPCode_jobStateQuery, paginated| WJOIN
|
||||
WJOIN --> QHANDLE
|
||||
QHANDLE --> RETAIN
|
||||
QHANDLE --> RUNNING
|
||||
QHANDLE -->|OPCode_jobStateReport, paginated| RQ
|
||||
RQ --> JOBC
|
||||
JOBC --> COMMIT --> HOOK --> SCHEDC --> NOTIFY --> HANDLER
|
||||
HANDLER -.->|never referenced from| PUBLISH
|
||||
RS -.->|existing worker-status mirror, core_events.c:527/595<br/>no new wire content| CANDFILT
|
||||
CANDFILT -.->|ordinary status-bit read, not a resync concept —<br/>no resync symbol added to this file| PUBLISH
|
||||
```
|
||||
|
||||
**Architecture Integration**:
|
||||
- Selected pattern: additive extension of an existing daemon-to-daemon RPC + existing fact/apply pipeline — no new subsystem, no new process.
|
||||
- Domain/feature boundaries: wire protocol (new opcode pair) / controller reconciliation bookkeeping (new, small) / job-table fact recording (three new functions — `record_job_reconcile_pending_fact`, `record_job_reconcile_assign_fact`, and `record_job_finish_fact_at` (**Correction, 2026-09-01, bug-1747**: added by task 3.2 for Requirement 4.2's finished-success fix; the original count of two predates that fix) — rest reused) / worker retention (new) / scheduler notify (zero new code — reused as-is) / scheduler candidate-filter (one added disjunct to an existing check, see below — not zero, but not new machinery either).
|
||||
- Existing patterns preserved: commit-then-apply idempotency, `{pos,count,n_total}` pagination, hook→notify scheduler path, per-worker write-lock discipline, and — newly leaned on — the scheduler's existing worker-status-bit candidate filter (`IS_WORKER_UNAVAIL` at `scheduler.c:1211`).
|
||||
- New components rationale: reconciliation bookkeeping and worker retention have no existing analog to extend; everything else is reuse.
|
||||
- **Scheduler-side touch, and why it doesn't cross `test_no_worker_resync_symbols_in_scheduler_publish_path`'s fence**: that standing test guards `scheduler.c`/`driver.c`'s *outbound decision-publish* direction against ever learning reconciliation *protocol* concepts (round tokens, query/report messages, retention windows). The one line this design adds there — skip a host if `IS_WORKER_RECONCILING(w->status)` — is not that: it's the same *category* of check as the pre-existing `IS_WORKER_UNAVAIL(w->status)` line right next to it, reading an ordinary worker-status bit the scheduler already mirrors for an unrelated reason (`core_events.c:527`/`:595`). No opcode, no reconciliation-round concept, and no symbol named "resync" is introduced into either file. This is called out explicitly, not left for an implementer to discover the fence and guess whether it applies.
|
||||
- Steering compliance: async-I/O thread-role contract (bounded wait via admission reservation, not raw blocking); persistence per-field completeness review (see Data Models); config-design.md classification (see Data Models).
|
||||
|
||||
### Technology Stack
|
||||
|
||||
| Layer | Choice / Version | Role in Feature | Notes |
|
||||
|-------|------------------|------------------|-------|
|
||||
| Wire protocol | Cap'n Proto, `src/schema/controller.capnp` | New `OPCode_jobStateQuery`/`OPCode_jobStateReport`, new payload structs | Mirrors `JobData`/`WorkerData`/`ResourceVals`'s existing `{ntotal,pos,count}` shape |
|
||||
| Controller | C (gnu17), `src/controller/` | Reconciliation trigger, bookkeeping, fact recording, notify | `worker.c`, `job.c`, new `resync.c`, `cluster.c` (opcode dispatch) |
|
||||
| Worker daemon | C (gnu17), `src/worker/` | Finished-job retention, query handling | `xcecd_worker_utils.c/h`, new `xcecd_resync.c` |
|
||||
| Async I/O | iomgr/vtaskmgr/delaymgr, `src/lib/` | Non-blocking query send/receive; bounded wait via admission reservation | Reuses `commit.c`'s `COMMIT_WAIT_RESERVED` pattern |
|
||||
| Shared config | Cap'n Proto `ClusterConfig` | Reporting window (Req 3.7); reconciliation timeout (Req 5.3) | Alongside existing `heartbeatInterval`/`inactiveHeartbeatCount` |
|
||||
|
||||
---
|
||||
|
||||
## File Structure Plan
|
||||
|
||||
### Modified Files
|
||||
- `src/schema/controller.capnp` — add `OPCode_jobStateQuery`, `OPCode_jobStateReport` to the `OPCode` enum; add `JobStateQuery{jobids: List(UInt64), pos, count, ntotal}`, `JobStateReport{entries: List(JobStateEntry), pos, count, ntotal}`, `JobStateEntry{jobid, status: JobStateStatus, exitCode, finishTimeMs, queried: Bool}` (`queried=false` marks a Requirement 3.6 extra job); add both as `Request`/`Reply` union arms; add `workerResyncWindowMs`/`workerResyncTimeoutMs` fields to `ClusterConfig`.
|
||||
- `src/controller/worker.c` — `rejoin_worker`/`apply_join_state`: after clearing `XCE_WORKER_UNAVAIL`, check for non-terminal jobs assigned to this host (Requirement 1) and, if any, set `XCE_WORKER_RECONCILING` and initialize the per-worker reconciliation record before releasing the write lock. No change to admin-bit preservation or attribute/endpoint update behavior. `on_worker_join` (worker.c:623) — extended, not `handle_worker_join`: it already calls `worker_push_config` as its one existing step (the config the worker needs — hosttype/hostmodel/resource variables — is foundational baseline state, and its own stub comment, `/* consider to push jobs on hosts to */`, already anticipates this exact follow-on). The reconciliation query send is added right after it, in that fixed order (config push, then query) — both sent fire-and-forget, no new ack/reply invented for this. This ordering is safe, not just conventional: the worker's incoming-request dispatch (`myreq_handler`, `controller_connector.c:1563`) is a plain synchronous `switch` invoked one message at a time off its single long-lived client connection (`xcecli_ctx`) — confirmed by reading it, not assumed — so the two requests are not just sent in order, they are *handled* to completion in that same order on the worker side; no thread-pool fan-out is in the path (the worker's own `thread_pool_t` is for job-forking work, unrelated to request dispatch). The actual wait in this design belongs at the *end* of the sequence, not in the middle: the controller does not consider the worker's rejoin/reconciliation complete until the job-state *report* arrives (or the Requirement 5.3 timeout fires) — see Error Handling.
|
||||
- `src/controller/join.c` — **no change**. `handle_worker_join` itself is untouched; the reconciliation query is sent from the (extended) `on_worker_join`, which `handle_worker_join` already calls unconditionally after the join reply.
|
||||
- `src/controller/job.c` — **Correction (2026-09-01, bug-1747, task 3.2): three new functions, not two — see below.** `record_job_reconcile_pending_fact()` and `record_job_reconcile_assign_fact()` both follow `dispatch_a_job_`'s eager-mutation-plus-explicit-notify pattern (not `record_job_finish_fact`'s deferred one — see Components for why): `record_job_reconcile_pending_fact()` (Requirement 4.6, eagerly calls `apply_job_update_transition(job, JOB_STAT_PEND)` under the write lock, releases `job->job.allocs` — `free_JobAlloc_ptr(&j->job.allocs); j->job.allocs = NULL;` — on an applied (non-no-op) transition, durably appends via the existing generic `persist_submit_job_fact(&c->persist, WAL_JOB_UPDATE, jobid, JOB_STAT_PEND)`, then calls `persist_notify_job_updated_already_persisted(c, job)` after unlocking; this is bug-1631's own documented candidate fix, applied here because Requirement 4.6 is exactly the "RUN job re-admitted to the PEND pool" precondition bug-1631's fix was deferred pending), `record_job_reconcile_assign_fact()` (Requirement 4.8, eagerly sets `job->job.state`/host assignment under the write lock, calls the existing `persist_submit_job_dispatch_fact(c->persist, jobid, state, ts, alloc=NULL)` — reusing bug-1678's already-tested NULL-alloc decode path on both apply seams verbatim, not a new fact shape — then calls `persist_notify_job_updated_already_persisted(c, job)` after unlocking, same as above and for the same reason). Existing `record_job_finish_fact`/`record_job_fail_fact` were originally scoped to be called as-is for the finished-success/finished-failure cases; **found live during task 3.2 implementation that `record_job_finish_fact`'s specific call site has no way to carry the worker-reported finish time (Requirement 4.2) — its own primitive, `persist_submit_job_finish_fact`, accepts one, but `record_job_finish_fact` always derives `time(NULL)` instead.** Fixed with a THIRD new function, `record_job_finish_fact_at(cluster_t*, job_t*, xce_jobfinish_t*, uint64_t finish_time_ms)` — `record_job_finish_fact`'s own worker-reported-finish-time sibling, following `record_job_finish_fact`'s own DEFERRED pattern (durable append only, not eager like the other two new functions above), differing only in passing the caller-supplied (ms→s converted) time to `persist_submit_job_finish_fact` instead of `time(NULL)`; `record_job_finish_fact` itself stays unmodified, still used as-is for `on_job_finish`'s ordinary (non-reconciliation) receipt path. `record_job_fail_fact` is still called as-is for the finished-failure case — Requirement 4.2 remains genuinely unsatisfiable there via any existing path (see the Persistence Field-Completeness Review section's correction note; tracked as the FAIL half of bug-1747, deliberately deferred). Also: split `dispatch_a_job_`'s `w->ep == NULL || w->host.status != 0` check (job.c:1511-1512) into two branches, checked in order — `w->ep == NULL` is resolved directly via `record_job_reconcile_pending_fact()` with **no retry-counter increment and no `mark_job_fail()`**, *before* the code ever reaches `xce_classify_dispatch_fail()` (a deliberate bypass, not a third value on the shared `xce_dispatch_fail_class_t` enum — see Components); `w->host.status != 0` keeps its existing `XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY` behavior unchanged (now rarely hit for `XCE_WORKER_RECONCILING` specifically, since the scheduler-side filter below keeps it from being selected in the first place).
|
||||
- `src/controller/cluster.c` — new `case OPCode_jobStateReport:` in `myreq_handler`'s dispatch switch (runs after `ha_dispatch_gate`); route to the new reconciliation-apply logic.
|
||||
- `src/scheduler/src/scheduler.c` — extend the existing candidate-host filter (`scheduler.c:1211`, today `if (IS_WORKER_UNAVAIL(w->status)) { ... }`) to also skip a host with the new reconciling bit set — same shape, one added disjunct, no new call site. **Correction (2026-09-01, bug-1751, task 5): `scheduler.c:1211`'s `sched_reset_worker_()` is reached only via `sched_begin_session()`, which has zero live callers in `driver.c`/`sim_loop.c` today — both call `xce_core_begin_session()` (`src/scheduler/src/core_session.c`) directly, which runs an independently-maintained mirror, `core_reset_workers()` (`core_session.c:102`, its own header comment: "mirrors scheduler.c's sched_reset_workers/sched_reset_worker_"), not listed below. The identical disjunct is required in BOTH files for Requirement 5.1/5.2 to hold on the actual live path; `scheduler.c:1211`'s copy is applied for parity/dead-code-hygiene but is not itself exercised in production today.
|
||||
- `src/include/xce/types.h` — add `XCE_WORKER_RECONCILING` to the worker status bitmask plus an `IS_WORKER_RECONCILING(status)` macro (mirroring `IS_WORKER_UNAVAIL`'s existing shape) for the scheduler-side filter above; add a `resync_state_t` (queried job-ID set, round token, deadline) referenced from `worker_t`.
|
||||
- `src/lib/request.c` — `xce_job_state_query_req()`/`xce_job_state_report_req()` encode/decode helpers, following `xce_reply_get`/`xce_push_data_req`'s pagination loop shape.
|
||||
|
||||
### New Files
|
||||
- `src/controller/resync.c` (+ `src/controller/include/resync.h`) — per-worker reconciliation bookkeeping: arm/clear `XCE_WORKER_RECONCILING`, track queried job IDs and round token (Requirement 6.3's late-report discard), the delaymgr-triggered timeout (Requirement 5.3), and the glue that maps each `JobStateEntry` outcome to the correct `job.c` call.
|
||||
- `src/worker/src/xcecd_resync.c` (+ header) — handles incoming `OPCode_jobStateQuery`: looks up each queried job ID against the running list (`xcecd_worker_t`) and the new finished-job retention; builds and sends paginated `OPCode_jobStateReport` replies, followed by extra-job pages (Requirement 3.6).
|
||||
- `src/controller/testsuite/test_worker_resync.c` — controller-side unit tests (trigger, query construction, per-outcome fact application, dispatch gating, timeout, discard-on-disconnect), following `test_worker_join.c`'s fixture conventions.
|
||||
- `src/worker/testsuite/` — **new directory**; `src/worker` has no `testsuite/` today (per `structure.md`/`testing.md`). This feature is the first worker-side C test coverage; add `test_xcecd_resync.c` and the minimal harness scaffolding it needs (mirroring `controller/testsuite/test.c`'s runner pattern).
|
||||
|
||||
### Modified Worker Files
|
||||
- `src/worker/include/xcecd_worker_utils.h` / `src/worker/src/xcecd_worker_utils.c` — add a finished-job retention structure (jobid, exit status, real finish time, retain-until) populated at job exit *before* `cleanup_worker_list()` would otherwise reap the entry; `cleanup_worker_list()` retains an entry in this new structure instead of discarding all trace of it, until the configured window elapses.
|
||||
|
||||
---
|
||||
|
||||
## System Flows
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant W as Worker
|
||||
participant JC as ControllerJoin
|
||||
participant WC as ControllerWorker
|
||||
participant R as ControllerResync
|
||||
participant JOBC as ControllerJob
|
||||
participant SCHED as Scheduler
|
||||
|
||||
W->>JC: OPCode_join (reconnect or failover reconnect)
|
||||
JC->>WC: rejoin_worker(...)
|
||||
WC->>WC: clear UNAVAIL, set ep (per-worker WRITE lock)
|
||||
WC->>R: non-terminal jobs assigned to this host?
|
||||
alt none
|
||||
R-->>WC: no reconciliation needed
|
||||
else one or more
|
||||
R->>WC: arm XCE_WORKER_RECONCILING, init round token + deadline
|
||||
end
|
||||
WC-->>JC: idx
|
||||
JC->>JC: persist_commit_barrier (join durable)
|
||||
JC->>W: xce_reply (join success reply, unchanged)
|
||||
JC->>JC: on_worker_join
|
||||
JC->>W: OPCode_push (config push, unchanged worker_push_config call)
|
||||
JC->>W: OPCode_jobStateQuery (NEW — sent right after, same on_worker_join)
|
||||
Note over W: single connection, synchronous per-message dispatch<br/>(myreq_handler in controller_connector.c line 1563) — config push<br/>is handled to completion before the query is even read as<br/>a distinct message, no ack or reply needed to enforce this
|
||||
|
||||
loop each query page
|
||||
W->>JC: OPCode_jobStateReport (queried=true entries)
|
||||
end
|
||||
W->>JC: OPCode_jobStateReport (queried=false entries — extras, Req 3.6)
|
||||
Note over W: worker holds off entering its normal post-join operating<br/>mode (resuming periodic background reporting) until its full<br/>report has finished sending (Req 3.8) — a new gate flag,<br/>mirroring the existing needs_rejoin pattern in<br/>controller_connector.c, not a new blocking wait
|
||||
|
||||
loop each reported entry
|
||||
alt finished success
|
||||
JC->>JOBC: record_job_finish_fact (existing, reused)
|
||||
else finished failure
|
||||
JC->>JOBC: record_job_fail_fact (existing, reused)
|
||||
else still running
|
||||
JC->>R: clear this jobid from outstanding set (no-op fact)
|
||||
else unrecognized
|
||||
JC->>JOBC: record_job_reconcile_pending_fact (NEW) -- eager mutation + allocs release, durable append, then notify
|
||||
JOBC->>SCHED: persist_notify_job_updated_already_persisted (Req 7.2), same HOOK_DETAIL_JOB_UPDATE path dispatch_a_job_ already uses
|
||||
else unqueried extra job
|
||||
JC->>JOBC: record_job_reconcile_assign_fact (NEW) -- eager mutation, durable append, then notify
|
||||
JOBC->>SCHED: persist_notify_job_updated_already_persisted, same call dispatch_a_job_'s own success path already makes
|
||||
end
|
||||
Note over JOBC: finished success/failure stay fully deferred (record_job_finish_fact/<br/>record_job_fail_fact, unchanged) -- mutation + notify happen later via<br/>apply_committed_entry once the fact commits, same as any ordinary finish/fail.<br/>The two NEW functions above are eager instead, mirroring dispatch_a_job_'s<br/>established pattern, not this deferred one -- see Components for why.
|
||||
end
|
||||
|
||||
R->>WC: outstanding set empty? clear XCE_WORKER_RECONCILING
|
||||
Note over R: if deadline fires first (delaymgr trigger),<br/>resolve outstanding as answered/timed out (Req 5.3)<br/>rather than withholding dispatch indefinitely
|
||||
```
|
||||
|
||||
**Gating conditions**: Requirement 5.1's primary enforcement is one layer up from `dispatch_a_job_` — the scheduler's own candidate filter (`scheduler.c:1211`) excludes a `RECONCILING` host from consideration before a dispatch decision is ever made (see Components). `dispatch_a_job_`'s existing check (`w->host.status != 0`, job.c:1511) still refuses dispatch as a backstop for the rare decision-predates-the-bit race, and — unchanged — routes through the existing retry-then-terminal-fail classification the same as any other non-UNAVAIL status bit already does today. A dispatch attempt to a disconnected worker (`w->ep == NULL`) is handled separately and differently: it fails immediately and returns the job to pending (Requirement 5.4) via `record_job_reconcile_pending_fact`, checked and resolved *before* the status-bit check reaches `xce_classify_dispatch_fail()` at all — not the existing retry-then-terminal-fail branch (whose retry semantics — retry the dispatch itself — don't fit "the worker isn't even connected," and whose bounded-retry-then-`mark_job_fail()` outcome is the wrong terminal shape for a case Requirement 5.4 wants recoverable-via-reschedule instead). This split is a required code change to `dispatch_a_job_`, not a reuse — see Components.
|
||||
|
||||
**Discard-on-disconnect** (Requirement 6): if the worker disconnects before its report is fully processed, `rejoin_worker`'s next invocation (on the next join) finds the prior round's token stale and starts a fresh one; any report that later arrives carrying the stale token is discarded (Requirement 6.3) rather than applied against the wrong round.
|
||||
|
||||
---
|
||||
|
||||
## Requirements Traceability
|
||||
|
||||
| Requirement | Summary | Components | Interfaces | Flows |
|
||||
|---|---|---|---|---|
|
||||
| 1.1, 1.2, 1.3 | Trigger reconciliation iff non-terminal jobs exist for the host, regardless of join type | `rejoin_worker`, `resync.c` | trigger check | Sequence: rejoin → trigger |
|
||||
| 2.1, 2.2 | Query lists exactly the non-terminal jobs by unique ID | `resync.c`, wire `JobStateQuery` | `xce_job_state_query_req` | Sequence: OPCode_jobStateQuery |
|
||||
| 3.1–3.5 | Worker reports definitive status + real finish time per queried job | worker `xcecd_resync.c`, retention | `JobStateEntry` | Sequence: report loop |
|
||||
| 3.6 | Worker reports extra unqueried jobs | worker `xcecd_resync.c` | `JobStateEntry.queried=false` | Sequence: extras page |
|
||||
| 3.7 | Bounded, cluster-configurable reporting window | `ClusterConfig.workerResyncWindowMs`, worker retention | config field | Data Models |
|
||||
| 3.8 | Worker delays resuming normal post-join operation until its report has finished sending | worker `xcecd_resync.c`, new gate flag | resync-in-progress flag | Sequence: post-report note |
|
||||
| 4.1–4.5 | Apply via existing idempotent commit-then-apply; no-op duplicates; never clobber newer/terminal state | `job.c` (existing fns), `apply_job_*_transition` | existing | Sequence: apply per outcome |
|
||||
| 4.6 | Unrecognized → return to pending, eligible for scheduling, allocation released | `record_job_reconcile_pending_fact` (NEW, incl. `job->job.allocs` release — bug-1631) | new fn | Sequence: unrecognized branch |
|
||||
| 4.7, 4.8 | Reconcile jobs outside the query; assign if no existing non-terminal record | `record_job_reconcile_assign_fact` (NEW, calls `persist_submit_job_dispatch_fact(alloc=NULL)`) | new fn | Sequence: extras branch; Data Models limitation |
|
||||
| 5.1, 5.2 | Gate dispatch while reconciliation outstanding; restore eligibility on completion | `scheduler.c:1211` candidate filter (NEW disjunct, primary), `XCE_WORKER_RECONCILING` bit + `dispatch_a_job_` (backstop, reused) | status bitmask | Architecture: dispatch gating |
|
||||
| 5.3 | Bounded timeout resolves outstanding reconciliation | `resync.c` delaymgr trigger, `ClusterConfig.workerResyncTimeoutMs` | timeout | Sequence: deadline note |
|
||||
| 5.4 | Dispatch to disconnected worker fails fast, returns to pending | new pre-classifier `w->ep == NULL` branch in `dispatch_a_job_`, `record_job_reconcile_pending_fact` (NEW) | — | Error Handling |
|
||||
| 6.1–6.3 | Disconnect mid-round discards and re-triggers; idempotent re-application; stale-round reports discarded | `resync.c` round token | round token | Sequence: discard-on-disconnect note |
|
||||
| 7.1–7.3 | Reconciled changes reach the scheduler via the existing notify path | `XCE_CONTROLLER_HOOK_JOB`, `sched.c`, `handler.c` (all reused, unchanged) | `notify()` | Sequence: apply → notify |
|
||||
| 8.1–8.3 | Works for any job count; paginated multi-message exchange | `JobStateQuery`/`JobStateReport` pagination | `{pos,count,ntotal}` | Sequence: loop each query/report page |
|
||||
|
||||
---
|
||||
|
||||
## Components and Interfaces
|
||||
|
||||
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies | Contracts |
|
||||
|---|---|---|---|---|---|
|
||||
| `resync.c` (new) | Controller | Per-worker reconciliation bookkeeping + timeout | 1, 5.3, 6 | `worker.c` (P0), `commit.c` admission pattern (P1) | State, Batch |
|
||||
| `job.c` additions | Controller | Two new idempotent fact-recording functions + split disconnected-worker dispatch-fail site | 4.6, 4.8, 5.4 | existing commit-then-apply seam (P0), `xce_classify_dispatch_fail()` (P0) | Service |
|
||||
| `xcecd_resync.c` (new) | Worker | Answer job-state query from running list + retention | 3, 8 | `xcecd_worker_utils.c` (P0) | Batch, Event |
|
||||
| `xcecd_worker_utils.c` additions | Worker | Short-lived finished-job retention | 3.5, 3.7 | none new | State |
|
||||
| Wire opcode pair | Protocol | Paginated query/report exchange | 2, 3, 8 | `xce_reply_get`/`xce_push_data_req` pattern (P0) | API/Batch |
|
||||
| `scheduler.c` candidate-filter addition | Scheduler | Exclude a RECONCILING host from dispatch candidacy | 5.1, 5.2 | existing `IS_WORKER_UNAVAIL` filter (P0) | State |
|
||||
|
||||
### Controller: `resync.c`
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Own the reconciliation-outstanding lifecycle for one worker: arm on trigger, track queried IDs + round token, resolve on completion or timeout |
|
||||
| Requirements | 1.1–1.3, 5.1–5.3, 6.1–6.3 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Called from `rejoin_worker`/`apply_join_state` under the per-worker write lock already held there — must not introduce a new lock ordering.
|
||||
- Owns the round token that distinguishes the current reconciliation attempt from a stale/superseded one (Requirement 6.3).
|
||||
- Schedules the Requirement 5.3 timeout via a delaymgr one-shot trigger (non-blocking fire, per the async-I/O thread-role contract) — never a blocking wait on iomgr or delaymgr.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: `worker.c` (P0) — trigger call site
|
||||
- Outbound: `job.c` (P0) — per-outcome fact application
|
||||
- Outbound: `commit.c`'s admission-reservation pattern (P1) — template for any bounded wait, if a synchronous variant is ever needed instead of the pure delaymgr-trigger design above
|
||||
|
||||
**Contracts**: Batch [x] / State [x]
|
||||
|
||||
##### Batch / Job Contract
|
||||
- Trigger: `rejoin_worker` finds ≥1 non-terminal job for the rejoining host
|
||||
- Input / validation: the controller's own non-terminal job set for that host (Requirement 2.1) — no external input to validate
|
||||
- Output / destination: `job.c` fact-recording calls, one per reported entry
|
||||
- Idempotency & recovery: round token discards stale reports (6.3); a fresh round replaces a discarded one on the next join (6.1); applying a report twice for the same round is a no-op by construction of the underlying `job.c` guards (6.2)
|
||||
|
||||
##### State Management
|
||||
- State model: one `resync_state_t` per `worker_t` — `{round_token, queried_jobids[], deadline}`; absent/cleared when no reconciliation is outstanding
|
||||
- Persistence & consistency: **not itself persisted** — it is in-memory reconciliation-in-progress bookkeeping, not a durable fact; the durable facts it produces are the existing job-table WAL entries. A controller crash mid-reconciliation loses this bookkeeping, which is correct: the next rejoin (post-failover) re-triggers reconciliation from scratch (Requirement 1.3), so nothing needs to survive a crash here.
|
||||
- Concurrency strategy: guarded by the same per-worker write lock `rejoin_worker` already holds
|
||||
|
||||
### Controller: `job.c` — `record_job_reconcile_pending_fact`, `record_job_reconcile_assign_fact`
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | The two reconciliation outcomes with no existing idempotent fact analog, plus a required correction to a dormant retryable-dispatch-failure gap this feature reactivates |
|
||||
| Requirements | 4.6, 4.7, 4.8, 5.4 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
|
||||
Both new functions follow `dispatch_a_job_`'s own established pattern (job.c:1686-1748: eager, synchronous mutation under the write lock, immediately followed by a durable append, immediately followed by an explicit `persist_notify_job_updated_already_persisted(c, job)` call) — **not** `record_job_finish_fact`'s purely-deferred pattern (state mutation withheld entirely until `apply_committed_entry` applies the committed fact). This choice is deliberate, not copied without thought: `persist_notify_job_updated_already_persisted`'s own doc comment (`persist.h`) frames it generically ("e.g. `dispatch_a_job_()`'s own... call" — an example, not an exclusive binding) as the sanctioned way to notify for *any* caller whose "live, synchronous state... mutation" would otherwise leave `apply_committed_entry`'s later replay finding "state already matches" and silently skipping its own notify (bug-1679's exact failure mode). Both reconciliation outcomes share dispatch_a_job_'s justification for going eager rather than deferred: the fact being recorded is something the *worker* already confirmed really happened (or definitely didn't) — ground truth already changed outside this controller — not a same-node admin command whose commit could still be rolled back with nothing external yet depending on it (that second, riskier shape is `job_ctrl`'s `mark_job_update`, explicitly flagged in `roadmap.md`'s backlog as a legacy pattern to migrate *away* from, not a template to extend). Mixing an eager mutation for one field (e.g. `allocs`) with a deferred mutation for another (`state`) on the *same* transition was considered and rejected — it would create a real window where a job reads back as `RUN` with `allocs == NULL`, an inconsistent combination nothing else in this codebase produces; both fields move together, eagerly, under one lock hold.
|
||||
|
||||
- `record_job_reconcile_pending_fact(cluster_t*, job_t*)`: under the job's write lock, calls `apply_job_update_transition(job, JOB_STAT_PEND)` directly (not as a "peek" — as the real, live mutation) — this reuses the existing `job_state_is_backward` guard unchanged, returning early as a no-op if it reports `JOB_TRANSITION_REJECTED_BACKWARD` or `JOB_TRANSITION_NOOP_DUPLICATE` (Requirement 4.3/4.4). Only on `JOB_TRANSITION_APPLIED`: releases `job->job.allocs` (`free_JobAlloc_ptr(&job->job.allocs); job->job.allocs = NULL;`) if non-NULL, still under the same lock hold — this is required, not optional: `apply_job_update_transition` only ever touches `job->job.state` (job.c:2808-2834, confirmed by reading it) and never allocs, and bug-1631 (confirmed live, deliberately left unfixed) documents exactly this gap on `dispatch_a_job_`'s own retryable-failure paths, scoped as unreachable "only because nothing in the current system re-admits an already-RUN job back into the scheduler's PEND pool." Requirement 4.6 is precisely that re-admission path. Then, still under the lock: durably appends the fact via the existing generic `persist_submit_job_fact(&c->persist, WAL_JOB_UPDATE, jobid, JOB_STAT_PEND)` (the plain `{jobid, state}` shape is sufficient here — no extra fields, unlike DISPATCH/START/FINISH's richer siblings). After unlocking: calls `persist_notify_job_updated_already_persisted(c, job)`, reaching the scheduler through Requirement 7's existing notify path exactly as `dispatch_a_job_` already does for its own eager mutation. Also used for Requirement 5.4 (dispatch-to-disconnected-worker fail-fast) — see the `job.c` split below; the allocs release is a no-op there since a disconnected-worker dispatch attempt never committed one.
|
||||
- `record_job_reconcile_assign_fact(cluster_t*, job_t*, worker_t*, xce_job_state_entry_t*)`: under the job's write lock, eagerly sets `job->job.state` and the host assignment to match the worker's report (mirroring `dispatch_a_job_`'s own inline mutation at job.c:1686-1717, minus the allocation fields — there are none to set); `job->job.allocs` stays `NULL` (see Data Models — there genuinely is none to write). Then, still under the lock: calls the existing `persist_submit_job_dispatch_fact(&c->persist, jobid, state, ts, alloc)` (job.c:1731-1734, the exact primitive `dispatch_a_job_`'s own success path already calls) directly, with `alloc = NULL` — bug-1678's fix (2026-08-25) already made this primitive's NULL-alloc case an explicit, backward-compatible branch on both **apply/decode** seams; the **encode** side had a latent NULL-deref (never exercised until this task — bug-1746, fixed as part of task 2.2, a minimal guard in `capnp_encode_joballoc`), so this is a thin caller into existing code that this task also had to make genuinely NULL-safe, not a new WAL fact shape. After unlocking: calls `persist_notify_job_updated_already_persisted(c, job)`, same as above and for the same reason — without it, `apply_committed_entry`'s later replay of this exact `WAL_JOB_DISPATCH` entry finds state already matching and silently skips its own notify, reproducing bug-1679 at this new call site if omitted.
|
||||
- Split in `dispatch_a_job_` (job.c:1511-1512): `w->ep == NULL` is checked and resolved **before** the existing `w->host.status != 0` check reaches `xce_classify_dispatch_fail()` at all — calling `record_job_reconcile_pending_fact()` directly, with the job's `dispatch_fail_count` left untouched and `mark_job_fail()` never called, then returning. This is deliberately a bypass, not a third value added to `xce_dispatch_fail_class_t`: that enum's whole contract (per its own precedent, `commit-batch-window`'s requirements.md: "an exhaustive site-identity enum maps to a decision — TERMINAL vs RETRYABLE") is a two-way choice every other call site's `switch` already exhaustively handles; adding a third value would force each of those ~5 unrelated switches to grow a case they can never hit. `w->host.status != 0` (any bit, `XCE_WORKER_RECONCILING` included) is otherwise unchanged — still reaches `xce_classify_dispatch_fail(XCE_DISPATCH_FAIL_SITE_WORKER_NOT_READY)` and its existing retryable/terminal path, same as today, on the (now rare, since the scheduler-side filter keeps it from being the common case) chance a dispatch decision predates the bit being observed.
|
||||
- No function introduces a new *commit/idempotency* guard — all three reuse existing guarded seams (`apply_job_update_transition`'s `job_state_is_backward`, `persist_submit_job_fact`/`persist_submit_job_dispatch_fact`'s existing WAL append contracts) and the existing, already-proven-safe `persist_notify_job_updated_already_persisted` notify primitive. Nothing here is a new persistence *mechanism* — it is the same eager-mutation-plus-explicit-notify shape `dispatch_a_job_` already established, applied to two new call sites.
|
||||
|
||||
**Contracts**: Service [x]
|
||||
|
||||
##### Service Interface (C)
|
||||
```c
|
||||
int record_job_reconcile_pending_fact(cluster_t* c, job_t* job);
|
||||
int record_job_reconcile_assign_fact(cluster_t* c, job_t* job,
|
||||
worker_t* w, xce_job_state_entry_t* report);
|
||||
```
|
||||
- Preconditions: caller holds `job`'s write lock on entry (matching every other `record_job_*_fact` sibling); `job` is non-terminal (pending case) or has no existing non-terminal assignment (assign case)
|
||||
- Postconditions: unlike `record_job_finish_fact`/`record_job_fail_fact` (deferred — mutation happens later via `apply_committed_entry`), both of these functions mutate `job`'s live in-memory state **eagerly, before returning** — matching `dispatch_a_job_`'s established pattern, not the deferred one — then durably append the fact, then (after the caller releases the lock) notify via `persist_notify_job_updated_already_persisted`. `record_job_reconcile_pending_fact` additionally guarantees `job->job.allocs == NULL` on return whenever it was non-NULL on entry, mutated atomically alongside the state change under the same lock hold (in-memory only, not itself a WAL field — see Data Models).
|
||||
- Invariants: never called for a job the controller already knows is terminal or more-final (Requirement 4.4/4.5 — enforced by the existing `job_state_is_backward` guard, not by new logic here); the caller must call `persist_notify_job_updated_already_persisted(c, job)` after unlocking, exactly once, for every actual (non-no-op) mutation — omitting it reproduces bug-1679 at this call site.
|
||||
|
||||
### Worker: `xcecd_resync.c` (new)
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Answer the controller's job-state query from the running list and the new finished-job retention; report extras; gate the worker's own post-join wake-up on completion |
|
||||
| Requirements | 3.1–3.8, 8.2 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- For each queried job ID: check the running list (`xcecd_worker_t`) first; if not running, check the new retention; if in neither, report `unrecognized` (Requirement 3.1, 3.3). Every job ID present in the query gets an entry in the reply — none are silently skipped (Requirement 3.2).
|
||||
- Include the real finish time from retention, not the time the report is sent (Requirement 3.5).
|
||||
- After answering all queried IDs, walk the running list and retention for anything not in the query and report it too (Requirement 3.6), using the same `JobStateEntry` shape with `queried=false`.
|
||||
- Send both in paginated `OPCode_jobStateReport` messages (Requirement 8.2).
|
||||
- Sets a new local `resync_pending` flag (worker-side, mirroring `controller_connector.c`'s existing `needs_rejoin` boolean pattern) when `OPCode_jobStateQuery` is received; clears it only once every report page (queried and extra) has finished sending. Whatever the worker's own post-join sequence does next (resuming periodic background reporting, entering normal operating mode) checks this flag first (Requirement 3.8). This is a local gate on the worker's own follow-on behavior, not a new wire-level ack and not an extension of `xce_join()`'s blocking wait (see Boundary Commitments' Out of Boundary).
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: query receipt (P0)
|
||||
- Outbound: `xcecd_worker_utils.c`'s running list + new retention (P0)
|
||||
|
||||
**Contracts**: Batch [x] / Event [x]
|
||||
|
||||
##### Batch / Job Contract
|
||||
- Trigger: `OPCode_jobStateQuery` received
|
||||
- Input / validation: job IDs in the query — no validation needed beyond matching against local state
|
||||
- Output / destination: `OPCode_jobStateReport`, one or more pages
|
||||
- Idempotency & recovery: re-sending the same query (e.g. after the worker's own reconnect mid-exchange) produces the same answers — the worker's local state is the read-only source of truth for this exchange
|
||||
|
||||
### Worker: `xcecd_worker_utils.c` additions — finished-job retention
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Retain a job's true outcome (status, exit code, real finish time) for a bounded window after exit, instead of discarding it at the next `cleanup_worker_list()` pass |
|
||||
| Requirements | 3.4, 3.5, 3.7 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Populated at the same point a job's exit is currently detected, before `cleanup_worker_list()` would otherwise delete the entry.
|
||||
- Bounded by `ClusterConfig.workerResyncWindowMs` (Requirement 3.7) — entries older than the window are dropped, not retained indefinitely.
|
||||
- Does not change `cleanup_worker_list()`'s existing behavior for the in-flight `xcecd_worker_t`/sqlite bookkeeping — it adds a second, smaller, timestamped structure fed at the same moment, not a rework of the existing one.
|
||||
|
||||
**Contracts**: State [x]
|
||||
|
||||
##### State Management
|
||||
- State model: a small bounded list/ring of `{jobid, exit_status, exit_code, finish_time_ms}`, in-memory only
|
||||
- Persistence & consistency: **not persisted** — a worker restart loses this retention, which is acceptable: a restarted worker has no running jobs of its own to report on rejoin either way (its child processes died with it), so the retention's only job is to survive a *controller-side* disconnect/reconnect window, not a worker restart
|
||||
- Concurrency strategy: same locking discipline already used for `xcecd_worker_t`'s list
|
||||
|
||||
### Scheduler: `scheduler.c` candidate-filter addition
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Keep a `RECONCILING` host out of dispatch candidacy at the source, so Requirement 5.1 is enforced without depending on the controller-side retry/terminal-fail machinery for the common case |
|
||||
| Requirements | 5.1, 5.2 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- One added disjunct to the existing check at `scheduler.c:1211` — today `if (IS_WORKER_UNAVAIL(w->status)) { ... }`, becomes `if (IS_WORKER_UNAVAIL(w->status) || IS_WORKER_RECONCILING(w->status)) { ... }` (new macro, `types.h`, same shape as the existing `IS_WORKER_UNAVAIL`/`IS_WORKER_LOCKEDU`/etc. family).
|
||||
- Reads `w->status`, a field the scheduler already receives via the existing worker-status mirror (`core_events.c:527`/`:595`) — no new wire content, no new push.
|
||||
- Does **not** touch `scheduler.c`/`driver.c`'s outbound decision-*publish* logic or introduce any reconciliation-protocol concept into either file — see Architecture Integration for why this stays clear of `test_no_worker_resync_symbols_in_scheduler_publish_path`'s fence.
|
||||
- Requirement 5.2 ("eligible for new job dispatch again" once reconciliation completes) needs no separate scheduler-side action: once the controller clears `XCE_WORKER_RECONCILING` and that status change is next mirrored to the scheduler, this same filter simply stops excluding the host.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: the existing worker-status mirror (P0) — no new dependency introduced
|
||||
|
||||
**Contracts**: State [x]
|
||||
|
||||
##### State Management
|
||||
- State model: none new — reads the existing `xce_sched_worker_t.status` mirror field
|
||||
- Persistence & consistency: not applicable — a read of already-existing, already-synced state
|
||||
- Concurrency strategy: unchanged — same read path as the existing `IS_WORKER_UNAVAIL` check it sits beside
|
||||
|
||||
---
|
||||
|
||||
## Data Models
|
||||
|
||||
### Wire/Protocol Model
|
||||
|
||||
```
|
||||
JobStateEntry {
|
||||
jobid: UInt64
|
||||
status: JobStateStatus (RUNNING | FINISHED_OK | FINISHED_FAIL | UNRECOGNIZED)
|
||||
exitCode: Int32 # meaningful only for FINISHED_FAIL
|
||||
finishTimeMs: UInt64 # meaningful only for FINISHED_OK/FINISHED_FAIL; worker-reported, per Req 3.5
|
||||
queried: Bool # false = Requirement 3.6 extra, not part of the controller's original query
|
||||
}
|
||||
|
||||
JobStateQuery { jobids: List(UInt64), pos: UInt32, count: UInt32, ntotal: UInt32 }
|
||||
JobStateReport { entries: List(JobStateEntry), pos: UInt32, count: UInt32, ntotal: UInt32 }
|
||||
```
|
||||
|
||||
### Controller-Side In-Memory Model
|
||||
|
||||
```
|
||||
resync_state_t { // new, attached to worker_t
|
||||
round_token: uint64 // discards stale/superseded reports (Req 6.3)
|
||||
queried_jobids: set<uint64>
|
||||
deadline: timestamp // Req 5.3
|
||||
}
|
||||
```
|
||||
|
||||
### Persistence Field-Completeness Review
|
||||
|
||||
Per `design-principles.md`'s Persistence Review requirement and `persistence.md`'s `APPLY_DIVERGENT`/bug-1678 precedent, every fact this feature introduces or reuses is enumerated below:
|
||||
|
||||
| Outcome | Fact path | New fields? | Both apply seams? |
|
||||
|---|---|---|---|
|
||||
| Finished success | `record_job_finish_fact_at` (NEW, task 3.2/bug-1747) | None — reuses `persist_submit_job_finish_fact`'s existing, already-complete `WAL_JOB_FINISH` payload shape verbatim (`finish_time`, exit info); the reconciliation caller passes the worker-reported `finishTimeMs` (Requirement 3.5, converted ms→s) as that fact's finish time, never the time the controller processed the report (Requirement 4.2) | Already covered by existing `apply_committed_entry`/standby-catch-up handling — unchanged, since the wire payload shape is identical to `record_job_finish_fact`'s own |
|
||||
| Finished failure | `record_job_fail_fact` (existing, reused as-is) | None — same Requirement 4.2 worker-reported-time rule applies | Unchanged, existing |
|
||||
| Still running | No fact — pure no-op against the outstanding set | N/A | N/A |
|
||||
| Unrecognized → pending | `persist_submit_job_fact(..., WAL_JOB_UPDATE, jobid, JOB_STAT_PEND)` (existing generic primitive, new call site) | None — the plain `{jobid, state}` shape is sufficient, same as every other `WAL_JOB_UPDATE` caller. **`job->job.allocs` release is in-memory only, eager, not itself a persisted field** — mutated live under the same lock as the state change (mirroring `dispatch_a_job_`'s own success path in reverse; see Components), then notified via `persist_notify_job_updated_already_persisted`, not left for `apply_committed_entry` to discover | Both seams already decode plain `WAL_JOB_UPDATE` identically today (it is not a new payload shape) — a replay of this fact re-derives `job->job.state = JOB_STAT_PEND` correctly; it does **not** re-derive the allocs release (that mutation is not wire content), but since `dispatch_a_job_`'s eager pattern already established this exact asymmetry (live mutation + explicit notify now, replay-idempotent state field only, no replay-time side effect needed for a purely in-memory cleanup), this is consistent with existing precedent, not a new gap |
|
||||
| Unqueried job → assign | `persist_submit_job_dispatch_fact(..., alloc=NULL)` (existing primitive, new call site, `record_job_reconcile_assign_fact`) | None — reuses `persist_submit_job_dispatch_fact`'s existing `WAL_JOB_DISPATCH` payload shape verbatim with `alloc=NULL`, exercising the same NULL-alloc decode branch bug-1678 already added. Live mutation is eager (see Components), matching `dispatch_a_job_`'s own pattern for the same fact type | Decode side already covered by bug-1678 on both seams. **Encode side required a fix** (bug-1746, task 2.2): `capnp_encode_joballoc` unconditionally dereferenced a NULL `alloc` — this was this codebase's first genuinely-NULL caller. Fixed with a minimal zero-length-buffer guard, matching the decode side's `alloc_len > 0` contract exactly; no new decode path needed once the encode side actually produces the zero-length payload the decode side already expected |
|
||||
|
||||
**Correction (2026-09-01, found live during task 3.2 implementation, bug-1747) — a THIRD new function, not two:** this section's original "Finished success" row asserted `record_job_finish_fact` could be reused "as-is," citing an "existing complete fact shape (already carries `finish_time`)" — accurate about `persist_submit_job_finish_fact` (the underlying primitive, which does accept an explicit `finish_time` parameter) but not about `record_job_finish_fact`'s own specific call site, which never exposed that parameter to a caller and always derives `time(NULL)` at receipt time instead. Calling `record_job_finish_fact` unmodified therefore could NOT satisfy Requirement 4.2 for the finished-success case — confirmed live when task 3.2 wired the reconciliation caller and found no way to pass the worker-reported time through. Fixed (coordinator-directed, mirroring bug-1746's own precedent for an in-boundary companion fix) by adding `record_job_finish_fact_at(cluster_t*, job_t*, xce_jobfinish_t*, uint64_t finish_time_ms)` (job.c) — `record_job_finish_fact`'s own worker-reported-finish-time sibling: an identical guard/hook-fire/durable-append body, differing only in passing the caller-supplied (ms→s converted) time to `persist_submit_job_finish_fact` instead of `time(NULL)`. `record_job_finish_fact` itself is left UNMODIFIED — its own caller (`on_job_finish`, an ordinary near-instantaneous worker report) is correctly served by receipt-time there. Unlike `record_job_reconcile_pending_fact`/`record_job_reconcile_assign_fact` (both EAGER, `dispatch_a_job_`-style), `record_job_finish_fact_at` follows `record_job_finish_fact`'s own DEFERRED pattern (durable append only; the in-memory mutation and scheduler notify happen later via `apply_committed_entry`'s replay, exactly as an ordinary finish already works) — so this section's job.c fact-recording count is **three new functions**, not two (see also the Architecture Pattern & Boundary Map bullet and the Modified Files entry for `job.c`, both correspondingly updated). The sibling "Finished failure" outcome remains genuinely unfixable via any existing path: `WAL_JOB_FAIL`'s wire payload carries no finish-time slot at any layer, for any caller in this codebase, and `apply_job_fail_transition` unconditionally stamps `time(NULL)` at apply time — a pre-existing, systemic gap that predates this feature entirely (bug-1747, re-scoped 2026-09-01), deliberately left deferred rather than patched narrowly inside this task.
|
||||
|
||||
**Documented scope decision — `job->allocs` on a reconciliation-assigned job (Requirement 4.8):** when the controller reconciles a job it never held any prior record of (e.g. it was dispatched by a controller instance before a failover whose dispatch fact was itself lost), it has no way to learn what resource allocation the scheduler originally computed for that job — the worker's report carries the job's execution outcome, not its original resource request. This design deliberately does not attempt to reconstruct `job->allocs` for this case. **Correction (2026-09-01, re-validated against current code):** an earlier draft of this section cited `persistence.md`'s `APPLY_DIVERGENT` section / bug-1678 as evidence this is "a tracked, separate gap" `WAL_JOB_DISPATCH` still has — that citation was stale. Bug-1678 was fixed 2026-08-25 (before this design was finalized): `WAL_JOB_DISPATCH` now durably carries `job->allocs` when one exists, on both apply/decode seams. The real reason this design still writes no allocation for Requirement 4.8 is simpler and unrelated to that gap: there just isn't one to write — the scheduler, not this controller, is the only party that ever computes a job's allocation, and for a job this controller never had any record of, no dispatch (successful or otherwise) ever ran through this controller to produce one. `record_job_reconcile_assign_fact` reuses the *fixed* `persist_submit_job_dispatch_fact` primitive because its NULL-alloc *decode* case is already the correct representation of "state and host known, allocation not" — though the *encode* side of that same NULL case had never actually been exercised before this feature (bug-1746: `capnp_encode_joballoc` dereferenced a NULL `alloc` unconditionally, since `dispatch_a_job_`, bug-1678's only prior caller, always passes a real allocation). Task 2.2 fixed the encode side with a minimal zero-length-buffer guard as part of making this call safe to actually make — a real, if narrow, gap in the "already-tested" characterization this design originally gave the primitive, closed as part of implementation rather than left latent. The practical consequence is bounded by `resource-accounting.md`'s existing policy: a job with an unknown/zero reservation is charged `max(reserved=0, actual)` — i.e., its real usage — the moment its next rusage tick arrives, identical to how a legitimately-unreserved job is already handled today. The job's *state* is correct immediately; its resource *accounting* converges within one rusage cycle, not instantly. No fabricated allocation value is ever written.
|
||||
|
||||
### Configuration Model
|
||||
|
||||
Per `config-design.md`'s classification rules, applied explicitly (not left implicit):
|
||||
|
||||
| Field | Rule 0 (bootstrap) | Independence | Frequency | Verdict | Why |
|
||||
|---|---|---|---|---|---|
|
||||
| `workerResyncWindowMs` (Req 3.7) | passes — not needed to obtain config | passes — one worker's reporting window has no cross-node correctness dependency | passes — rarely changed | **Shared, via operational-uniformity override** | Structurally identical to `heartbeatInterval`/`inactiveHeartbeatCount` (controller→worker liveness domain): no correctness invariant demands agreement, but operators want uniform, predictable reconciliation behavior across the cluster rather than per-node drift — same documented exception, same rationale |
|
||||
| `workerResyncTimeoutMs` (Req 5.3) | passes | passes — one worker's reconciliation deadline has no cross-node dependency | passes | **Shared, via operational-uniformity override** | Same reasoning as above; kept alongside `workerResyncWindowMs` in `ClusterConfig` rather than splitting the override rationale across two places |
|
||||
|
||||
Both fields are added to the existing `ClusterConfig` capnp struct, distributed via the existing shared-config push mechanism — no new configuration surface or distribution channel.
|
||||
|
||||
---
|
||||
|
||||
## Error Handling
|
||||
|
||||
### Error Strategy
|
||||
Every reconciliation outcome resolves definitively — there is no "leave it ambiguous" outcome. A worker that cannot determine a job's true status reports `UNRECOGNIZED` rather than guessing (Requirement 3.3), which the controller treats as "return to pending," never as a fabricated finish.
|
||||
|
||||
### Error Categories and Responses
|
||||
- **Worker disconnects mid-round** (Requirement 6.1): the outstanding round is discarded (not applied partially); the next join re-triggers a fresh round via the same Requirement 1 check.
|
||||
- **Stale/superseded report arrives** (Requirement 6.3): discarded by round-token mismatch in `resync.c` — never applied against a round that's already been superseded.
|
||||
- **Reconciliation report never arrives / no reply within the bound** (Requirement 5.3): a delaymgr-scheduled trigger (non-blocking fire, per the async-I/O thread-role contract) resolves the outstanding round — accepted-as-answered, timed out, or left for the next rejoin to retry — rather than withholding that worker from dispatch indefinitely. This must not be a blocking wait anywhere in the iomgr/delaymgr call chain; if a synchronous wait style is preferred at implementation time instead of a pure trigger-and-resolve design, it must follow `commit.c`'s `COMMIT_WAIT_RESERVED` admission-reservation pattern (bounded blocking under an explicit cap on the vtask pool), never a raw wait.
|
||||
- **Dispatch attempted to a disconnected worker** (Requirement 5.4): fails immediately (not queued for later delivery) and returns the job to pending via `record_job_reconcile_pending_fact` — checked and resolved directly in `dispatch_a_job_`, ahead of and separate from the `w->host.status != 0`/`xce_classify_dispatch_fail()` path, not a new value bolted onto that shared TERMINAL/RETRYABLE enum: retrying the same dispatch attempt against a worker that isn't even connected doesn't fit either existing outcome, and unlike a TERMINAL resolution this one must NOT call `mark_job_fail()` — it must leave the job schedulable again, not fail it.
|
||||
- **Dispatch attempted to a `RECONCILING` worker** (Requirement 5.1): primarily prevented at the source — `scheduler.c`'s candidate filter (extended, see Components) excludes a `RECONCILING` host, so the scheduler does not produce a dispatch decision for it in the first place. If one is produced anyway (a decision-predates-the-bit race), `dispatch_a_job_`'s unchanged `w->host.status != 0` check still refuses it, falling into the existing `WORKER_NOT_READY` retry-then-terminal-fail path — the same, unmodified behavior every other non-UNAVAIL status bit (`LOCKEDU`/`LOCKEDW`/`BUSY`/`CLOSED`) already produces today. This design does not change that behavior for the race case; it only makes the race rare by filtering earlier.
|
||||
- **A job reconciled from RUN back to PEND is later re-dispatched and fails retryably** (Requirement 4.6, bug-1631): confirmed as bug-1631's own deliberately-deferred trigger condition — a RUN job with a real `job->job.allocs` returned to PEND and re-admitted to the scheduler's dispatch pool, something nothing in the system did before this feature. `record_job_reconcile_pending_fact` closes this by releasing `job->job.allocs` at the same point it sets the job PEND (see Components), so a subsequent redispatch attempt — successful or retryably-failing — never observes a stale allocation left over from the job's prior dispatch. This must ship together with Requirement 4.6, not be left as a follow-on: shipping 4.6 without it would take bug-1631 from "deliberately deferred, unreachable" to "live, reachable, and unfixed."
|
||||
- **New opcode reaches a non-primary controller**: per `api-standards.md`'s "Adding a New Opcode" checklist (bug-1700 precedent) — verify at implementation time whether `OPCode_jobStateQuery`/`OPCode_jobStateReport` need an explicit redirect-reply path. Expected answer: no, since a worker only ever holds a connection to the current primary (connection-lifecycle-tied-to-primary-status, per the HA connection-ownership model), so this opcode pair should never reach a standby in practice — but this must be confirmed, not assumed, given bug-1700's documented history of this exact class of gap recurring across two other opcodes.
|
||||
|
||||
### Monitoring
|
||||
Reuses existing job-state-change logging (job.c's existing log call sites for finish/fail/update facts) — no new logging framework. A reconciliation-driven fact should log which outcome and round produced it, for operator diagnosis of a rejoin event, matching the existing style (structured, operator-visible on the ERROR/WARN path per `error-handling.md`; INFO for normal reconciliation completion).
|
||||
|
||||
---
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Unit Tests (Controller, `src/controller/testsuite/test_worker_resync.c`)
|
||||
- Reconciliation triggers iff the host has ≥1 non-terminal job at rejoin (Requirement 1.1/1.2), for all three join circumstances (first join, network reconnect, failover reconnect) without the code needing to distinguish them (Requirement 1.3).
|
||||
- Query construction lists exactly the controller's non-terminal jobs for that host by ID (Requirement 2).
|
||||
- Each of the five reported outcomes (running, finished-success, finished-failure, unrecognized, unqueried-extra) applies through the correct existing/new fact function exactly once, and a repeated report for the same round is a no-op (Requirement 4, mirroring `test_guard_replay_finish_equals_single_application`'s existing style).
|
||||
- `XCE_WORKER_RECONCILING` still blocks `dispatch_a_job_` as a backstop (unchanged existing behavior) and clears once the round resolves (Requirement 5.1/5.2).
|
||||
- `record_job_reconcile_pending_fact` releases a non-NULL `job->job.allocs` when reconciling a RUN job back to PEND, and leaves it untouched (already NULL) for a job that never held one — regression coverage for bug-1631's exact deferred trigger condition: dispatch (real allocs committed) → reconcile-to-pending via an `unrecognized` report → assert `job->job.allocs == NULL` and `job->job.state == JOB_STAT_PEND` **immediately after the call returns, before any commit round-trip** (the mutation is eager, not deferred — see Components) → redispatch → a retryable dispatch failure on that redispatch leaves no stale allocation and no incorrect state (Requirement 4.6).
|
||||
- Both `record_job_reconcile_pending_fact` and `record_job_reconcile_assign_fact` call `persist_notify_job_updated_already_persisted` exactly once per actual (non-no-op) mutation, reaching the scheduler's/apiserver's notify path immediately — not only after a later `apply_committed_entry` replay finds a no-op and silently skips its own notify (bug-1679's exact failure mode, reactivated at these two new call sites if this assertion is missing). Assert the hook fires synchronously within the call, not merely "eventually."
|
||||
- `record_job_reconcile_assign_fact` round-trips through both apply seams (`apply_committed_entry`, `apply_wal_entry`) via `persist_submit_job_dispatch_fact`'s existing NULL-alloc path — assert a real WAL entry is written and a **restart replay** (a separate scenario from the live eager-mutation path above) reconstructs jobid/host/state with `allocs == NULL` and correctly no-ops its own hook fire (state already matches from the eager mutation, or from the fact's own replay if this happens after a crash before the eager mutation's process ever ran again) — not a new decode path (Requirement 4.8).
|
||||
- `dispatch_a_job_`'s new pre-classifier `w->ep == NULL` branch: dispatch to a disconnected worker fails immediately, does not increment `dispatch_fail_count`, does not call `mark_job_fail`, never reaches `xce_classify_dispatch_fail()`, and returns the job to PEND via `record_job_reconcile_pending_fact` (Requirement 5.4) — asserted separately from the unchanged `w->host.status != 0`/`WORKER_NOT_READY` case, which keeps its existing retry-then-terminal-fail behavior verbatim.
|
||||
- Timeout resolves an outstanding round without a blocking call (Requirement 5.3).
|
||||
- Disconnect mid-round discards and re-triggers; a stale-round report is discarded (Requirement 6).
|
||||
- A reconciled state change reaches the scheduler via the existing notify path, asserted the same way existing finish/fail notify tests assert it (Requirement 7).
|
||||
|
||||
### Unit Tests (Scheduler, existing `scheduler.c` candidate-selection test coverage)
|
||||
- A host with `XCE_WORKER_RECONCILING` set is excluded from dispatch candidacy the same way an `IS_WORKER_UNAVAIL` host already is (Requirement 5.1); a host with the bit cleared is eligible again (Requirement 5.2). Added alongside the existing `IS_WORKER_UNAVAIL` candidate-filter test, not a new test file.
|
||||
- Confirm the new check introduces no string/symbol named `resync` into `scheduler.c`/`driver.c` — `test_no_worker_resync_symbols_in_scheduler_publish_path` (`test_scheduler_decision_redelivery.c:432`) must stay green unmodified.
|
||||
|
||||
### Unit Tests (Worker, new `src/worker/testsuite/test_xcecd_resync.c`)
|
||||
- Query answered correctly from the running list, from retention, and as `UNRECOGNIZED` when in neither (Requirement 3.1–3.3).
|
||||
- Real finish time reported, not send time (Requirement 3.5).
|
||||
- Extras reported for jobs outside the query (Requirement 3.6).
|
||||
- Retention window boundary: an entry just inside/outside `workerResyncWindowMs` is/isn't reported (Requirement 3.7).
|
||||
- `resync_pending` gate: set on receiving `OPCode_jobStateQuery`, stays set across multiple report pages, and only clears after the last page (queried and extra) has been sent — the worker's post-join wake-up path must observe it set throughout, never cleared early (Requirement 3.8).
|
||||
|
||||
### Integration Tests
|
||||
- Full rejoin-reconciliation round trip against a live controller+worker pair, mirroring `test_worker_join.c`'s fixture conventions (`setup_test_cluster`/`teardown_test_cluster`, direct calls, one test per behavior).
|
||||
- Failover-during-job: a job's finish notify is lost across a primary failover; the new primary's rejoin reconciliation recovers it (this is the feature's core bug-1741 regression scenario).
|
||||
- Burst/array scenario resembling bug-1741's original reproduction (many near-simultaneous finish reports lost on one host) — resync must recover all of them, unbounded by count (Requirement 8).
|
||||
|
||||
### Performance/Load
|
||||
- Reconciliation exchange for a worker running an order of magnitude more jobs than a single message can carry completes correctly across multiple pages, not truncated (Requirement 8.1–8.3).
|
||||
|
||||
---
|
||||
|
||||
## Security Considerations
|
||||
No new authentication/authorization surface — the new opcode pair travels over the same authenticated controller↔worker connection as every other daemon message, subject to the same `ha_dispatch_gate` role-based admission already governing every opcode. No new external input validation beyond matching reported job IDs against the controller's own records (an unrecognized ID is simply itself information, per Requirement 3.3/4.5 — never treated as an error condition to reject).
|
||||
|
||||
## Supporting References
|
||||
Deep file:line citations, the alternatives considered for the wire-protocol shape, and the full rationale for each design decision above live in `research.md`.
|
||||
98
.kiro/specs/worker-rejoin-resync/requirements.md
Normal file
98
.kiro/specs/worker-rejoin-resync/requirements.md
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
# Requirements Document
|
||||
|
||||
## Introduction
|
||||
|
||||
Today, a worker's job start/finish/fail report to the controller is a single fire-and-forget send (`XCECLI_FF_CALL`, `src/lib/client_internal.h`) with no retry and no redelivery. If that send is lost — because the worker briefly disconnects, or because the controller it was talking to fails over to a new primary mid-flight — the fact is gone forever: the job's true outcome is never recorded, and the job's state in the cluster stays wherever it last was (typically `RUN`) permanently, even though the worker's own OS process already exited. This was confirmed live (bug-1741): a burst of near-simultaneous job completions on one worker host lost 6 of 7 finish reports, leaving those jobs stuck in `RUN` forever while their siblings all completed normally, and no controller resync ever corrected them because nothing in the cluster still knew they had actually finished.
|
||||
|
||||
This feature closes that gap for the specific case of a worker reconnecting to the controller (after a network disconnect, or after the controller it was talking to fails over and the worker rejoins the new primary): on every such rejoin, the controller asks the worker directly what really happened to the jobs the controller believes are running there, and reconciles the worker's answer into its own job records through the existing idempotent commit-then-apply path. This does not attempt to prevent the underlying send loss (a separate concern); it recovers from it after the fact, the next time the worker and controller re-establish contact.
|
||||
|
||||
## Boundary Context
|
||||
|
||||
- **In scope**: A worker reconnecting to the controller (network disconnect + reconnect, or reconnecting to a new primary after controller failover) triggers a job-state reconciliation exchange, initiated by the controller, covering jobs the controller currently believes are running on that specific worker host. The controller applies the worker's answer through the existing idempotent commit-then-apply path, with no duplicate facts and no clobbering of state the controller already knows is newer or more final.
|
||||
- **Out of scope**: Preventing the original notification loss (the fire-and-forget send itself is unchanged). Resyncing anything other than job execution facts on worker rejoin — apiserver rejoin content, scheduler rejoin content, and any other cluster/queue/job *list* state resync are separate, deferred concerns and not covered here. Any change to the controller's existing pending-facts queue (which already retries facts it received but could not commit while the worker stayed connected) — this feature complements that mechanism for the case where the fact never arrived at all, it does not modify it. Delta-based or session-token re-attach optimizations for rejoin in general. Coupling the recently-finished reporting window (Requirement 3, Acceptance Criterion 7) to commit-batch-window's commit-urgency tuning — the window's sizing is driven by disconnect/reconnect coverage, and any mitigation it happens to provide for batched-but-uncommitted facts is incidental, not a designed dependency; commit-batch-window remains a separately parked feature.
|
||||
- **Adjacent expectations**: This feature depends on the controller's existing commit-then-apply idempotency guards (already in place) to apply reconciled facts safely. It does not introduce a second, competing mechanism for deciding a job's state — reconciled facts are applied as ordinary job-state facts through that same path.
|
||||
- **Multi-host jobs**: the Controller dispatches and tracks a multi-host job as a single record owned by that job's head/primary host — it never dispatches separately to the job's other hosts. Job-state reconciliation for a multi-host job therefore happens through that same head host, consistent with how the job is already dispatched and tracked; no separate handling is needed for the job's other hosts. This is unrelated to bug-1741's own documented gap (per-host resource-usage/rusage attribution for a multi-host job's individual tasks), which this feature does not touch.
|
||||
|
||||
## Requirements
|
||||
|
||||
### Requirement 1: Reconciliation Trigger
|
||||
|
||||
**Objective:** As the Controller, I want to start job-state reconciliation whenever a joining worker host has jobs I still believe are running on it, so that I don't need to reliably distinguish a worker's first-ever join from a reconnect.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When a worker host joins the cluster and the Controller currently records one or more non-terminal jobs assigned to that host, the Controller shall initiate job-state reconciliation for that worker as part of that join.
|
||||
2. When a worker host joins the cluster and the Controller currently records no non-terminal jobs assigned to that host, the Controller shall not initiate job-state reconciliation for that worker.
|
||||
3. The Controller shall apply this same trigger condition regardless of whether the join is the host's first-ever registration, a reconnect after a network disconnect, or a reconnect to a new primary after controller failover — the Controller shall not need to separately determine which of these a given join is.
|
||||
|
||||
### Requirement 2: Controller-Initiated Job-State Query
|
||||
|
||||
**Objective:** As the Controller, I want to tell a joining worker exactly which jobs I currently believe are running on it, so that the worker only needs to answer about jobs I have an open question about.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When job-state reconciliation is initiated for a worker (Requirement 1), the Controller shall send that worker a list of the jobs the Controller currently records as assigned to that worker host and not yet in a terminal state.
|
||||
2. The Controller shall identify each job in the query unambiguously (by the job's unique identifier) so the worker can match it to its own local job records.
|
||||
|
||||
### Requirement 3: Worker Job-Status Report
|
||||
|
||||
**Objective:** As the Controller, I want the worker to tell me the real, current outcome of every job I asked about, so that I can correct any job-state fact I lost while we were disconnected.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When a worker receives a job-state query from the Controller, the Worker shall report back, for every job identifier in the query, one of: still running, finished successfully, finished with a failure/exit code, or unrecognized (the worker has no record of ever running that job).
|
||||
2. The Worker shall report a status for every job identifier present in the query it received.
|
||||
3. If the worker cannot determine a job's true status (for example, the job's process state is inconclusive at the time of the query), the Worker shall report that job as unrecognized/indeterminate rather than guessing a definite outcome.
|
||||
4. If a job in the query has already finished before the rejoin completes, the Worker shall report the same finish outcome (success/failure and exit code) it would have reported through the ordinary job-finish notification.
|
||||
5. When the Worker reports a job as finished (successfully or with a failure), the Worker shall include the time the job actually finished, not the time the report itself is sent.
|
||||
6. The Worker shall also report any job it is currently running or has recently finished that was not included in the Controller's query, so the Controller can recover a job assignment its own records are missing.
|
||||
7. The Worker shall bound "recently finished" (Acceptance Criterion 6) to a configurable time window, exposed as a cluster configuration parameter, so operators can tune how long a completed job remains eligible for this extra reporting without unbounding the exchange.
|
||||
8. The Worker shall delay resuming its normal post-join operation (for example, periodic background reporting) until it has finished sending its complete job-state report (Acceptance Criteria 1-6) for the current join, so that the worker's other activity does not begin or interleave with the reconciliation exchange it is still completing.
|
||||
|
||||
### Requirement 4: Controller Reconciliation
|
||||
|
||||
**Objective:** As the Controller, I want to apply a worker's job-status report as authoritative, idempotent corrections to my own job records, so that a job's recorded state converges to the truth without duplicating or corrupting existing facts.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When the Controller receives a worker's job-status report, the Controller shall apply each reported job outcome through the same idempotent commit-then-apply path used for ordinary job-state facts.
|
||||
2. When the Controller reconciles a job's finish outcome, the Controller shall record the finish time the Worker reported (Requirement 3, Acceptance Criterion 5) as the job's finish time, rather than the time the Controller processed the reconciliation.
|
||||
3. If a reported job outcome would duplicate a fact the Controller has already committed for that job, the Controller shall apply it as a no-op rather than recording a duplicate.
|
||||
4. If the Controller has already recorded a more final or newer state for a job than the worker's report implies (for example, the Controller already recorded the job as finished by another means), the Controller shall not overwrite that state with a less final or older report.
|
||||
5. When a worker reports a queried job as unrecognized, the Controller shall treat this as evidence the job's true outcome could not be confirmed by the worker and shall not fabricate a finish outcome for it.
|
||||
6. When a worker reports a queried job as unrecognized, the Controller shall return that job to pending state, making it eligible for the normal scheduling path to assign it again, rather than leaving it permanently in its prior non-terminal state.
|
||||
7. When a worker reports a job that was not part of the Controller's query (Requirement 3, Acceptance Criterion 6), the Controller shall reconcile that job's state through the same idempotent commit-then-apply path and precedence rules used for queried jobs (Acceptance Criteria 1, 3, 4), rather than discarding the report.
|
||||
8. When the Controller reconciles a job reported outside its original query and the Controller has no existing non-terminal record of that job assigned elsewhere, the Controller shall record the job as assigned to the reporting worker, matching what the worker reported.
|
||||
|
||||
### Requirement 5: Dispatch Gating During Resync
|
||||
|
||||
**Objective:** As the Controller, I want to withhold new job dispatch to a rejoining worker until its job-state reconciliation completes, so that I never assign new work to a worker whose true resource usage I don't yet know.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. While a worker's job-state reconciliation is outstanding, the Controller shall not dispatch new jobs to that worker.
|
||||
2. When a worker's job-state reconciliation completes (all queried jobs have a reported and reconciled outcome), the Controller shall make that worker eligible for new job dispatch again.
|
||||
3. If a worker's job-state report does not arrive within a bounded time after the query is sent, the Controller shall resolve the outstanding reconciliation (accepting the query as answered, timed out, or reattempted on a subsequent rejoin) rather than withholding that worker from dispatch indefinitely.
|
||||
4. If the Controller attempts to dispatch a job to a worker that is not currently connected, the Controller shall fail that dispatch attempt immediately and return the job to pending state, making it eligible for the normal scheduling path to assign it again, rather than holding the dispatch for delivery once the worker reconnects.
|
||||
|
||||
### Requirement 6: Resync Failure and Repeated Rejoin
|
||||
|
||||
**Objective:** As the Controller, I want repeated or interrupted rejoin attempts to stay safe, so that a worker that keeps disconnecting during reconciliation never leaves job state permanently ambiguous or duplicated.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. If a worker disconnects again before its job-state report is fully processed, the Controller shall discard the incomplete reconciliation and shall re-initiate job-state reconciliation on the worker's next rejoin.
|
||||
2. The Controller shall produce the same reconciled result whether a worker's job-state report is applied once or is, due to a retry, presented to the Controller more than once for the same rejoin.
|
||||
3. If a worker's job-state report arrives for a reconciliation round the Controller has already discarded or superseded with a newer one (Acceptance Criterion 1), the Controller shall discard that report rather than apply it.
|
||||
|
||||
### Requirement 7: Scheduler Notification of Reconciled State
|
||||
|
||||
**Objective:** As the Controller, I want every job-state change produced by reconciliation to reach the Scheduler, so that the Scheduler's view of a job converges to the same truth as the Controller's, not just the Controller's own job table.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When reconciliation changes a job's recorded state, the Controller shall notify the Scheduler of that change through the same notification path used for an ordinary (non-reconciliation) job-state change.
|
||||
2. When reconciliation returns a job from running back to pending state (Requirement 4, Requirement 5), the Controller shall notify the Scheduler of the job's new pending state, so the Scheduler makes the job available for scheduling again rather than continuing to treat it as running.
|
||||
3. The Controller shall not consider reconciliation of a job complete until the resulting state change, if any, has been handed to the same notification path the Scheduler already relies on for ordinary job-state changes.
|
||||
|
||||
### Requirement 8: Bounded Job-State Exchange Size
|
||||
|
||||
**Objective:** As an Operator, I want job-state reconciliation to complete reliably even for a worker running many jobs at once, so that a busy host is never unable to rejoin the cluster simply because it has a lot of jobs to reconcile.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. The Controller shall be able to query and reconcile job state for a worker regardless of how many non-terminal jobs are currently assigned to that worker, without the reconciliation exchange failing solely because of the number of jobs involved.
|
||||
2. The Worker shall be able to report status for every job in the Controller's query, and any additional jobs it is running (Requirement 3, Acceptance Criterion 6), without the report failing solely because of the number of jobs involved.
|
||||
3. Where the number of jobs to reconcile for a worker exceeds what the Controller and Worker exchange in a single message, the exchange shall complete correctly across multiple messages rather than truncating the list or silently dropping jobs beyond a fixed limit.
|
||||
113
.kiro/specs/worker-rejoin-resync/research.md
Normal file
113
.kiro/specs/worker-rejoin-resync/research.md
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
# Research & Design Decisions: worker-rejoin-resync
|
||||
|
||||
## Summary
|
||||
- **Feature**: `worker-rejoin-resync`
|
||||
- **Discovery Scope**: Extension (existing system) — integration-focused, cross-cutting across controller, worker, and scheduler around an already-implemented join/rejoin path and an already-implemented commit-then-apply seam.
|
||||
- **Key Findings**:
|
||||
- `rejoin_worker`/`apply_join_state` (worker.c, from `worker-join-refactor`) is the exact moment a host becomes "connected" again — the natural trigger point for Requirement 1, and a new `worker_t` status bit here gates new dispatch "for free" through the *existing* `dispatch_a_job_` connected-state check with zero changes to that function.
|
||||
- The job table's idempotent commit-then-apply split (`record_job_*_fact` / `apply_job_*_transition`, with `job_state_is_terminal`/`job_state_is_backward` guards) is the exact dedup mechanism the requirements' Boundary Context says this feature must reuse, not duplicate. Three of four reconciliation outcomes (finished-success, finished-failure, still-running) already have a fully-idempotent home; only "unrecognized → return to pending" and "unqueried job → assign" are genuinely new fact shapes.
|
||||
- The controller→scheduler notification path (Requirement 7) is a hook → notify → wire → dequeue pipeline (`XCE_CONTROLLER_HOOK_JOB` → `sched.c` hook callbacks → `notify()` → scheduler's `handler.c` inbound switch → `sched_job_state_change()`), and it is **strictly separate** from the scheduler's own outbound decision-publish path (`scheduler.c`/`driver.c`), which a standing text-scan test (`test_no_worker_resync_symbols_in_scheduler_publish_path`) guards against ever learning about "resync" concepts. This fixes the correct integration seam: `handler.c`'s inbound switch only.
|
||||
- The worker has **no existing retention** of finished-job state — `cleanup_worker_list()` reaps and deletes an exited job's record (memory + sqlite row) on the very next cleanup pass, with no `finish_time` field anywhere in the worker's job structs today. New short-lived retention is required, not a rewiring of an existing cache.
|
||||
- `xce_reply_get()`/`xce_push_data_req()` (`{pos, count, n_total}` chunking, used today for joblist/workerlist/reslist replies) is the established bounded/paginated multi-message idiom in this codebase and is the direct template for Requirement 8, not the heavier WAL catch-up/replication protocol and not `XCECLI_FF_CALL` (whose whole point is "loss is fine" — the opposite of what this feature needs).
|
||||
- `config-design.md`'s rule set classifies the new "recently finished" window (Requirement 3.7) the same way `heartbeatInterval`/`inactiveHeartbeatCount` were classified: passes the independence rule (no cross-node correctness dependency) but is kept in shared config anyway via the documented operational-uniformity override, for the same "predictable cluster-wide behavior" reason.
|
||||
|
||||
## Research Log
|
||||
|
||||
### Controller-side join/rejoin flow and job state machine
|
||||
- **Context**: Where does reconciliation trigger, and how does it reuse the existing idempotent fact/apply path?
|
||||
- **Sources Consulted**: `src/controller/worker.c` (`rejoin_worker:403`, `apply_join_state:321`, `on_worker_join:623`), `src/controller/join.c` (`handle_worker_join:48`), `src/controller/job.c` (`record_job_finish_fact:2626`, `record_job_fail_fact:2781`, `record_job_postdone_fact:2921`, `apply_job_update_transition:2808`, `dispatch_a_job_:1278`), `src/controller/persist.c` (`apply_committed_entry:4333`), `src/controller/commit.c` (`commit_submit:485`), `src/include/xce/types.h` (`worker_t:284`, status bits `170-184`), `.kiro/specs/worker-join-refactor/design.md`.
|
||||
- **Findings**:
|
||||
- `handle_worker_join` durably commits the join (`persist_commit_barrier`) before `on_worker_join` runs — a second, later plug-in point after the join is durable.
|
||||
- `worker_t.host.status` is a bitmask; `dispatch_a_job_` (job.c:1511-1512) already refuses dispatch when `w->ep == NULL || w->host.status != 0`. A new `XCE_WORKER_RECONCILING`-style bit therefore gates Requirement 5.1 with no change to `dispatch_a_job_` itself.
|
||||
- `record_job_finish_fact`/`apply_job_finish_transition` already sets `finish_time` at receipt time (bug-1689), matching Requirement 3.5/4.2 exactly — the finished-job reconciliation case is a straight reuse.
|
||||
- `JOB_STAT_PEND` is otherwise only ever set at job creation — there is no existing "return a RUN job to PEND" transition. Since RUN→PEND is non-terminal→non-terminal, the existing `job_state_is_backward` guard does not block it; a thin new fact wrapper routed through the same `WAL_JOB_UPDATE`/`apply_job_update_transition` seam is the correct reuse (not a new state-machine core).
|
||||
- No existing "assign a job to a worker the controller didn't previously have a record for" fact exists; the closest analog is `dispatch_a_job_`'s own assignment logic (job.c:1686-1734).
|
||||
- The pending-facts queue is not a separate structure — it *is* the commit engine (`commit_submit`, `COMMIT_MODE_RETAIN`) plus `apply_committed_entry`'s guards. Reconciliation must submit through this exact seam, never a parallel one (this is also stated explicitly in the requirements' own Boundary Context).
|
||||
- **Implications**: Reconciliation needs exactly two new fact-recording functions in `job.c` (reconcile-to-pending, reconcile-assign) plus straight reuse of the three existing ones (finish, fail, no-op-for-running).
|
||||
|
||||
### Worker-side job bookkeeping and wire protocol
|
||||
- **Context**: What can the worker actually report, and how should the query/report exchange ride the wire?
|
||||
- **Sources Consulted**: `src/worker/include/xcecd_worker_utils.h`, `src/worker/src/xcecd_worker_utils.c` (`cleanup_worker_list:385-404`), `src/worker/include/xcecd_utils.h`, `src/lib/request.c` (`xce_join_req:1398-1429`, `xce_reply_get:1627-1784`, `xce_push_data_req:1491-1610`), `src/schema/controller.capnp` (`JoinData:155-161`, `HostAttrs:126-135`), `src/lib/client_internal.h` (`XCECLI_FF_CALL:290-302`), `src/lib/client.c`.
|
||||
- **Findings**:
|
||||
- `xcecd_worker_t` (PID-keyed) and `xcecd_job_t` carry no timestamp field at all; `cleanup_worker_list()` deletes both the in-memory entry and its sqlite row essentially immediately after exit — there is no retention window today.
|
||||
- `JoinData`'s capnp union has exactly two arms (`nodata`, `attrs`) with no job-state content, confirming the brief's own claim. Cramming a paginated exchange inline into `JoinData` would force it through `xce_join()`'s single-shot synchronous `cbuf_timedget` call shape, which cannot paginate — a new follow-up opcode/message pair after the join reply is the right shape, not a `JoinData` union extension.
|
||||
- `xce_reply_get`/`xce_push_data_req`'s `{pos, count, n_total}` chunking (already used for joblist/workerlist/reslist) is the established, directly-reusable bounded-exchange template for Requirement 8.
|
||||
- `XCECLI_FF_CALL`'s own header comment explicitly names "worker-rejoin-resync" as the intended fix for the loss it tolerates — confirming this feature's problem statement directly in code, not just in the bug report.
|
||||
- **Implications**: New capnp opcode pair (controller→worker query, worker→controller report), both using the existing pagination shape; new worker-side finished-job retention structure with a timestamp field, bounded by the Requirement 3.7 window.
|
||||
|
||||
### Scheduler notification path and scope-fence test
|
||||
- **Context**: Requirement 7 requires reconciliation-driven job-state changes to reach the scheduler through "the same path" as an ordinary change — what is that path, concretely, and are there any standing tests that constrain where new code may live?
|
||||
- **Sources Consulted**: `src/controller/job.c` (hook call sites `HOOK_DETAIL_JOB_EXIT:2641`, `HOOK_DETAIL_JOB_FAIL:2745`), `src/controller/sched.c` (`sched_job_hook_fun:547`, `sched_job_finish_fun:392`, `sched_job_fail_fun:438`, `sched_job_update_fun:468`), `src/controller/dataex.c` (`notify:399`), `src/controller/job.c` (`notify_dispatch:1971`), `src/scheduler/src/handler.c` (`sched_handle_notify:525`, `sched_job_state_change:407`), `src/scheduler/testsuite/test_scheduler_decision_redelivery.c:424-456`.
|
||||
- **Findings**:
|
||||
- Job-state-change code fires `XCE_CONTROLLER_HOOK_JOB` (not `HOOK_WORKER` — that category is for worker join/dead events); `sched.c`'s registered hook callback dispatches by `HOOK_DETAIL_*` to one of three functions, each of which calls the shared `notify()` with a distinct `NotifyData_data_sched*` event.
|
||||
- `notify_dispatch()`/`NotifyData_data_jobDispatchFail` is an existing, direct precedent for "tell the scheduler this job is back to pending" — its handler in `handler.c` already calls `sched_job_state_change(sched, jobid, JOB_STAT_PEND)`.
|
||||
- `test_no_worker_resync_symbols_in_scheduler_publish_path` is a static text-scan (`strstr` for `"resync"`) over `scheduler.c` and `driver.c` — the scheduler's own **outbound** decision-publish direction. It is a deliberate scope-fence: that direction must never learn about worker-resync concepts. It does not cover `handler.c` (the **inbound** Controller→Scheduler direction), which is unambiguously the correct, and only permitted, integration seam.
|
||||
- The async-I/O steering doc's thread-role contract applies identically to the scheduler; a bounded reconciliation wait must follow the vtask + admission-reservation pattern (`commit.c`'s `COMMIT_WAIT_RESERVED`, `wait_cap`/`wait_admit_locked`) rather than any raw blocking wait on iomgr/delaymgr, with the timeout itself fired by a delaymgr trigger.
|
||||
- **Implications**: Reconciliation's finish/fail cases need zero new scheduler-facing code (they already drive `schedExitJob`/`schedFailJob`). The reconcile-to-pending case should reuse the existing `jobDispatchFail`-style `sched_job_state_change(..., JOB_STAT_PEND)` call shape (either by literally reusing that notify event or adding a same-shaped one) — never by adding anything to `scheduler.c`/`driver.c`.
|
||||
|
||||
## Architecture Pattern Evaluation
|
||||
|
||||
| Option | Description | Strengths | Risks / Limitations | Notes |
|
||||
|--------|-------------|-----------|---------------------|-------|
|
||||
| Inline extension of `JoinData` union | Add job-state content as a third `JoinData` union arm | Minimal new opcode surface | Forces the exchange through `xce_join()`'s single-shot synchronous reply — cannot paginate (Requirement 8) | Rejected |
|
||||
| New opcode pair, ad hoc payload shape | New `OPCode_jobStateQuery`/`OPCode_jobStateReport`, custom framing | Full control over shape | Reinvents pagination the codebase already solved | Rejected |
|
||||
| New opcode pair reusing `{pos,count,n_total}` pagination | New opcode pair, payload shape mirrors `JobData`/`WorkerData`/`ResourceVals` | Directly reuses an established, tested idiom (`xce_reply_get`/`xce_push_data_req`); minimal new protocol concepts | None significant | **Selected** |
|
||||
| Reuse WAL catch-up/replication streaming protocol | Model the exchange on `replEntry`/`walCatchupEntry` | Already resumable/ack-per-entry | Heavier machinery designed for log replication, wrong semantic fit for a bounded one-time query/report | Rejected |
|
||||
|
||||
## Design Decisions
|
||||
|
||||
### Decision: New opcode pair for the job-state query/report exchange, following the `xce_reply_get`/`xce_push_data_req` pagination template
|
||||
- **Context**: Requirements 2, 3, and 8 need a controller-initiated, bounded/paginated, bidirectional exchange that today's join message cannot carry.
|
||||
- **Alternatives Considered**: inline `JoinData` extension; ad hoc new framing; WAL-catch-up-style streaming (see table above).
|
||||
- **Selected Approach**: Two new opcodes — `OPCode_jobStateQuery` (controller→worker) and `OPCode_jobStateReport` (worker→controller) — both paginated with the existing `{pos, count, n_total}` shape, issued as a follow-up exchange after the join reply (not inline in `JoinData`).
|
||||
- **Rationale**: Reuses a proven, already-tested idiom instead of inventing a new one; keeps the join message itself unchanged (no regression risk to `worker-join-refactor`'s existing behavior, per its own Boundary Commitments).
|
||||
- **Trade-offs**: One more opcode pair to add to `ha_dispatch_gate`'s role tables and `myreq_handler`'s switch, versus a cleaner, template-following protocol.
|
||||
- **Follow-up**: Confirm at implementation time whether `OPCode_jobStateReport` needs an explicit redirect-reply path if ever received by a non-primary (per `api-standards.md`'s "Adding a New Opcode" step 6 / bug-1700 precedent) — expected answer is no, since workers only ever connect to the current primary, but this must be verified, not assumed.
|
||||
|
||||
### Decision: Reconciliation-outstanding state lives on `worker_t` as a status bit + a small tracking record, not a new global registry
|
||||
- **Context**: Requirement 5 needs to gate new dispatch per-worker while reconciliation is outstanding, and Requirement 6 needs "discard on disconnect, re-trigger on next rejoin" semantics.
|
||||
- **Alternatives Considered**: a separate global map keyed by worker; per-worker state (selected).
|
||||
- **Selected Approach**: A new `XCE_WORKER_RECONCILING` status bit (reusing the existing `w->host.status != 0` dispatch check "for free") plus a small struct on `worker_t` holding the queried job-ID set, a round/generation token (for Requirement 6.3's late-report discard), and the timeout deadline.
|
||||
- **Rationale**: `worker_t` is already the natural per-host session object (confirmed by `worker-join-refactor`'s own design); a new global structure would duplicate lifecycle management the existing object already handles (creation, teardown, locking).
|
||||
- **Trade-offs**: None significant — this is the minimal-surface option.
|
||||
- **Follow-up**: None.
|
||||
|
||||
### Decision: Reconcile-to-pending and reconcile-assign are new, narrow fact-recording functions in `job.c`, not a new state-machine core
|
||||
- **Context**: Requirement 4.6 (unrecognized → pending) and Requirement 4.8 (unqueried job → assign) have no existing idempotent fact path.
|
||||
- **Alternatives Considered**: a new, separate reconciliation-specific mutation path; thin wrappers over the existing `WAL_JOB_UPDATE`/dispatch-assignment seams (selected).
|
||||
- **Selected Approach**: `record_job_reconcile_pending_fact()` submits through the existing `WAL_JOB_UPDATE`/`apply_job_update_transition` seam with target state `PEND` (same shape as any other job-state-update fact, so it drives the existing `HOOK_DETAIL_JOB_UPDATE` → scheduler-notify path for free). `record_job_reconcile_assign_fact()` mirrors `dispatch_a_job_`'s own assignment logic for host/state but does **not** attempt to reconstruct the job's original resource allocation (see Risks & Mitigations).
|
||||
- **Rationale**: The requirements' Boundary Context and Constraints explicitly forbid a second, competing idempotency mechanism; both new functions are additive call sites into the existing guarded seam, not new guards.
|
||||
- **Trade-offs**: The reconcile-assign case inherits a real, pre-existing limitation (below) rather than solving it.
|
||||
- **Follow-up**: None — the limitation is out of this spec's boundary by design.
|
||||
|
||||
### Decision: The "recently finished" reporting window (Requirement 3.7) is shared cluster config, via the operational-uniformity override
|
||||
- **Context**: Requirement 3.7 asks for a bounded, cluster-configurable window; `config-design.md` governs where any such field belongs.
|
||||
- **Alternatives Considered**: per-node local config (passes the independence rule cleanly — no cross-node correctness dependency); shared config via override (selected).
|
||||
- **Selected Approach**: Add the window (and, for the same reason, the Requirement 5.3 reconciliation timeout) as new fields on the existing `ClusterConfig` capnp blob, alongside `heartbeatInterval`/`inactiveHeartbeatCount`.
|
||||
- **Rationale**: `config-design.md`'s own worked example classifies the structurally identical `heartbeatInterval`/`inactiveHeartbeatCount` fields (controller→worker liveness, not HA/Raft election heartbeat) as shared-via-override for exactly this reason: no correctness invariant demands cluster-wide agreement, but operators want uniform, predictable behavior rather than per-node drift. This feature's window is the same shape of parameter in the same domain (worker liveness/reconnection).
|
||||
- **Trade-offs**: A config push is required to change it (heavier than a per-node flag), accepted for the same reason the precedent field accepted it.
|
||||
- **Follow-up**: None.
|
||||
|
||||
### Decision: Config push and the job-state query are sent in a fixed order (config first) with no new ack — ordering is guaranteed by the single connection, not invented
|
||||
- **Context**: `on_worker_join` (worker.c:623) must trigger both the existing config push and the new job-state query for a rejoining worker. Does the worker need to fully apply config before it can correctly answer the query, and if so, how is that ordering enforced?
|
||||
- **Alternatives Considered**: (1) have the controller wait for an explicit worker-sent config-applied ack before sending the query (initially adopted, then retracted); (2) rely on the connection's existing ordering guarantees (selected).
|
||||
- **Selected Approach**: `on_worker_join` sends config push (unchanged `worker_push_config` call), then immediately sends the new job-state query — both fire-and-forget, no new ack/reply added.
|
||||
- **Rationale**: Initially assumed the worker might dispatch incoming controller requests concurrently across its `thread_pool_t`, which would make arrival order insufficient to guarantee handling order — this assumption was wrong. Reading `controller_connector.c:1563`'s `myreq_handler` directly confirmed it is a plain synchronous `switch`, invoked one message at a time off the worker's single long-lived client connection (`xcecli_ctx`); the worker's `thread_pool_t` is scoped to job-forking/job-control work only, not request dispatch. On one connection, TCP ordering + this synchronous single-message dispatch together guarantee config push is *handled to completion* before the job-state-query handler even begins — not just sent first. No config content is actually needed to answer the query correctly either way (job IDs are globally unique, independent of host config), so this was a belt-and-suspenders concern, not a load-bearing correctness dependency — but the ordering guarantee turned out to be free, so it's stated explicitly rather than left implicit.
|
||||
- **Trade-offs**: None — this is strictly simpler than the retracted ack-based alternative, with no loss of guarantee.
|
||||
- **Follow-up**: If the worker's request-dispatch model is ever changed to fan out across a thread pool (unlike today), this ordering guarantee must be re-derived — it is not inherent to the wire protocol, only to the current single-threaded-dispatch implementation.
|
||||
|
||||
## Risks & Mitigations
|
||||
- **Risk**: A reconciliation-assigned job (Requirement 4.8) has no recoverable original resource allocation (`job->allocs`) — the controller never held a dispatch record for it, so there is nothing to read the allocation back from. — **Mitigation**: Document as a deliberate, bounded scope limitation (see `design.md`'s Data Models persistence-completeness note): the job's *state* and *host* are corrected; its resource *reservation* accounting starts from zero/unknown, converging via the existing `resource-accounting.md` "charged = max(reserved, actual)" policy on the job's next rusage tick — identical to how any legitimately-unreserved job is already handled today, not a new class of gap.
|
||||
- **Risk**: A new WAL fact type (reconcile-assign) must be decoded/adopted identically by both apply seams (`apply_committed_entry` primary-boot-replay and the standby catch-up apply path) or it reproduces the exact bug-1678 field-completeness failure mode. — **Mitigation**: `design.md` calls this out explicitly as a persistence-completeness checklist item, per `design-principles.md`'s Persistence Review requirement.
|
||||
- **Risk**: A blocking wait for the worker's report, implemented carelessly, could stall a liveness-critical thread (iomgr/delaymgr). — **Mitigation**: Reuse `commit.c`'s `COMMIT_WAIT_RESERVED` admission-reservation pattern for any bounded wait; drive the timeout via a delaymgr-scheduled trigger, never a blocking sleep.
|
||||
- **Risk**: The requirements' Boundary Context explicitly disclaims coupling the Requirement 3.7 window to `commit-batch-window`'s (separately parked) commit-urgency tuning. — **Mitigation**: `design.md` does not reference or depend on `commit-batch-window` in any component contract.
|
||||
|
||||
## References
|
||||
- `.kiro/specs/worker-rejoin-resync/requirements.md` — approved-for-design requirements (8 requirement groups, 30 acceptance criteria)
|
||||
- `.kiro/specs/worker-rejoin-resync/brief.md` — original discovery brief with file:line evidence
|
||||
- `.kiro/specs/worker-join-refactor/design.md` — `rejoin_worker`/`apply_join_state` contract this feature extends
|
||||
- `.kiro/steering/config-design.md` — shared-vs-local config classification rule, applied to Requirement 3.7
|
||||
- `.kiro/steering/resource-accounting.md` — reserved/used accounting model the reconcile-assign limitation converges into
|
||||
- `.kiro/steering/persistence.md` — `APPLY_DIVERGENT`/bug-1678 per-field completeness precedent
|
||||
- `.kiro/steering/api-standards.md` — "Adding a New Opcode" checklist (bug-1700 redirect-reply precedent)
|
||||
- `.kiro/steering/ha-async-io.md` — thread-role contract for the bounded reconciliation wait
|
||||
191
.kiro/specs/worker-rejoin-resync/tasks.md
Normal file
191
.kiro/specs/worker-rejoin-resync/tasks.md
Normal file
|
|
@ -0,0 +1,191 @@
|
|||
# Implementation Plan
|
||||
|
||||
- [x] 1. Foundation: wire protocol schema and shared reconciliation types
|
||||
- [x] 1.1 Define the wire protocol for the reconciliation exchange
|
||||
- Add the controller→worker job-state query and worker→controller job-state report as new opcodes, each carrying a paginated list of per-job entries (job ID, status, exit code, worker-reported finish time, and whether the job was part of the original query)
|
||||
- Add the two new cluster-wide configuration fields: the recently-finished reporting window and the reconciliation timeout
|
||||
- Observable completion: the schema builds cleanly and the new message/opcode/config-field definitions are visible in the generated bindings
|
||||
- _Requirements: 2.1, 2.2, 3.5, 3.7, 5.3, 8.1, 8.2, 8.3_
|
||||
|
||||
- [x] 1.2 (P) Add a worker reconciling status and round-tracking state
|
||||
- Add a new worker status value meaning "job-state reconciliation outstanding for this host"
|
||||
- Add a per-worker reconciliation-round structure capable of holding a round identifier, the set of queried job IDs, and a deadline
|
||||
- Observable completion: the new status value and round structure compile and are reachable from the worker record, unused by any caller yet
|
||||
- _Requirements: 5.1, 6.3_
|
||||
- _Boundary: shared types_
|
||||
|
||||
- [x] 1.3 (P) Establish worker-side C test infrastructure
|
||||
- Add the first test-runner harness for the worker daemon, mirroring the controller's existing runner conventions
|
||||
- Observable completion: an empty/placeholder worker-side test program builds and runs successfully via the existing test build target
|
||||
- _Requirements: 3.1, 3.2, 3.3_
|
||||
- _Boundary: worker test infrastructure_
|
||||
- _Note: satisfied by pre-existing infrastructure — see Implementation Notes._
|
||||
|
||||
- [x] 1.4 (P) Add wire encode/decode helpers for the reconciliation exchange
|
||||
- Add request/reply encode and decode helpers for the new query and report messages, following the existing paginated-list pattern already used for job/worker/resource listings
|
||||
- Observable completion: a round-trip encode-then-decode of a query and of a report (including an empty page and a multi-page case) reproduces the original data exactly
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Boundary: wire codec_
|
||||
- _Depends: 1.1_
|
||||
|
||||
- [x] 1.5 (P) Wire the new config fields into xceadmin's config-build pipeline
|
||||
- Add the two new cluster-config fields (the recently-finished reporting window, the reconciliation timeout) to xceadmin's YAML validation schema and its YAML-to-binary field mapping, following the existing `heartbeatInterval`/`inactiveHeartbeatCount` pattern exactly, so an operator can actually set them through `xceadmin config build` — added to the wire schema by task 1.1, but with no configuration path into it until this task
|
||||
- Regenerate the checked-in golden config-build fixtures to match the wire schema's current field set (task 1.1 grew it; the fixtures predate that and no longer match a fresh build)
|
||||
- Observable completion: a YAML config supplying values for both new fields produces a binary that decodes back to those exact values, and the existing golden-fixture regression tests pass again
|
||||
- _Requirements: 3.7, 5.3_
|
||||
- _Boundary: xceadmin config-build_
|
||||
- _Depends: 1.1_
|
||||
|
||||
- [x] 2. Core: controller job-table reconciliation outcomes
|
||||
- [x] 2.1 Return an unrecognized job to pending state, releasing its resource allocation
|
||||
- When a worker reports a queried job as unrecognized, transition that job back to pending using the existing state-transition guard so a terminal or already-more-final job is never overwritten
|
||||
- Release any resource allocation the job was carrying, so a job returned to pending never carries a stale allocation from a prior dispatch into its next dispatch attempt
|
||||
- Durably record the transition and notify the scheduler of the new pending state through the existing notification path, without waiting for a later replay to discover the change
|
||||
- Observable completion: a job previously dispatched (real allocation committed) that reconciles to unrecognized ends the call with pending state, no resource allocation, and the scheduler notified — verified before any commit round-trip completes
|
||||
- _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 7.1, 7.2, 7.3_
|
||||
- _Boundary: job.c additions_
|
||||
|
||||
- [x] 2.2 Assign an unqueried job to the worker that reported it
|
||||
- When a worker reports a job the controller has no existing non-terminal record of, record that job as assigned to the reporting worker and host, matching the worker's reported status, through the same idempotent path used for a normal dispatch
|
||||
- Do not fabricate a resource allocation for this job — leave the allocation unset, consistent with the controller genuinely not knowing what the scheduler originally computed
|
||||
- Notify the scheduler of the new assignment through the existing notification path
|
||||
- Observable completion: a job the controller never previously tracked, reported running by a worker, ends the call recorded as running on that host with the scheduler notified, and durably survives a restart replay with the same result
|
||||
- _Requirements: 4.1, 4.3, 4.4, 4.7, 4.8, 7.1, 7.2, 7.3_
|
||||
- _Boundary: job.c additions_
|
||||
|
||||
- [x] 2.3 Fail a dispatch to a disconnected worker immediately, without retry or job failure
|
||||
- When the controller attempts to dispatch a job to a worker it is not currently connected to, fail that specific dispatch attempt right away and return the job to pending, reusing the pending-and-release behavior from task 2.1, rather than retrying the attempt or exhausting a retry count toward permanent failure
|
||||
- Leave the existing behavior for every other reason a worker might not be ready (locked, busy, closed, or mid-reconciliation) completely unchanged — confirm those cases still retry and eventually fail exactly as before
|
||||
- Observable completion: dispatching to a disconnected worker returns the job to pending on the first attempt with the job's failure-retry count untouched and no permanent failure recorded, while dispatching to a worker that is connected but otherwise not ready still follows the pre-existing retry-then-fail behavior unchanged
|
||||
- _Requirements: 5.1, 5.4_
|
||||
- _Boundary: job.c additions_
|
||||
- _Depends: 2.1_
|
||||
|
||||
- [x] 3. Core: controller reconciliation-round bookkeeping
|
||||
- [x] 3.1 (P) Track one reconciliation round per worker with a bounded timeout
|
||||
- Given a worker host with one or more non-terminal jobs currently assigned to it, construct the exact list of those jobs' unique IDs as the round's query set
|
||||
- Track a round identifier that distinguishes the current reconciliation attempt from any prior one for the same worker
|
||||
- Schedule a bounded timeout for the round using a non-blocking trigger; when it fires before the round otherwise resolves, resolve the round rather than leaving the worker withheld from dispatch indefinitely
|
||||
- Observable completion: arming a round for a worker with known non-terminal jobs produces a query set matching exactly those job IDs, a fresh round identifier, and an active timeout; letting the timeout elapse resolves the round without any blocking call
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 5.3, 6.1_
|
||||
- _Boundary: resync.c_
|
||||
|
||||
- [x] 3.2 Apply each reported job outcome through the correct idempotent path
|
||||
- For each entry in a worker's job-state report: a still-running job is cleared from the round's outstanding set with no fact recorded; a finished-success or finished-failure job is applied through the existing finish/fail recording used for an ordinary report; an unrecognized queried job is applied through task 2.1's pending-and-release path; a reported job outside the original query is applied through task 2.2's assignment path
|
||||
- Discard a report that arrives for a round already superseded by a newer one for the same worker, rather than applying it
|
||||
- When the round's outstanding set becomes empty, resolve the round so the worker becomes eligible for dispatch again
|
||||
- Applying the same report twice for the same round must produce the same result as applying it once
|
||||
- Observable completion: a full report covering all five outcome kinds leaves the job table in the correct state for each, a duplicate re-application of the same report changes nothing further, and a report carrying a stale round identifier is dropped without touching the job table
|
||||
- _Requirements: 4.1, 4.2, 4.3, 4.4, 4.5, 6.1, 6.2, 6.3_
|
||||
- _Boundary: resync.c_
|
||||
- _Depends: 2.1, 2.2, 3.1_
|
||||
|
||||
- [x] 4. Core: worker-side job-state reporting
|
||||
- [x] 4.1 (P) Retain a job's true outcome for a bounded window after it exits
|
||||
- At the moment a job's exit is detected, before its record would otherwise be discarded, retain its exit status, exit code, and true finish time for a configurable bounded window
|
||||
- Observable completion: a job's outcome remains queryable for the configured window after it exits and is no longer available once the window elapses
|
||||
- _Requirements: 3.4, 3.5, 3.7_
|
||||
- _Boundary: worker retention_
|
||||
|
||||
- [x] 4.2 Answer the controller's job-state query, including extra unqueried jobs
|
||||
- For every job ID in the controller's query, report exactly one outcome: still running (from the live running list), finished successfully or with failure and its true finish time (from the new retention), or unrecognized if found in neither — every queried ID gets an answer, none are silently skipped, and an indeterminate case is reported as unrecognized rather than guessed
|
||||
- After answering the query, also report any job the worker is currently running or has recently finished that was not part of the query
|
||||
- Send both the query answers and the extra jobs in paginated reports, correct regardless of how many jobs are involved
|
||||
- Observable completion: a query covering a mix of running, finished, and unknown job IDs produces one correctly-classified entry per ID plus a separate page of extras for jobs outside the query, with no job silently dropped even when the total exceeds one page
|
||||
- _Requirements: 3.1, 3.2, 3.3, 3.6, 8.2, 8.3_
|
||||
- _Boundary: xcecd_resync.c_
|
||||
|
||||
- [x] 4.3 Wire the worker's receipt of a job-state query and delay resuming normal post-join operation until the report has fully sent
|
||||
- Also close a gap task 4.2 correctly left out of its own boundary: nothing today wires the worker's actual receipt of `OPCode_jobStateQuery` (`src/worker/src/controller_connector.c`'s `myreq_handler` switch has no case for it, confirmed by inspection — only `OPCode_startjob`/`OPCode_runtasks`/`OPCode_jobcontrol`/`OPCode_fetchfile` exist there today) to task 4.2's `xcecd_resync_answer_query()`. Add that dispatch case: reassemble an incoming query that itself arrives across multiple pages (Requirement 8.1/8.3) into the jobids array `xcecd_resync_answer_query()` expects, then call it
|
||||
- Set a local gate the moment a job-state query is received; keep it set across every page of the report; clear it only once the full report, including any extra-job pages, has finished sending
|
||||
- Make whatever the worker does next after rejoining (resuming its own periodic background reporting) check this gate first, so that activity never begins or interleaves while the report is still being sent
|
||||
- Observable completion: the worker's post-join activity is observed to start only after the last report page has been sent, never before, across a report spanning multiple pages; a job-state query arriving from the controller (single- or multi-page) reaches `xcecd_resync_answer_query()` and produces a real reply, not just a directly-called-in-tests function
|
||||
- _Requirements: 3.8, 8.1, 8.3_
|
||||
- _Boundary: xcecd_resync.c_
|
||||
- _Depends: 4.2_
|
||||
|
||||
- [x] 4.4 Gate the worker's other periodic background reporting on the same resync_pending flag
|
||||
- Close a Requirement 3.8 gap found by feature-level validation, missed by task 4.3's own boundary: `checking_connection()`'s `rejoin_call()` is not the worker daemon's only periodic post-join activity — `src/worker/src/resource_sync.c`'s `resource_sync_thread()` is a separate pthread, started once at daemon init (unrelated to any join event), that periodically (every `LOAD_INDICES_INTERVAL_DEFAULT`, 15s) sends live `xcecd_send_load_indices()`/`xcecli_update_resvals()` RPCs to the controller over the same connection used for the resync exchange, with zero reference to `resync_pending`/`xcecd_resync_is_pending()` anywhere in that file — confirmed by inspection
|
||||
- Make the periodic load-indices send check `xcecd_resync_is_pending()` first, the same way `checking_connection()`'s `rejoin_call()` already does (task 4.3), and skip/defer that cycle's send while a reconciliation report is still in flight — the send is already periodic, so a skipped cycle is naturally retried on the next tick, no new retry/queueing mechanism needed
|
||||
- Observable completion: a load-indices send that would otherwise fire while `resync_pending` is set is observed not to fire during that window, and resumes firing on its normal periodic cadence once the flag clears; the existing periodic cadence and behavior when no reconciliation is in flight are unchanged
|
||||
- _Requirements: 3.8_
|
||||
- _Boundary: resource_sync.c_
|
||||
- _Depends: 4.3_
|
||||
|
||||
- [x] 5. (P) Core: exclude a reconciling host from scheduler dispatch candidacy
|
||||
- Extend the scheduler's existing candidate-host filter so a host currently undergoing job-state reconciliation is excluded from dispatch candidacy the same way an already-unavailable host is
|
||||
- When the reconciling status clears, the same filter must stop excluding that host without any separate action
|
||||
- Introduces no new reconciliation-protocol name or concept into the scheduler's outbound decision-publish code — this is an ordinary status-bit read alongside the existing unavailable-host check, not a new symbol; the standing regression test guarding that publish path against ever referencing reconciliation concepts must stay green, unmodified
|
||||
- Observable completion: a host flagged as reconciling is never selected as a dispatch candidate while the flag is set, and becomes selectable again as soon as it clears, with no change to how any other exclusion reason behaves, and the publish-path symbol-guard test still passes unmodified
|
||||
- _Requirements: 5.1, 5.2_
|
||||
- _Boundary: scheduler candidate filter_
|
||||
|
||||
- [x] 6. Integration: cross-daemon wiring
|
||||
- [x] 6.1 Trigger reconciliation on worker rejoin and send the query
|
||||
- At the moment a worker host becomes connected again (first join, network reconnect, or reconnect after controller failover, treated identically), check whether the controller currently holds non-terminal jobs for that host and, if so, arm a reconciliation round for it
|
||||
- Send the round's query to the worker as a follow-up exchange after the join reply and the existing configuration push have both already been sent, not as an extension of the join call itself
|
||||
- Also close a gap task 4.1 discovered and correctly left out of its own boundary: the worker daemon has no delivery path at all today for any `ClusterConfig` field (the existing `worker_push_config`/`on_worker_join` push carries a generic `confvars` key/value list plus a small number of directly-pulled fields — `clustername` is the existing precedent, `src/controller/worker.c` around the `collect_confvars`/`conf4worker` construction). Add `workerResyncWindowMs` to that same push, mirroring the `clustername` field precedent exactly (no new opcode needed — this reuses the existing `OPCode_push`/`WorkerConfig` wire struct this task already touches), and have the worker's existing config-receive handling call `xcecd_finished_job_retention_set_window_ms()` (task 4.1) with the received value so the retention window task 4.1 built is actually operator-tunable end-to-end, not stuck on its documented placeholder default
|
||||
- Observable completion: rejoining a worker with known non-terminal jobs results in a reconciliation round being armed and the corresponding query being sent to that worker, in that order, after the join reply; rejoining a worker with none results in no round and no query; a worker that receives a config push with a non-default `workerResyncWindowMs` value applies it to its own retention window before the push completes
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 3.7_
|
||||
- _Depends: 1.4, 3.1, 4.1_
|
||||
|
||||
- [x] 6.2 Route an incoming job-state report to reconciliation processing
|
||||
- Dispatch an incoming job-state report message to the reconciliation-processing logic, correctly reassembling a report that arrives across multiple pages before it is applied
|
||||
- Confirm whether the two new message kinds need an explicit redirect response when received by a controller that is not currently primary, matching how every other primary-only message already behaves for a standby recipient — do not assume the existing generic rejection path already covers this without checking, given this exact class of gap has recurred for other message kinds before
|
||||
- Observable completion: a report sent across several pages is fully reassembled and applied exactly once, with the same result as an equivalent single-page report; and a query or report sent to a non-primary controller receives the same redirect behavior an equivalent primary-only message already receives, not a bare unrecoverable error
|
||||
- _Requirements: 8.1, 8.3_
|
||||
- _Depends: 1.4, 3.2_
|
||||
|
||||
- [x] 7. Validation: end-to-end and regression coverage
|
||||
- [x] 7.1 Full rejoin-reconciliation round trip
|
||||
- Exercise a real rejoin against a live controller and worker pair: a worker with a mix of still-running, finished-success, finished-failure, and unrecognized jobs reconnects, and every job ends up in the correct final state with the scheduler notified
|
||||
- Observable completion: the round trip produces the correct end state for every job kind in one live test run, matching the per-outcome behavior verified in isolation by tasks 2.1-2.2 and 3.2
|
||||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 2.2, 3.1, 3.2, 3.3, 3.6, 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 4.7, 4.8, 7.1, 7.2, 7.3_
|
||||
- _Depends: 6.1, 6.2, 4.3, 5_
|
||||
|
||||
- [x] 7.2 Failover-during-job recovery
|
||||
- Reproduce the feature's core motivating scenario: a job's finish report is lost across a controller primary failover, and the new primary's rejoin reconciliation with that worker recovers the job's true outcome
|
||||
- Observable completion: a job whose finish notification never reached any controller instance ends up correctly finished, not stuck running, once the worker rejoins the new primary
|
||||
- _Requirements: 4.1, 4.2, 4.6_
|
||||
- _Depends: 7.1_
|
||||
|
||||
- [x] 7.3 Burst/array recovery unbounded by job count
|
||||
- Reproduce a burst of many near-simultaneous job completions on one worker host where most of the individual finish reports are lost, and confirm reconciliation recovers all of them on the worker's next rejoin regardless of count
|
||||
- Observable completion: every job in the burst is correctly finished after reconciliation, with none left stuck and none duplicated
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 7.1_
|
||||
|
||||
- [x] 7.4 Large job-count pagination correctness
|
||||
- Drive the reconciliation exchange for a worker running enough jobs to require multiple pages of both the query and the report, confirming the exchange completes correctly rather than truncating or silently dropping jobs past a single page
|
||||
- Observable completion: a job count an order of magnitude larger than a single page holds is fully queried and fully reported, with every job accounted for
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 7.1_
|
||||
|
||||
- [x] 7.5 Wire a real page-size bound into the controller's outbound reconciliation query
|
||||
- Close bug-1755, found and correctly left unfixed by task 7.4's own test-only boundary: `worker_send_resync_query()` (`src/controller/worker.c`) hardcodes `pktlimit=0` in its `xce_job_state_query_req()` call, which task 7.4 independently confirmed means "send every non-terminal job ID in one unbounded page" — so the query direction never actually paginates in production today, even though task 1.4's underlying pagination mechanism (proven correct by task 7.4's own test) fully supports it, and design.md's Boundary Commitments explicitly states both opcodes are paginated
|
||||
- Give the query send site a real, bounded page size, mirroring the worker's own already-wired `XCECD_RESYNC_REPORT_PKTLIMIT` precedent (`src/worker/src/controller_connector.c`) — same constant-driven approach, not a new configuration surface
|
||||
- Observable completion: a worker rejoin with a non-terminal job count exceeding the chosen page size produces a query genuinely sent across multiple wire pages (observably, not just theoretically supported), with the same job-count-independent completeness task 7.4 already proved at the mechanism level; a rejoin with a job count under one page still produces exactly one page, unchanged from today's observable behavior
|
||||
- _Requirements: 8.1, 8.2, 8.3_
|
||||
- _Depends: 6.1, 7.4_
|
||||
|
||||
## Implementation Notes
|
||||
- Task 1.3: this task's premise (`src/worker` has no `testsuite/`) is FALSE as of implementation time — real, tracked worker test infrastructure already exists (`src/worker/testsuite/{meson.build,test_*.c}`, 5 passing tests, `suite: 'worker'` registered in meson, `/xce-build worker` already runs it), built by `component-failover-adoption` (commits `ce9f734a`/`331fa024`/`619b119e`, 2026-08-11) weeks before this spec was written. `.kiro/steering/structure.md`, `testing.md:55`, and `build-deployment.md:16` are all stale on this point — worth a `/kiro-steering` sync, out of this feature's boundary to fix. Confirmed directly (fresh build + `meson test -C src/xce.build --suite worker`: 5/5 OK) rather than trusting the finding secondhand. No files changed for this task — closed as satisfied by pre-existing infrastructure.
|
||||
- **Task 4.2 must follow the EXISTING worker testsuite convention**, not the controller aggregator-runner pattern the original task 1.3 text pointed at: each test file is its own standalone Unity executable (own `UNITY_BEGIN()`/`RUN_TEST`/`UNITY_END()`, white-box `#include`s the production `.c` file under test — see `test_resource_sync.c`'s shape) with its own explicit `executable()` + `test(..., suite: 'worker', ...)` block added to `src/worker/testsuite/meson.build`, mirroring `test_controller_connector.c`'s registration exactly. Do not introduce a second, competing test-runner shape.
|
||||
- Task 1.5: `xceadmin config dump`'s `_format_cluster_config` (`build.py`) does not print `workerResyncWindowMs`/`workerResyncTimeoutMs` — an operator can set them via `config build` but can't verify the built value via `config dump`. Confirmed genuinely out of task 1.5's boundary (config-*build*, not config-*dump*) and not required by Requirements 3.7/5.3 as written, but a real, minor observability gap worth a small follow-up task if this becomes a live operator pain point. Not blocking; not added to this plan.
|
||||
- **Task 2.2 found and fixed bug-1746**, a real pre-existing defect in `persist.c` (part of bug-1678's original fix, predates this feature): `capnp_encode_joballoc()` unconditionally dereferenced a NULL `alloc` via `encode_JobAlloc()` instead of the already-existing NULL-safe `encode_JobAlloc_ptr()` wrapper — never triggered before since `dispatch_a_job_` (the only prior caller) always passes non-NULL. This directly contradicted design.md's claim that the NULL-alloc case was "tested, backward-compatible" on both apply seams — true for *decode*, never actually true for *encode* until this task exercised it. Fixed with a minimal guard producing a true zero-length buffer (not a capnp-null-pointer encoding — verified against `apply_committed_entry`'s `alloc_len > 0` decode gate, which a non-zero-length "null" encoding would have wrongly satisfied). design.md corrected in 4 places. **Any future task calling `persist_submit_job_dispatch_fact` with a NULL alloc can now trust this path — it wasn't safe before task 2.2, it is now.**
|
||||
- Task 2.2 (self-correction): a coordinator-applied one-line comment fix to `job.c` accidentally closed the enclosing `/* */` comment block early, leaving the comment's remaining lines as bare (uncompilable) code — caught immediately by re-running the build before marking the task complete, not left for review to find. Reinforces why fresh-evidence re-verification applies even to "trivial, obviously-safe" doc-only edits made outside the TDD subagent loop.
|
||||
- Task 3.1: `resync.c`'s `resync_timeout_ms()` applies its own private default-if-unset fallback (15000ms) for `workerResyncTimeoutMs` rather than following `myconf.c`'s established `SET_DEFAULT_IF_UNSET`-style canonicalization convention used for the sibling `heartbeat_interval` field (`myconf.c:74`) — `myconf.c` is outside task 3.1's boundary, so this was correctly left as-is, but if a SECOND consumer of `workerResyncTimeoutMs` (or `workerResyncWindowMs`) is ever added, it would have to either duplicate this private fallback or the two could silently disagree. Worth canonicalizing in `myconf.c` if/when that happens — not needed for this feature's own single consumer. delaymgr timer cancellation: `delay.c`/`delay.h` expose no dequeue/cancel primitive for an already-armed one-shot task; `resync.c` handles this correctly via a by-value round-token check inside the fired callback (verified safe against both the logical "resolves the wrong round" bug and use-after-free, since the callback's closure never holds a pointer into the freed `resync_state_t`) — any future delaymgr one-shot user in this codebase facing the same "need to cancel" problem should use this same by-value-token-check pattern rather than assuming a cancel API exists.
|
||||
- **Task 3.2 needed 3 review rounds — a genuine cautionary tale about a boolean short-circuit hiding a second, adjacent bug.** Round 1 caught the unqueried-extra-"running" branch gating on terminal-vs-non-terminal instead of Req 4.8's real trigger (assignment status), silently leaving a job stuck PEND when its `WAL_JOB_DISPATCH` fact never reached durable commit before a failover — the feature's own core motivating scenario. The FIRST fix (`!terminal && assigned_to_w's_host`) solved that but introduced a NEW, distinct Req 4.4 violation via the same `&&`'s short-circuit: a TERMINAL job (DONE/EXIT) falls into the same `else`/assign branch and gets clobbered back to RUN — caught by round 2, and confirmed the implementer's own new test had been asserting the bug as correct behavior (a green suite is not proof of correctness when the test itself encodes the wrong expectation). The SECOND fix (a genuine three-way `if`/`else if`/`else`: terminal→noop, non-terminal-and-already-assigned-anywhere→noop, non-terminal-and-unassigned→assign) passed round 3's from-scratch re-derivation plus an explicit hunt for job==NULL reachability. **Lesson for future tasks in this codebase**: when a guard combines two orthogonal conditions with `&&`/`||` and only ONE of the two is corrected after a review finding, re-derive the FULL truth table before re-submitting — a targeted patch to the specific counterexample a reviewer names can leave an adjacent, differently-shaped counterexample in the same boolean untouched. bug-1747 (finish-time gap, split fixed) and bug-1748 (this guard defect) both logged.
|
||||
- Task 4.1: found and fixed a real-but-non-blocking gap the implementer's own doc comment claimed didn't exist: `finished_job_retention_prune()` was only invoked from `xcecd_finished_job_lookup()`, never from `xcecd_retain_finished_job()`, so with `xcecd_finished_job_lookup()` having zero production callers today (task 4.2 not yet implemented) the retention list had no active pruning driver and would grow one entry per finished job for the daemon's entire uptime — even after 4.2 lands, pruning would only fire on this worker's own rejoin, so a long-uptime well-connected worker could still accumulate substantial entries between rejoins. Fixed by also pruning on retain (using `finish_time_ms`, the value already captured at genuine exit-detection time, as the prune's `now_ms`). Also confirms task 4.1 has **no runtime delivery path yet for `workerResyncWindowMs`** — `xcecd_finished_job_retention_set_window_ms()` is a real, fully-wired seam but nothing calls it in production; task 6.1's scope was already expanded (see its entry above) to plug this in via `worker_push_config`'s existing `WorkerConfig` struct.
|
||||
- Task 4.2: implemented `xcecd_resync_classify_job`/`xcecd_resync_build_report`/`xcecd_resync_answer_query` (classify RUNNING/FINISHED_OK/FINISHED_FAIL/UNRECOGNIZED, compute extras, paginate via task 1.4's `xce_job_state_report_req()`) as directly-callable, fully-tested functions (20/20 tests) — necessarily added one small, purely-additive companion function to task 4.1's API, `xcecd_finished_job_retention_snapshot()`, since 4.1 only exposed single-jobid lookup and 4.2 needs to enumerate ALL retained jobs for the extras set. Confirmed by inspection that nothing wires the worker's actual receipt of `OPCode_jobStateQuery` to call into this code yet (`controller_connector.c`'s `myreq_handler` has no case for it) — folded that wiring explicitly into task 4.3's scope (see its entry above), since 4.3 already needs the real receive path to implement its post-join gate correctly and is the same file/boundary.
|
||||
- **Task 4.3 needed 2 review rounds — a genuine deadlock, not a style nitpick.** Round 1 placed the `resync_pending` gate check as `checking_connection()`'s entry guard (returning early before `xcecli_hello()`/the `needs_rejoin` check could run), so a worker whose connection dropped after a non-final page of a multi-page `OPCode_jobStateQuery` (no further page ever arriving to clear the gate) would have its ONLY connection-recovery mechanism permanently disabled — a direct Requirement 6.1 violation the existing suite didn't catch because no test drove the multi-page-then-disconnect sequence. Round 2 fixed it two ways: (a) narrowed the gate to guard only the `rejoin_call()` step inside the `needs_rejoin` branch, so `xcecli_hello()`/reconnect-detection always run every tick regardless of gate state; (b) added `xcecd_resync_reset()`, called from `xcecd_on_reconnect()` (the same signal that sets `needs_rejoin`), which clears the gate and discards any stalled partial multi-page accumulation the moment a real reconnect completes — bounding the deadlock window to at most one reconnect cycle instead of forever. Round 2's reviewer independently confirmed the fix by reintroducing the round-1 bug and rebuilding, and found the new *behavioral* regression test alone did NOT catch the reintroduced bug — only a new *structural* source-order test (asserting `xcecli_hello()` precedes the `needs_rejoin` check precedes the gate check, textually) actually distinguished correct from broken placement. **Lesson for future tasks in this codebase**: when a fix's correctness hinges on WHERE a check is placed relative to other statements (not just what value it reads), a pure behavioral/outcome test can pass even with the wrong placement if the test's specific stimulus happens not to exercise the broken ordering — add a structural/ordering assertion alongside the behavioral one for placement-sensitive gates.
|
||||
- Task 4.3 (advisory, non-blocking, not fixed in this pass): `resync_pending`/`query_accum` in `xcecd_resync.c` are plain unsynchronized statics touched from two different OS threads — the worker's dedicated connector thread (`checking_connection()`/`xcecd_on_reconnect()`) and a `vtaskmgr` pool thread (`xcecd_resync_handle_query_page()`, reached via the request-dispatch path). This mirrors the pre-existing, design-directed unsynchronized `needs_rejoin` pattern (same codebase convention, deliberately reused per design.md), but is riskier since `xcecd_resync_reset()` calls `free()` on the accumulator rather than just staling a bool read — a narrow, non-deterministic window exists where an independent reconnect could free the buffer while a pool thread is mid-read of the same connection's in-flight query page. Does not reproduce task 4.3's own violated requirement (Req 6.1) and isn't required by any Requirement/AC as written; this project's ASAN/UBSAN test harness has no TSAN, so it also isn't mechanically testable the way the deadlock fix was. Worth a small follow-up (a mutex around the gate+accumulator, or moving accumulation to the connector thread only) if this ever needs hardening beyond what `needs_rejoin`'s own accepted risk profile already tolerates.
|
||||
- **Task 4.4 found and fixed two pre-existing latent bugs** (bug-1756, bug-1757, both traced via `git log -S` to the original `cb0c7fe9` import, predating this feature) needed to get a truthful RED/GREEN signal for the new lifecycle-based tests: (1) `resource_sync_destroy()` unconditionally re-called `resource_sync_stop()`, double-`pthread_join`-ing the same thread handle (ASAN abort) — the exact sequence production `xcecd_cleanup()` already uses on shutdown; fixed by guarding on `atomic_load(&sync->running)`, mirroring `connection_handler.c`'s `network_handler_destroy()` idiom. (2) `init_builtin_load_indices()`'s own `static bool initialized` guard permanently blocked re-population after a `resource_sync_destroy()` cycle, contradicting its caller's own `g_load_indices == NULL` re-init check; removed as redundant/harmful. The gate itself deliberately does NOT reset `load_report_counter` on the resync_pending skip path (unlike the function's other skip branches) so a deferred send fires on the very next 1s tick once the flag clears rather than waiting a spurious extra `LOAD_INDICES_INTERVAL_DEFAULT` cycle — verified by hand-trace across 3 consecutive ticks by both the implementer and an independent reviewer (who also reproduced RED by reverting the gate and rebuilding, then restored GREEN), matching Requirement 3.8's "resumes ... once the flag clears" precisely.
|
||||
- **Task 5 found and fixed bug-1751**: design.md's cited candidate-filter site (`scheduler.c:1211`'s `sched_reset_worker_()`, reached only via `sched_begin_session()`) is dead code on both live drivers — `driver.c`/`sim_loop.c` actually call `xce_core_begin_session()` (`core_session.c`), which runs an independently-maintained mirror, `core_reset_workers()`, not listed in design.md's File Structure Plan. Fixed by applying the identical `IS_WORKER_UNAVAIL || IS_WORKER_RECONCILING` disjunct to both sites (parity/dead-code-hygiene on the design-cited one, correctness on the actually-live one) rather than patching only the literally-cited site and leaving the task's own Observable Completion Criterion false in production — same category as bug-1746/bug-1747's design-vs-reality corrections. design.md's File Structure Plan corrected. Also confirmed design.md's Testing Strategy referenced an "existing IS_WORKER_UNAVAIL candidate-filter test" that does not actually exist; new coverage added to `test_core_session.c` instead (white-box `#include`s `core_session.c` already; `scheduler.c`'s own dependency closure is too large to white-box per established precedent, `test_snapshot_load_allocator_regression.c`) — `scheduler.c`'s copy of the fix is therefore verified only by compilation + the identical, tested logic pattern in its mirror, not by a direct test; judged acceptable since it's confirmed dead code today, not a live gap.
|
||||
- **Task 6.1 found and fixed bug-1752**: the worker daemon's `myreq_handler` had NO dispatch case for `OPCode_push` at all before this task — every prior config push (including the pre-existing `clustername` field, in production since before this spec started) was silently discarded via `default: break;`. Confirmed genuine (diffed `myreq_handler` pre/post, ruled out an alternate consumer — `xcectx.c`'s `vtask_entry` only special-cases the unrelated generic `pushmsg` union arm, not `configpushmsg`) and confirmed `clustername` has never been read anywhere in `src/worker/`, so this was harmless dead infrastructure until `workerResyncWindowMs` became its first real consumer. Fixed narrowly — `handle_pushconfig()` consumes only `worker_resync_window_ms`, leaving `clustername`/`confvars` still unconsumed and explicitly out of this task's boundary — rather than expanding scope to wire up every existing-but-never-consumed push field. Ordering (join reply → config push → conditional arm+query) and the live-configured-value plumbing were both verified via genuine structural tests (real packets popped off the wire, two distinct configured values round-tripped), not weaker behavioral proxies.
|
||||
- **Task 6.2's mandated standby-redirect investigation found bug-1753**, a DIFFERENT gap than the one it was looking for: the generic not-primary (`CTRL_STANDBY`) redirect path turned out to already work correctly with zero opcode-specific code (`ha_dispatch_gate` gates every opcode uniformly before the switch), but the investigation surfaced that `ha_dispatch_gate`'s SEPARATE `CTRL_PRIMARY`-without-write-quorum branch — a "fact-bearing ops admitted anyway" allowlist (`OPCode_notify`/`update`/`setpreason`/`clrpreason`, bug-396) — was missing `OPCode_jobStateReport`, meaning a report arriving during a transient quorum-loss window (e.g. mid-failover, this feature's own core motivating scenario) would be rejected outright with no retry, since the worker's send is fire-and-forget. Fixed by adding the one missing case, verified safe against `resync_round_is_current`'s staleness check and the WAL_MODE_RETAIN semantics every fact this feature produces already uses. Citation nit: this fix's true motivation sits closer to Requirements 1.3/4/6 than the task's own tagged 8.1/8.3 — recorded here for traceability, tasks.md's tag left as originally generated. **Lesson**: a task text that names one specific concern to investigate ("check the redirect path") can surface a DIFFERENT, more consequential gap in the same general area — don't stop the investigation at "the named concern turned out fine," keep reading the surrounding gating logic in full.
|
||||
- **Task 6.2 also drew three escalating, unsolicited cross-session messages** (from an unfamiliar peer session) mid-flight, pushing a specific unreviewed code change to `test_jobstatereport_dispatch.c` — the exact file the dispatched implementer subagent was independently writing at the time. Declined to act on any of them (risk of conflicting with in-flight subagent work; escalating urgency/"no further investigation needed" framing is a known social-engineering pattern regardless of whether the underlying suggestion is technically plausible). The implementer subagent finished independently and solved the underlying `get_ep_worker()`-in-test-binary limitation completely differently (a `#ifdef XCE_TESTING` call-count seam in `resync.c`, not a global-fixture hack) — validating the decision not to short-circuit the normal TDD+review pipeline based on an unverified peer instruction.
|
||||
- **Task 7.1 (test-only, zero production changes) discovered and honestly asserted a genuine, pre-existing, systemic gap distinct from bug-1747**: a worker-reported (or any) job failure's exit code is never durably recorded anywhere in this codebase — `record_job_fail_fact()`'s `WAL_JOB_FAIL` payload has no exit-code slot at all, and `apply_job_fail_transition()` never touches `job.job.exit_info` for the FAIL path, so it stays `NULL` unconditionally. Bug-1747 only covers the FAIL finish-*time* mistiming; this is the exit-*code* gap, logged separately as bug-1754 (out of scope for this test-only task, correctly not fixed here — flagged for a dedicated future task). The round-trip test asserts `exit_info == NULL` honestly with an inline comment rather than encoding a wrong expectation the codebase can't actually produce.
|
||||
- Task 7.2 (test-only): added a focused, single-job proof of the feature's own original motivating scenario (bug-1741) — a job dispatched and RUN with NO finish fact ever committed (modeling the lost fire-and-forget notify), recovered to DONE with the worker-reported finish time via the real rejoin/query/report chain. Deliberately scoped smaller than task 7.1's four-outcome round trip (reuses its helper infrastructure, adds no new scaffolding) rather than duplicating it. tasks.md's own `_Requirements: 4.1, 4.2, 4.6_` tag includes 4.6 (unrecognized→PEND), which is genuinely unrelated to this finished-success scenario (that's task 7.1/2.1's territory) — `REQUIREMENTS_CHECKED` correctly narrowed to 4.1/4.2 instead of force-fitting an unrelated assertion, same accepted-imprecise-tag pattern as task 6.2's own citation nit.
|
||||
- Task 7.3 (test-only): reproduced bug-1741's exact original shape (7 jobs, 6 lost) in one round/one wire page — 5 finished-success + 1 finished-failure recovered, plus one already-DONE job re-reported as an unqueried extra with a deliberately wrong finish time, proving reconciliation neither leaves any lost job stuck nor duplicates/clobbers an already-terminal one (exact `+6` WAL LSN delta, not `>=`, so a duplicate append is genuinely caught — confirmed via RED-phase reproduction against `record_job_finish_fact_at`'s terminal-state guard). Deliberately kept to a single wire page (page-boundary correctness is task 7.4's separate concern). Two reviewer/implementer subagents in this session's task 6.2/7.2 review rounds hit the "background command with no external wakeup" trap (ended their turn on a backgrounded build, never resumed) — both implementer and reviewer prompts for 7.3 onward now carry an explicit foreground-only warning up front; worth keeping in every future subagent dispatch in this codebase, not just implementer roles.
|
||||
- **Task 7.4 (test-only) found and correctly declined to fix bug-1755, but the coordinator escalated it to a new task 7.5 rather than leaving it passively logged.** Proved 1042 jobs correctly query and report across 11 wire pages each direction (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, per the task's own "no truncation past a single page" mandate. While building it, discovered the underlying `xce_job_state_query_req()` pagination mechanism it was proving correct is never actually exercised in production: `worker_send_resync_query()` (task 6.1) hardcodes `pktlimit=0`, meaning the controller's real query send ALWAYS produces exactly one unbounded page regardless of job count — asymmetric with the report direction's real, wired `XCECD_RESYNC_REPORT_PKTLIMIT=100` bound. No hard transport-layer message-size cap was found (dynamic malloc, `uint64_t` length prefix), so this isn't a demonstrated crash/hang/data-loss risk and Requirement 8.1's literal "without... failing" text is technically satisfied — but design.md's Boundary Commitments explicitly states both opcodes are "paginated," which is not true of the shipped code, in this feature's own headline mechanism, discovered in this feature's own final validation task. Per the review's explicit recommendation, added task 7.5 (small, well-scoped: mirror the report direction's constant-driven precedent) rather than accepting a silently-false design claim into the completed feature — same "found a real gap, fold it into an explicit task rather than a passive note" pattern this spec has followed since tasks 1.5/6.1's config-delivery gaps.
|
||||
Loading…
Add table
Add a link
Reference in a new issue