Design validation (2026-09-01) found and fixed 4 issues: a stale bug-1678 citation blocking reuse of persist_submit_job_dispatch_fact, missing XCE_WORKER_RECONCILING accounting against scheduler-dispatch-commit's dispatch-fail retry/terminal-fail classification, bug-1631 reactivation (job.allocs never released on RUN->PEND reconciliation), and an eager-vs- deferred mutation inconsistency between the new job.c functions and the bug-1679 explicit-notify precedent. design.approved and tasks.approved both set; tasks.md drafted (7 major/19 sub-tasks) and passed two rounds of independent task-graph sanity review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4M5MJM2zK6nU43da267Ku
498 lines
68 KiB
Markdown
498 lines
68 KiB
Markdown
# 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); 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).
|
||
|
||
### 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` — a `NULL` alloc is explicitly the documented, tested backward-compatible case. `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.
|
||
- 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]
|
||
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 (two new functions, 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` — two new functions, both following `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` are called as-is for the finished-success/finished-failure cases. 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.
|
||
- `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_jobstate_report_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, tested, backward-compatible branch on both apply seams, so this is a thin caller into existing, proven code, 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_jobstate_report_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` (existing, reused as-is) | None — existing complete fact shape (already carries `finish_time`, exit info); the reconciliation caller passes the worker-reported `finishTimeMs` (Requirement 3.5) 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 |
|
||
| 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 branch bug-1678 already added and tested. Live mutation is eager (see Components), matching `dispatch_a_job_`'s own pattern for the same fact type | Already covered — both seams (`apply_committed_entry`, `apply_wal_entry`) were updated for this exact NULL-alloc case by bug-1678's fix; no new decode path to verify |
|
||
|
||
**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 seams, with an explicit, tested `NULL`-alloc backward-compatible case. The real reason this design still writes no allocation for Requirement 4.8 is simpler and unrelated to that now-closed 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 precisely because its NULL-alloc case is already the correct, tested representation of "state and host known, allocation not." 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`.
|