docs(job-array-single-wal-record): sync spec docs with phase-7 residual-gap closure
brief/requirements updated to reflect the reopened-and-now-closed post-GO gap-closure phase (tasks 7.1-7.3); adds research.md capturing the investigation trail; roadmap.md reflects current spec status.
This commit is contained in:
parent
53bd686897
commit
b469b174a2
4 changed files with 224 additions and 16 deletions
|
|
@ -29,7 +29,7 @@ A job-array submission persists, replicates, and pushes as a **single** Raft log
|
|||
|
||||
Single atomic array record on top of the post-decoupling apply seam:
|
||||
|
||||
- Define a `WAL_JOB_ARRAY_SUBMIT` record encoding base job spec + index range/count + sparse per-index overrides.
|
||||
- Define a `WAL_JOB_ARRAY_SUBMIT` record encoding base job spec + index range/count (no per-index overrides — array elements are uniform clones today, distinguished only by jobid; the client submit API has no per-index override field).
|
||||
- Command handler appends + commits this one record through a single barrier; on commit, the shared `apply_committed_entry()` seam expands it into N in-memory `job_t` entries (reusing `create_job` per element) under one apply step.
|
||||
- Restart replay decodes the single record and expands identically (same apply seam → no online/replay divergence).
|
||||
- Scheduler push delivers one array descriptor; scheduler expands locally.
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@
|
|||
|
||||
A job-array submission today fans out into one Raft log record per array element: `job_submit()` loops over every element calling `create_job()`, and the persistence layer commits each resulting job through its own independent majority-commit barrier. A 10,000-element array therefore produces 10,000 WAL appends, 10,000 sequential majority-commit round trips, and 10,000 scheduler pushes — amplifying commit-engine load, replication bandwidth, scheduler push volume, and failover replay time linearly with array size, while holding the submitting thread across every per-element barrier in sequence.
|
||||
|
||||
This feature makes a job-array submission persist, replicate, and push as a single atomic unit: one WAL record describing the whole array (base job specification, index range/count, and any sparse per-index overrides), committed through exactly one commit barrier. Standbys reconstruct the entire array from that one committed log position. The scheduler receives the array as a single push and expands it locally. After expansion, every array element retains its existing per-element identity and is addressable exactly as it is today for status, cancellation, and completion.
|
||||
This feature makes a job-array submission persist, replicate, and push as a single atomic unit: one WAL record describing the whole array (base job specification plus index range/count — every element shares the base spec, distinguished only by jobid), committed through exactly one commit barrier. Standbys reconstruct the entire array from that one committed log position. The scheduler receives the array as a single push and expands it locally. After expansion, every array element retains its existing per-element identity and is addressable exactly as it is today for status, cancellation, and completion.
|
||||
|
||||
This feature builds on the apply seam introduced by `raft-apply-decoupling` (now complete): the single array record is expanded into individual in-memory job entries inside `apply_committed_entry()`, the same seam that already handles single-job submission, so online apply and restart-replay expand identically with no divergence between the two paths.
|
||||
|
||||
|
|
@ -12,8 +12,8 @@ This feature builds on the apply seam introduced by `raft-apply-decoupling` (now
|
|||
|
||||
## Boundary Context
|
||||
|
||||
- **In scope**: a new WAL record format for a job-array submission (base spec + index range/count + sparse per-index overrides); atomic single-barrier commit of that one record; standby reconstruction of the full array from one committed LSN; the expand-on-apply step shared by online apply and restart replay; a single scheduler push carrying the array descriptor with local scheduler-side expansion; preservation of per-element identity and state addressing after expansion; idempotent re-application of a committed array record (replay or retry must not double-create elements); a bounded record size or chunking policy so a pathologically large array does not produce a single oversized WAL entry.
|
||||
- **Out of scope**: the apply-ordering/hook-decoupling refactor itself (delivered by `raft-apply-decoupling`, consumed here unchanged); any change to the non-array single-job submission path; scheduler dispatch/allocation policy; per-element override semantics beyond what single-job submission already supports; the commit engine's or commit-barrier's internal mechanics (consumed unchanged from `ha-commit-engine` / `ha-commit-barrier-rollout`).
|
||||
- **In scope**: a new WAL record format for a job-array submission (base spec + index range/count — every element shares the base spec, distinguished only by jobid); atomic single-barrier commit of that one record; standby reconstruction of the full array from one committed LSN; the expand-on-apply step shared by online apply and restart replay; a single push carrying the full array to each `HOOK_DETAIL_JOB_NEW` subscriber — scheduler (with local scheduler-side expansion) and API server; preservation of per-element identity and state addressing after expansion; idempotent re-application of a committed array record (replay or retry must not double-create elements); a bounded record size or chunking policy so a pathologically large array does not produce a single oversized WAL entry; log-based confirmation on the scheduler and API server that all array elements were received, for harness verification; hardening `src/lib/jobj.c`'s job/job-submit construction primitives (`job_init`, `makeJobsub`) so they are reliable and leak-free under N-element array construction, since Requirement 9's deterministic-apply guarantee depends on them.
|
||||
- **Out of scope**: the apply-ordering/hook-decoupling refactor itself (delivered by `raft-apply-decoupling`, consumed here unchanged) — `persist_job_hook_fun`'s `WAL_JOB_SUBMIT` branch is already a no-op (task 3.1/6.1: commit is invoked explicitly by `handle_jobsubmit`, never from this hook, for either single-job or today's per-element array submission); this spec replaces the explicit per-element commit loop (`submit_job_wal_entry_and_commit`, called once per array element today) with one array-shaped commit call — the hook plays no role in submission before or after this change, and stays untouched for its other live purpose (job-state facts originating from `job_ctrl`'s direct synchronous path); any change to the non-array single-job submission path; scheduler dispatch/allocation policy; per-element override semantics beyond what single-job submission already supports; the commit engine's or commit-barrier's internal mechanics (consumed unchanged from `ha-commit-engine` / `ha-commit-barrier-rollout`); bulk query/cancel of a whole array as one client operation — deferred to a future spec because array elements can be in heterogeneous states at request time (done/exited elements need no action, running elements need a worker kill signal, pending elements need a direct state transition without ever running), which is a materially different bulk-operation problem from atomic array submission and deserves its own requirements rather than scope creep here.
|
||||
- **Adjacent expectations**: depends on `raft-apply-decoupling`'s `apply_committed_entry()` seam existing and being the single point where a committed WAL entry becomes in-memory state (now complete); depends on `ha-commit-engine` / `ha-commit-barrier-rollout` for one-record atomic commit and idempotency guards, consumed unchanged; the scheduler-side push path is the one `scheduler-controller-msg-gap` already established, extended here to carry an array descriptor instead of only single-job pushes.
|
||||
|
||||
## Requirements
|
||||
|
|
@ -23,19 +23,18 @@ This feature builds on the apply seam introduced by `raft-apply-decoupling` (now
|
|||
**Objective:** As a cluster operator submitting a large job array, I want the whole array persisted as one Raft log record, so that submission cost no longer scales linearly with array size.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When a job-array submission is accepted, the Controller shall encode the entire array — base job specification, index range/count, and any sparse per-index overrides — into a single WAL record.
|
||||
1. When a job-array submission is accepted, the Controller shall encode the entire array — base job specification plus index range/count — into a single WAL record. Every array element shares the same base job specification; the only per-element distinction is its jobid (base id + index).
|
||||
2. The Controller shall commit that single WAL record through exactly one majority-commit barrier, regardless of the array's element count.
|
||||
3. The Controller shall not append a separate WAL record per array element for a job-array submission.
|
||||
4. When array elements share the same base specification with no per-index overrides, the encoded record shall not need to enumerate each element individually.
|
||||
|
||||
### Requirement 2: Atomic, all-or-nothing commit
|
||||
|
||||
**Objective:** As a cluster operator, I want a job-array submission to commit as a single unit, so that a standby can never observe a partial array.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When the array's WAL record is committed on a majority, the Controller shall consider the entire array committed as one unit — there shall be no state in which some array elements are committed and others are not.
|
||||
2. If the array's WAL record fails to reach majority commit, then the Controller shall treat the whole array submission as not committed and shall not construct any array elements from it.
|
||||
3. A standby reconstructing committed state shall never observe a job-array submission in a partially-applied form.
|
||||
1. When a single array WAL record is committed on a majority, the Controller shall consider every element described by that record committed as one unit — there shall be no state in which some elements of that record are committed and others are not. (When Requirement 7's chunking policy splits a pathologically large array into multiple records, this atomicity guarantee applies per committed record/chunk, not across the whole array — Requirement 7.2 defines the atomicity unit in that case.)
|
||||
2. If an array WAL record fails to reach majority commit, then the Controller shall treat that record's elements as not committed and shall not construct any array elements from it.
|
||||
3. A standby reconstructing committed state shall never observe a single array WAL record in a partially-applied form.
|
||||
|
||||
### Requirement 3: Shared expand-on-apply for online and replay
|
||||
|
||||
|
|
@ -45,6 +44,7 @@ This feature builds on the apply seam introduced by `raft-apply-decoupling` (now
|
|||
1. When a committed job-array WAL record is applied — whether during live/online apply or during restart replay — the Controller shall expand it into individual job entries through the same `apply_committed_entry()` seam used for single-job submission.
|
||||
2. The Controller shall use identical expansion logic for the online-apply path and the restart-replay path, so that replaying a committed array record produces byte-for-byte the same set of job elements as the original online apply produced.
|
||||
3. When re-applying an already-applied array record (e.g. during replay after a restart, or a retried commit), the Controller shall not create duplicate job elements for any array index.
|
||||
4. The array-expand step shall fire `HOOK_DETAIL_JOB_NEW` exactly once per committed array record, carrying all of that record's constructed job entries in one list — not once per element. (This is what Requirement 6's single-push guarantee to the scheduler and API server rests on — both are subscribers of this same hook fire.)
|
||||
|
||||
### Requirement 4: Standby reconstruction from one committed position
|
||||
|
||||
|
|
@ -64,14 +64,20 @@ This feature builds on the apply seam introduced by `raft-apply-decoupling` (now
|
|||
2. The Controller shall support querying, cancelling, and completing an individual array element by its per-element identity exactly as it does for job elements created via the current per-element path.
|
||||
3. This feature shall not change any client-visible identity, status, or addressing behavior for an individual array element.
|
||||
|
||||
### Requirement 6: Single scheduler push with local expansion
|
||||
### Requirement 6: Single push to scheduler and API server, with local expansion
|
||||
|
||||
**Objective:** As a maintainer of scheduler load, I want the scheduler to receive one push per committed array, so that scheduler push volume no longer scales linearly with array size.
|
||||
**Objective:** As a maintainer of scheduler and API-server load, I want both the scheduler and the API server to receive one push per committed array, so that push volume to either consumer no longer scales linearly with array size.
|
||||
|
||||
**Context:** `HOOK_DETAIL_JOB_NEW` (`HOOK_AFTER_EVENT`) is a single hook fired once per constructed job entry, with two independent subscribers: `sched.c` (`xcesched_push_joblist`) and `apiserver.c` (`apiserver_push_joblist`). Today both fire once per array element, N times per array. Batching the hook fire to once per array (Requirement 3's shared apply step) fixes both consumers by construction — this requirement holds both to the same bar explicitly, so a fix that batches only the scheduler side and leaves the API server side unbatched does not satisfy it.
|
||||
|
||||
**Wire shape note:** "single push" means the push carries the array descriptor (base spec + index range/count + jobid basis) — the same compact shape as the WAL record (Requirement 1) — not N individually-encoded full job structs bundled into one message. Encoding N full duplicate structs would satisfy "one push" by message count while silently reintroducing N-scaling in payload size, since the wire encoder does not know two `Text` fields share the same interned source and will encode each independently. Each receiver decodes one base spec and expands it locally into N entries (Requirement 6.3 / Requirement 6.5), so sharing happens by construction on the receiving side too, with no separate interning step needed there.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When a job-array submission commits, the Controller shall deliver it to the scheduler as a single push carrying the array descriptor.
|
||||
2. The Scheduler shall expand the array descriptor into individual schedulable tasks locally, without requiring one push per element from the Controller.
|
||||
3. The Controller shall not send a separate scheduler push per array element for a job-array submission.
|
||||
1. When a single array WAL record commits, the Controller shall deliver it to the scheduler as a single push carrying that record's array descriptor (base spec + index range/count), not N individually-encoded job structs. (When Requirement 7's chunking policy applies, this is one push per committed record/chunk, not one push for the whole array — mirroring Requirement 3.4's per-record hook fire.)
|
||||
2. When a single array WAL record commits, the Controller shall deliver it to the API server as a single push carrying that record's array descriptor (base spec + index range/count), not N individually-encoded job structs. (Same per-record scoping as 6.1 when chunking applies.)
|
||||
3. The Scheduler shall expand the array descriptor into individual schedulable tasks locally, without requiring one push per element from the Controller. The scheduler shall treat each array element as an ordinary single job for scheduling purposes; scheduling/dispatch outcome for array elements is out of scope for this feature.
|
||||
4. The Controller shall not send a separate scheduler push, nor a separate API-server push, per array element for a job-array submission.
|
||||
5. The API server shall expand the array descriptor into individual per-element records locally (mirroring the scheduler's local expansion in 6.3), rather than the Controller sending or the API server requiring one full job struct per element on the wire.
|
||||
|
||||
### Requirement 7: Bounded record size for very large arrays
|
||||
|
||||
|
|
@ -86,6 +92,34 @@ This feature builds on the apply seam introduced by `raft-apply-decoupling` (now
|
|||
**Objective:** As a maintainer, I want this feature scoped strictly to array submissions, so that ordinary single-job submission and existing HA guarantees are unaffected.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. The Controller shall leave the non-array, single-job submission WAL path unchanged by this feature.
|
||||
1. The Controller shall leave the non-array, single-job submission WAL record format and commit-barrier count unchanged by this feature. (This does not prohibit Requirement 10's fixes to `jobj.c`'s shared construction primitives — `job_init`/`makeJobsub` are used by single-job construction too, and Requirement 10's bugfixes are expected to also improve, not regress, that path; "unchanged" here means the WAL wire format and commit-barrier behavior, not a blanket freeze on shared-library code this feature must touch for Requirement 9's correctness.)
|
||||
2. A full HA soak (sequential and chaotic `/xce-test-ha` regression), including job submission and failover, shall show no new job loss, divergence, or liveness stall attributable to this feature.
|
||||
3. Regression tests shall cover: WAL/commit-barrier count for a large-array submission (one record/one barrier, not N), failover replay time for a committed array, and correct per-element status/cancel/completion after array expansion.
|
||||
3. Regression tests shall cover: WAL/commit-barrier count for a large-array submission (one record/one barrier, not N), failover replay time for a committed array, correct per-element status/cancel/completion after array expansion, single-push delivery to both the scheduler and the API server, and a client retrying an identical array submission across a primary failover — the retry shall follow the same cross-primary dedup mechanism as single-job submission (bug-1419), extended to the array's full index range as one unit, and shall not double-submit or partially-duplicate the array.
|
||||
4. The Scheduler and the API server shall each log a single, identifiable confirmation that all N elements of a committed array were received and handled correctly (e.g. base jobid + element count), so that push-completeness for the whole array can be verified by log inspection without a new query API.
|
||||
|
||||
### Requirement 9: Deterministic, total apply — content failures rejected pre-commit, resource failures fenced not rejected
|
||||
|
||||
**Objective:** As a maintainer of cluster correctness, I want array expansion at apply time to never fail for a reason the commit already promised was safe, so that a committed array can never be un-appliable and no node can silently diverge from its peers.
|
||||
|
||||
**Context:** once an entry commits, Raft guarantees it is eventually applied on every correct server — but the apply function itself must be deterministic (every replica computes the same outcome from the same entry). Two distinct failure classes require two distinct handlings: (A) content-derived failures (jobid-range collision, malformed spec) are computable identically by every replica from the entry's own bytes plus prior state — these must be caught before the entry is ever appended/committed, never left to fail at apply time. (B) node-local resource exhaustion (e.g. OOM while constructing element 7000 of 10000) cannot be predicted at submit time and must not be allowed to vary apply's outcome across replicas — it is not a commit failure and must never cause element rejection or divergence.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. Before appending a job-array WAL record, the Controller shall validate everything that could deterministically prevent successful expansion of every element (jobid-range collision with existing or in-flight jobs, base-spec validity) — reusing the existing in-flight jobid-range claim mechanism (bug-1419), extended to cover the array's full index range in one check.
|
||||
2. If pre-commit validation fails, the Controller shall reject the submission before the WAL append — the array shall never be committed and shall never reach the apply stage.
|
||||
3. Once a job-array WAL record is committed, expansion at apply time shall not fail for any content-derived reason — pre-commit validation (9.1) shall make apply total for this class.
|
||||
4. If a node encounters a resource-exhaustion failure (e.g. allocation failure) while expanding a committed array record, that node shall not reject the array, shall not construct a partial/divergent set of elements, and shall not treat this as a reason to fail the commit. The node shall log the condition loudly, fence itself as behind on that LSN (non-promotable while fenced, per the existing `apply_divergent`/fence pattern), and keep retrying the apply until it succeeds — consistent with the "controller never self-terminates" invariant.
|
||||
5. A resource-exhaustion apply failure on one node shall not block other nodes that can successfully apply the entry from proceeding; the fenced node shall self-heal (clear its fence) once the apply succeeds.
|
||||
6. A fault-injection test shall force a resource-exhaustion failure (e.g. an injected allocation failure) during array expansion on one node and verify: the node fences itself as behind on that LSN, constructs no partial or divergent set of elements, other nodes proceed unaffected, and the fenced node self-heals (clears its fence, completes the apply) once the injected failure condition clears.
|
||||
|
||||
### Requirement 10: jobj.c hardening — the shared construction primitive must be reliable under array-scale reuse
|
||||
|
||||
**Objective:** As a maintainer of cluster correctness, I want `src/lib/jobj.c`'s job/job-submit construction primitives to be reliable and leak-free under N-element array construction, so that Requirement 9's "deterministic, total apply" guarantee rests on a sound foundation rather than a primitive with known gaps.
|
||||
|
||||
**Context:** `job_init` (jobj.c) already interns/shares `job_submit_data_t` across identical submissions via `makeJobsub()` (hash+refcount) — for N uniform array elements this collapses to one real allocation plus N cheap refcount bumps, which is the right foundation for Requirement 9. But inspection of the current implementation found gaps that predate this spec and become higher-stakes at array scale.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. `job_init` shall check `makeJobsub()`'s return value; on NULL (allocation failure or interning-table error), `job_init` shall fail cleanly (propagate an error) rather than store a NULL `job.submit` and report success. The same check shall apply to `makeString(src->ckptinfo)`.
|
||||
2. The root cause of the previously observed `makeJobsub()` NULL-with-EINVAL failure (returned even for a valid non-NULL `src`, traced to a likely `jobsub_table` initialization-ordering issue) shall be identified and fixed, not merely papered over by 10.1's error propagation.
|
||||
3. The array-expansion construction path shall not rely on a pattern that triggers `job_init`'s destructive `MOVE_PTR` fields (`pidinfo, allocs, accounts, run_usage, exit_info, pend_reasons`) more than once against a shared source struct. Either confirm these fields are always NULL for a freshly-submitted (not-yet-started) job — making repeated MOVE_PTR from a shared template benign — or construct each array element from its own independent source struct.
|
||||
4. The `job_destroy` leak on `accounts` (marked `/* FIXME??? */`, never freed) shall be fixed. Array submission multiplies this leak by element count; it shall not remain unfixed while this spec optimizes everything else for array scale.
|
||||
5. Whether `decode_Job` ever populates `ckptinfo` for a freshly-submitted job shall be verified; if it does, `free_Job`'s coverage shall be extended to include it (currently undocumented/unclear whether it is freed).
|
||||
|
|
|
|||
172
.kiro/specs/job-array-single-wal-record/research.md
Normal file
172
.kiro/specs/job-array-single-wal-record/research.md
Normal file
|
|
@ -0,0 +1,172 @@
|
|||
# Research & Design Decisions: job-array-single-wal-record
|
||||
|
||||
---
|
||||
**Purpose**: Capture discovery findings, architectural investigations, and rationale that inform `design.md`.
|
||||
---
|
||||
|
||||
## Summary
|
||||
- **Feature**: `job-array-single-wal-record`
|
||||
- **Discovery Scope**: Extension (existing system) — the controller's job-submit and apply-seam machinery already exists (`raft-apply-decoupling`, complete); this feature adds an array-shaped WAL record and expansion step on top of it.
|
||||
- **Key Findings**:
|
||||
- The brief's premise ("`job_submit()` loops calling `create_job()` per element") is stale. `raft-apply-decoupling` task 3.2 already removed that loop: `handle_jobsubmit()` today builds N lightweight `xce_job_t` WAL payloads (`build_job_submit_payloads`, job.c:221) and appends N separate `WAL_JOB_SUBMIT` entries via `persist_submit_job_payloads_async()` (persist.c:4846), each with its own commit continuation, batched into **one** client reply (`jobsubmit_async_ctx_t`, job.c:754). `create_job()` itself now runs only inside the apply seam (`restore_job()`, job.c:271). This feature's real job is collapsing those N WAL appends into one, not removing a `create_job()` loop that no longer exists.
|
||||
- The client-facing `JobSubmit` capnp struct (`controller.capnp:533`) already carries exactly `(jobid, arraySize, submitData)` — the compact shape Requirement 1 needs for the new WAL record. No new capnp struct is needed for encoding; a new `wal_entry_type` (`WAL_JOB_ARRAY_SUBMIT`) is needed so the existing single-job `WAL_JOB_SUBMIT`/`struct Job` format stays byte-for-byte unchanged (Requirement 8.1).
|
||||
- Because array elements are uniform clones (locked earlier in requirements discovery — no per-index overrides), the new record's encoded size is **O(1)** in array size: one `JobSubmitData` regardless of whether `array_size` is 2 or 32767. Combined with the pre-existing hard cap `arraySize <= MAX_ARRAY_IDX + 1` (32768) enforced at jobid-minting time (`idgen()`, idgen.c:31, called from the Go apiserver before a request ever reaches the controller), Requirement 7's "bounded record size" is satisfied by construction — no chunking policy needs to be built.
|
||||
- `apply_committed_entry()` (persist.c:2721) is not the *only* live dispatcher of `WAL_JOB_SUBMIT`. `apply_wal_entry()` (persist.c:2574) is a second, hook-free dispatcher that remains the live standby catch-up/replicated-apply path (confirmed by an explicit comment at persist.c:3722-3725: "`apply_wal_entry()` ... is NOT retired by this change; it remains the live standby catch-up/replicated-apply dispatch used elsewhere (ha.c)"). Today's codebase avoids divergence between these two dispatchers by sharing one primitive, `restore_job()`, called from both switch statements. This feature must follow the same pattern with a new `restore_job_array()` shared primitive, not add expansion logic to only one dispatcher.
|
||||
- The scheduler/apiserver push infrastructure already accepts an N-element `xlist_t` in one hook fire (`sched_job_new_fun`/`apiserver_job_new_fun` unconditionally call `xcesched_push_joblist`/`apiserver_push_joblist` with whatever list they receive) — so "one push call" is already achievable for free. But Requirement 6's wire-shape note explicitly demands more: the wire payload itself must stay compact (a descriptor), not N individually-encoded `Job`/`SchedJob` structs, because the existing `List(Job)`/`List(SchedJob)` wire shape re-introduces O(N) payload size even inside a single RPC. This requires a genuinely new compact push message and a new discriminator (`xlist->count > 1`) in the two existing hook handlers, plus new local-expansion logic on the scheduler (`src/scheduler/src/handler.c:112`, `case Data_data_schedjobs:`) and the Go apiserver (`src/frontend/apiserver/backend/convert.go`, sibling to `cJobToModel`).
|
||||
- `persist_apply_pump_run()` (persist.c:3244) currently **discards** `apply_committed_entry()`'s return value (`(void)apply_committed_entry(...)`, persist.c:3281) and unconditionally advances `last_applied_lsn`. A resource-exhaustion failure mid-array-expansion would, under today's code, be silently skipped forever — the array would never be constructed and never be retried. This is a genuine, load-bearing gap this feature must close (not a hypothetical): the pump must start checking the return value and gate `last_applied_lsn` advancement on it, mirroring the existing `apply_pump_read_entry` failure branch immediately above it.
|
||||
- No existing promotion-gate condition considers `last_applied_lsn` vs `committed_lsn` at all (`ha.c` gates only on `truncated`, `durable_floor`, `apply_verdict == APPLY_DIVERGENT`, `self_restart_pending`). A node stuck retrying a resource-exhaustion apply failure would look fully promotable today. A new, narrowly-scoped live fence is required — deliberately **not** reusing `recovery_unrecoverable` directly, because that flag's existing periodic auto-clear (`ha.c:1069-1071`, gated on WAL *durability* catch-up, not *apply* progress) would wrongly clear a fence whose underlying condition (stuck apply) is unrelated to durability and has not actually resolved.
|
||||
- `jobj.c`'s `job_init` (jobj.c:532) does not check `makeJobsub()`'s return value before storing it (jobj.c:543) — a real, pre-existing gap, N×-amplified by this feature's tight per-array `create_job()` loop. `job_destroy`'s `accounts` leak (jobj.c:584, `/* FIXME??? */`) is likewise pre-existing and N×-amplified. Both are already in scope via Requirement 10.
|
||||
|
||||
## Research Log
|
||||
|
||||
### Job-array submission path: brief vs. live code
|
||||
- **Context**: the brief (written before `raft-apply-decoupling` fully landed) describes a `for` loop over `create_job()` in `job_submit()`. Needed to confirm this is still accurate before designing an insertion point.
|
||||
- **Sources Consulted**: `src/controller/job.c` (live read, lines 196-1076), git history reference in code comments (commit `6f1fa97`).
|
||||
- **Findings**: The loop was removed by task 3.2. Current path: `handle_jobsubmit()` → `build_job_submit_payloads()` (builds N plain `xce_job_t` payload structs, no RTTI objects) → `persist_submit_job_payloads_async()` (N WAL appends, N commit continuations sharing one `jobsubmit_async_ctx_t`) → each continuation hands off to `persist_register_pending_apply()` → the apply pump's `apply_committed_entry()` constructs the actual `job_t` via `restore_job()`/`create_job()`, one per committed entry.
|
||||
- **Implications**: The N-scaling this feature must eliminate is in WAL appends/commit continuations (job.c/persist.c), not in a `create_job()` loop inside `handle_jobsubmit` — that loop is already gone. Design must replace `build_job_submit_payloads` + `persist_submit_job_payloads_async`'s per-element loop with a single array-shaped append, and add the N-way expansion loop to the apply seam instead (where `create_job()` already lives today).
|
||||
|
||||
### WAL wire format and capnp schema
|
||||
- **Context**: Requirement 1 needs "base spec + index range/count" as one record; needed to know whether new capnp schema is required.
|
||||
- **Sources Consulted**: `src/schema/controller.capnp` (`struct Job` line 287, `struct JobSubmit` line 533), `src/include/wal_types.h`.
|
||||
- **Findings**: `struct JobSubmit $C.mapname("xce_job_submit_t")` already has exactly `(jobid, arraySize, submit: JobSubmitData)` — the client request format. `struct Job` (the current WAL_JOB_SUBMIT payload) has no array fields and is single-job only.
|
||||
- **Implications**: Reuse `struct JobSubmit`'s existing capnp encoder (`encode_JobSubmit`, already generated) as the new WAL_JOB_ARRAY_SUBMIT payload verbatim — zero schema changes for the WAL format itself. Gate on a new `wal_entry_type` value so the single-job `WAL_JOB_SUBMIT`/`struct Job` format is untouched (Requirement 8.1).
|
||||
|
||||
### Record size bound (Requirement 7)
|
||||
- **Context**: needed to determine whether a chunking policy must be built.
|
||||
- **Sources Consulted**: `src/include/config.h` (`N_ARRAY_IDX_BITS`, `MAX_ARRAY_IDX`), `src/lib/idgen.c` (`idgen()`), `src/frontend/apiserver/backend/clib.go` (`GenerateJobID`).
|
||||
- **Findings**: `idgen()` rejects `arraySize > MAX_ARRAY_IDX + 1` (32768) at jobid-minting time, called from the Go apiserver before any submission reaches the controller (clib.go:165) — a request whose `array_size` exceeds this cap never gets a jobid and never reaches `handle_jobsubmit`. Combined with the record's O(1) payload (one `JobSubmitData`, not per-element), the WAL_JOB_ARRAY_SUBMIT record's byte size is already bounded and does not grow with `array_size` at all.
|
||||
- **Implications**: No new chunking mechanism is needed. Requirement 7.1/7.2 are satisfied by (a) reusing the compact `JobSubmit` shape (Requirement 1's own design decision) and (b) citing the pre-existing `MAX_ARRAY_IDX` cap as the hard upper bound on element count. Building "multiple atomically-committed chunks" machinery for a scenario that cannot occur would be scope creep (design-synthesis Simplification lens).
|
||||
|
||||
### Apply-seam dispatch: two live dispatchers, not one
|
||||
- **Context**: Requirement 3 demands one shared expansion path for online apply and replay; needed to confirm `apply_committed_entry()` is really the only live dispatcher.
|
||||
- **Sources Consulted**: `src/controller/persist.c` (`apply_wal_entry`, line 2574; `apply_committed_entry`, line 2721; `persist_recover`'s replay loop, line ~3742), `.kiro/specs/raft-apply-decoupling/design.md`.
|
||||
- **Findings**: `apply_wal_entry()` was NOT retired by `raft-apply-decoupling` — an explicit in-code comment (persist.c:3722-3725) confirms it "remains the live standby catch-up/replicated-apply dispatch used elsewhere (ha.c)". `persist_recover()`'s own replay loop was switched to call `apply_committed_entry()` (persist.c:3742), but `apply_wal_entry()` is a second, separate switch statement over the same WAL types, still reachable from `ha.c` for standby catch-up. Both dispatchers avoid divergence today only because they both delegate the actual single-job construction to one shared helper, `restore_job()`.
|
||||
- **Implications**: the array-expansion loop must live in one new shared helper (`restore_job_array()`, sibling to `restore_job()`), called from a new `WAL_JOB_ARRAY_SUBMIT` case added to **both** `apply_committed_entry()` and `apply_wal_entry()`'s switch statements — matching the codebase's existing (not the aspirational) architecture. Recorded as a correction to Requirement 3's framing: the shared seam is the helper function, not either dispatcher alone.
|
||||
|
||||
### Live apply-pump failure handling gap
|
||||
- **Context**: Requirement 9.4/9.5 need "keep retrying the apply until it succeeds" and "fence non-promotable" for a resource-exhaustion failure during array expansion.
|
||||
- **Sources Consulted**: `src/controller/persist.c` (`persist_apply_pump_run`, line 3244), `src/controller/ha.c` (promotion-gate call sites, lines ~1069 and ~1344).
|
||||
- **Findings**: `persist_apply_pump_run()`'s dispatch loop discards `apply_committed_entry()`'s return value (`(void)apply_committed_entry(...)`, persist.c:3281) and unconditionally advances `last_applied_lsn`. The read-failure branch immediately above it (`apply_pump_read_entry` failing) DOES stop the walk and leave `last_applied_lsn` unadvanced for retry — but nothing analogous exists for an `apply_committed_entry()`-internal failure. Separately, `ha.c`'s promotion-eligibility gate never inspects `last_applied_lsn` vs `committed_lsn` at all; it gates on WAL durability/truncation/apply-divergence/self-restart-pending only.
|
||||
- **Implications**: two concrete, necessary fixes, both required for Requirement 9 to hold, neither optional: (1) the pump must check `apply_committed_entry()`'s return value and mirror the existing read-failure branch (stop, don't advance, retried next kick) on a negative result; (2) a new, narrow live fence must gate promotion on "no LSN is currently stuck failing to apply" — deliberately not reusing `recovery_unrecoverable` (see next entry).
|
||||
|
||||
### Why not reuse `recovery_unrecoverable` for the live apply fence
|
||||
- **Context**: `recovery_unrecoverable` + `persist_recover_loud_fence_if_unrecoverable()` already implement the exact "loud, non-promotable, stay alive, keep retrying, self-heal" shape Requirement 9.4/9.5 wants — first instinct was to reuse it directly.
|
||||
- **Sources Consulted**: `src/controller/ha.c` (both promotion-gate call sites), `src/include/persist.h` (`recovery_unrecoverable` field doc comment, `APPLY_DIVERGENT` handling).
|
||||
- **Findings**: `recovery_unrecoverable` already has two distinct existing consumers with different clear semantics: (a) a periodic/timer-driven clear (`ha.c:1069-1071`) gated purely on WAL *durability* catch-up (`durable_floor >= target_hwm`), unrelated to apply progress; (b) `apply_verdict == APPLY_DIVERGENT`, a **separate** field, explicitly documented as "only ever recomputed by a fresh boot-time `persist_recover()` replay" and never live-cleared. If the new live apply-resource fence reused `recovery_unrecoverable` directly, condition (a)'s periodic clear would fire as soon as WAL durability caught up — which happens independently of whether the stuck array LSN has actually finished applying — silently un-fencing a node that is still failing to construct the array.
|
||||
- **Implications**: introduce a new, narrowly-scoped field (design.md calls it the Live Apply Resource Fence) instead of overloading `recovery_unrecoverable`. Set by the pump on an `apply_committed_entry()` failure; cleared by the pump itself the moment a retry of that same LSN succeeds — no dependency on the election/promotion-gate call sites ever running, so a long-lived primary that never loses leadership still self-heals (Requirement 9.5).
|
||||
|
||||
### Scheduler/apiserver compact push wire shape
|
||||
- **Context**: Requirement 6's wire-shape note explicitly rules out "N individually-encoded full job structs bundled into one message" — needed to confirm today's push path does exactly that despite already being list-shaped.
|
||||
- **Sources Consulted**: `src/controller/sched.c` (`sched_job_new_fun` line 271, `xcesched_push_joblist` line 14), `src/controller/apiserver.c` (`apiserver_job_new_fun` line 127, `apiserver_push_joblist` line 97), `src/schema/controller.capnp` (`struct Data` union, line 355; `SchedJobData`/`JobData`, lines 306-323), `src/scheduler/src/handler.c` (`case Data_data_schedjobs:`, line 112), `src/frontend/apiserver/backend/convert.go` (`cJobToModel`, line 31).
|
||||
- **Findings**: both `sched_job_new_fun` and `apiserver_job_new_fun` unconditionally call their respective `..._push_joblist(c, l)` regardless of `l->count` — for an N-element list (which the new array apply case will produce) this would encode N full `Job`/`SchedJob` capnp structs on the wire, reintroducing exactly the O(N) payload growth Requirement 6 rules out. `struct Data`'s `data :union $C.mapuniontag("table")` (controller.capnp:356) is an additive discriminated union already carrying 6 variants (`schedjobs`, `schedworkers`, `jobs`, `workers`, `resources`, `resvals`) — adding one more compact variant is the established, low-risk extension pattern (matches `scheduler-controller-msg-gap`'s own precedent).
|
||||
- **Implications**: `sched_job_new_fun`/`apiserver_job_new_fun` need a branch on `l->count > 1` (the only way `apply_committed_entry`'s `HOOK_DETAIL_JOB_NEW` fire ever produces a multi-element list, per Requirement 3.4) that pushes a new compact `Data` union variant (reusing `JobSubmit`'s shape again — base jobid + array_size + one `JobSubmitData`) instead of the existing per-job list. The scheduler and Go apiserver each need one new local-expansion routine, mirroring the controller's own `base_jobid + i` arithmetic.
|
||||
|
||||
## Architecture Pattern Evaluation
|
||||
|
||||
| Option | Description | Strengths | Risks / Limitations | Notes |
|
||||
|--------|-------------|-----------|---------------------|-------|
|
||||
| New `WAL_JOB_ARRAY_SUBMIT` type + reused `JobSubmit` capnp shape | Add one new `wal_entry_type`, reuse the existing `JobSubmit` struct as its payload | Zero capnp schema change; keeps `WAL_JOB_SUBMIT`/`struct Job` untouched (Req 8.1) by construction; symmetric with client wire format | New type must be added to two live dispatch switches (`apply_committed_entry`, `apply_wal_entry`) and to `wal_classify` | Selected |
|
||||
| Overload `WAL_JOB_SUBMIT` with a variable-shape payload (array flag + optional range) | Single WAL type handles both single-job and array | Fewer enum values | Directly violates Requirement 8.1 ("leave the non-array WAL record format... unchanged"); forces every existing single-job decode site to branch defensively | Rejected |
|
||||
| Chunk large arrays into multiple committed records | Bounds record size explicitly via a chunking policy | Handles unbounded `array_size` | Unnecessary: `array_size` is already hard-capped at 32768 by `idgen()`, and the record is O(1) regardless of count — chunking machinery would have no scenario to ever trigger | Rejected (Requirement 7 satisfied by construction instead) |
|
||||
|
||||
## Design Decisions
|
||||
|
||||
### Decision: Reuse `JobSubmit` as the WAL_JOB_ARRAY_SUBMIT payload and the compact push payload
|
||||
- **Context**: Requirement 1 needs a compact WAL shape; Requirement 6 needs a compact push shape "the same compact shape as the WAL record."
|
||||
- **Alternatives Considered**:
|
||||
1. Define a new `JobArraySubmit` capnp struct duplicating `JobSubmit`'s fields.
|
||||
2. Reuse `struct JobSubmit` verbatim for both the WAL payload and the push payload.
|
||||
- **Selected Approach**: (2) — one capnp struct, three uses (client request, WAL payload, push payload), one encoder/decoder pair already generated.
|
||||
- **Rationale**: `JobSubmit` already has exactly the three fields every one of these three uses needs (`jobid` as base, `arraySize`, one `submit: JobSubmitData`); requirements.md's own wire-shape note for Requirement 6 explicitly asks for "the same compact shape as the WAL record," making this the requirement-mandated answer, not just a simplification.
|
||||
- **Trade-offs**: none identified — no functionality is given up by reuse here.
|
||||
- **Follow-up**: confirm at implementation time that `encode_JobSubmit`/`decode_JobSubmit` (capnp-generated) round-trip cleanly when `submit` is a shared/interned pointer across all three use sites.
|
||||
|
||||
### Decision: Array expansion lives in a new shared helper, called from both live dispatchers
|
||||
- **Context**: `apply_wal_entry()` (standby catch-up) and `apply_committed_entry()` (primary live apply + boot replay) are both live and both currently delegate single-job construction to `restore_job()`.
|
||||
- **Alternatives Considered**:
|
||||
1. Add the expansion loop only inside `apply_committed_entry()`, leaving `apply_wal_entry()` unchanged.
|
||||
2. Add a new shared `restore_job_array()` helper, called from a new case in both switches.
|
||||
- **Selected Approach**: (2).
|
||||
- **Rationale**: Option (1) would silently leave standby catch-up unable to expand an array record, reintroducing exactly the online/replay divergence Requirement 3 exists to prevent — for the array case specifically, on day one.
|
||||
- **Trade-offs**: one more call site to keep in sync (mitigated: both call sites do nothing but forward to the shared helper, so there is no logic to keep in sync, only a switch-case line).
|
||||
- **Follow-up**: none — this mirrors `restore_job()`'s existing, already-proven sharing pattern exactly.
|
||||
|
||||
### Decision: A new, narrow live apply-resource fence, not a reuse of `recovery_unrecoverable`
|
||||
- **Context**: Requirement 9.4/9.5 need a live, self-healing, non-promotable fence for a resource-exhaustion apply failure.
|
||||
- **Alternatives Considered**:
|
||||
1. Set `recovery_unrecoverable` directly from the live apply-pump failure path.
|
||||
2. Introduce a new, dedicated field with its own set/clear triggers.
|
||||
- **Selected Approach**: (2).
|
||||
- **Rationale**: `recovery_unrecoverable`'s existing periodic clear (ha.c:1069-1071) is gated on WAL durability catch-up, a condition orthogonal to whether the stuck array LSN has actually finished applying — reusing it would risk a premature, incorrect un-fence.
|
||||
- **Trade-offs**: one more boolean/condition threaded through the two promotion-gate call sites in `ha.c`; justified by correctness.
|
||||
- **Follow-up**: verify at implementation time that both `ha.c` promotion-gate call sites (periodic release, election-time) are updated together — a design.md Revalidation Trigger.
|
||||
|
||||
## Risks & Mitigations
|
||||
- **Apply-pump return-value fix is a behavior change on an existing, heavily-tested path** (every WAL type's apply failure, not just arrays, now halts the walk instead of silently skipping) — Mitigation: today's `apply_committed_entry()` implementations never actually return a negative value in practice for any existing type (verified during discovery), so this fix is a no-op for all pre-existing call paths and only activates for the new array-expansion failure case; full HA soak (Requirement 8.2) is the regression gate.
|
||||
- **Two live dispatchers (`apply_committed_entry`, `apply_wal_entry`) must both gain the new case** — Mitigation: shared helper (`restore_job_array`) means the only per-dispatcher code is one switch-case line; covered explicitly in File Structure Plan and Revalidation Triggers.
|
||||
- **`jobj.c` hardening (Requirement 10) touches a shared primitive used by every job construction, not just arrays** — Mitigation: scoped narrowly to the five specific gaps requirements.md names (NULL-check, table-init-ordering root cause, MOVE_PTR-reuse audit, `accounts` leak, `ckptinfo` coverage audit); no broader refactor.
|
||||
|
||||
## References
|
||||
- `.kiro/specs/raft-apply-decoupling/design.md` — the `apply_committed_entry()` seam contract this feature builds on; its own Revalidation Triggers already flag "any new WAL entry type must be added to the `apply_committed_entry()` dispatch table or it silently fails to apply" and "`job-array-single-wal-record` landing — builds its array-expand step directly on `apply_committed_entry()`."
|
||||
- `.kiro/specs/ha-commit-engine/`, `.kiro/specs/ha-commit-barrier-rollout/` — majority-commit and idempotency primitives consumed unchanged.
|
||||
- `.kiro/specs/scheduler-controller-msg-gap/` — established precedent for extending the controller→scheduler push protocol with a new `Data` union variant.
|
||||
- `src/controller/CLAUDE.md` — Process Lifecycle invariant ("a controller process MUST NEVER self-terminate... fail loud, fence, stay alive, keep retrying") governing the Live Apply Resource Fence's behavior.
|
||||
|
||||
---
|
||||
|
||||
# Gap Analysis: job-array-single-wal-record
|
||||
|
||||
**Note**: `design.md` and `tasks.md`-adjacent work for this spec already exist and have passed design review (GO, zero critical issues, 2026-07-13). This gap analysis is produced retroactively per explicit user request and is used here to confirm/cross-check the design's approach against the framework below, not to precede it.
|
||||
|
||||
## Requirement-to-Asset Map
|
||||
|
||||
| Requirement | Existing Asset | Gap |
|
||||
|---|---|---|
|
||||
| 1.1-1.3 (single WAL record/barrier) | `handle_jobsubmit()` (job.c:926), `build_job_submit_payloads()` (job.c:221), `persist_submit_job_payloads_async()` (persist.c:4846) — today's N-append, N-continuation path | **Missing** — no single-array-append path; `wal_entry_type` (wal_types.h:102) has no array variant |
|
||||
| 2.1-2.3 (atomic commit) | `commit_submit_async()` (commit engine, unchanged), `jobsubmit_async_ctx_t`/`pending` refcount (job.c:754, already generalizes to `pending=1`) | **Constraint** — commit-barrier atomicity is free (existing engine); *apply*-side atomicity (no partial `c->jobmgr` visibility) is not, since `c->jobmgr` insertion isn't natively transactional across N constructor calls |
|
||||
| 3.1-3.4 (shared expand-on-apply) | `apply_committed_entry()` (persist.c:2721), `apply_wal_entry()` (persist.c:2574) — **two** live dispatchers, both call `restore_job()` for the single-job case today | **Missing** — no `restore_job_array()`-equivalent; neither switch has a `WAL_JOB_ARRAY_SUBMIT` case |
|
||||
| 4.1-4.3 (standby reconstruction) | Replication streams whatever WAL bytes exist (repl.c, unchanged) — one record replicates as one entry by construction | **None** — falls out of 1+3 once built; no separate replication-layer work |
|
||||
| 5.1-5.3 (identity preservation) | `base_jobid + i` arithmetic (job.c:238), `idgen()` (idgen.c:22) reserving a contiguous range | **None** — verification-only; new expansion loop reuses this arithmetic identically |
|
||||
| 6.1-6.5 (compact push) | `sched_job_new_fun`/`apiserver_job_new_fun` (sched.c:271, apiserver.c:127), `Data` union (controller.capnp:355) — already N-capable by *call count*, not by *wire shape* | **Missing** — no compact `Data` union variant; no scheduler-side (`handler.c:112`) or apiserver-side (Go `backend/`) local-expansion logic |
|
||||
| 7.1-7.2 (bounded record size) | `idgen()`'s `MAX_ARRAY_IDX` cap (config.h:68, idgen.c:31), enforced upstream in the Go apiserver (clib.go:165) before a request reaches the controller | **None** — pre-existing cap plus the record's O(1) payload satisfies this by construction; no chunking policy needed |
|
||||
| 8.1-8.4 (non-regression) | Existing single-job path (job.c), existing `/xce-test-ha` harness | **Constraint** — `array_size == 1` must stay byte-for-byte identical; verified achievable by branching before any shared state touches |
|
||||
| 9.1-9.2 (pre-commit validation) | `inflight_range_try_add`/`jobid_range_exists` (job.c:90-212) — already range-shaped from bug-1419 | **None** — consumed unchanged, called once per array submission exactly as today |
|
||||
| 9.3-9.5 (deterministic apply / live fence) | `persist_recover_loud_fence_if_unrecoverable`/`recovery_unrecoverable` (persist.c:4259) — boot-replay-only fence shape | **Missing** — `persist_apply_pump_run()` (persist.c:3244) discards `apply_committed_entry()`'s return value (a real, load-bearing bug, not array-specific); no live-path fence exists |
|
||||
| 9.6 (fault injection) | This codebase's `#ifdef XCE_TESTING`-guarded test-only-symbol convention (structure.md) | **Missing** — no injection seam exists yet in any job-construction path |
|
||||
| 10.1, 10.4 (jobj.c NULL-check, leak) | `job_init()`/`job_destroy()` (jobj.c:532, 565) | **Missing** — both are concrete, located bugs with no fix yet |
|
||||
| 10.2 (jobsub_table ordering) | `jobsub_table_init()`/`job_obj_init()` (jobj.c:80) | **Unknown** — root cause suspected (init-ordering) but not confirmed; flagged as a task-level investigation in design.md, not pre-decided |
|
||||
| 10.3, 10.5 (MOVE_PTR safety, ckptinfo coverage) | `job_init()`'s MOVE_PTR fields, `decode_Job`/`free_Job` (generated code) | **None** — both verified as non-issues during design (freshly-calloc'd payloads are always NULL on these fields; `ckptinfo` has no wire field to leak) |
|
||||
|
||||
## Implementation Approach Options
|
||||
|
||||
### Option A: Extend Existing Components Only
|
||||
Every change lands inside the file that already owns that domain — `job.c` (submit), `persist.c` (apply/commit/pump), `sched.c`/`apiserver.c` (push), `ha.c` (promotion), `jobj.c` (construction) — with no new non-test files.
|
||||
- ✅ Matches `structure.md`'s "one domain per file" convention exactly; no domain boundary is invented.
|
||||
- ✅ Reuses `jobsubmit_async_ctx_t`, the in-flight range registry, and the commit engine wiring completely untouched.
|
||||
- ❌ `persist.c` is already one of the largest files in the component; this adds more surface to it.
|
||||
- ❌ `ha.c` gets touched for a narrow addition (2 conditions, 2 call sites) but still requires the full TLA+/protocol-verification checklist review (`src/controller/CLAUDE.md`) since it's HA-critical.
|
||||
|
||||
### Option B: New Standalone Component (e.g. a `job_array.c`)
|
||||
Isolate the array-specific logic (`restore_job_array`, array-submit helpers) into a new file.
|
||||
- ✅ Cleaner surface for unit-testing `restore_job_array()` in isolation.
|
||||
- ❌ `restore_job_array()` must still be called from `persist.c`'s two switch statements and share `persist_t`/`cluster_t` internals plus locking discipline (`apply_pump_lock`, `c->jobmgr`'s RTTI lock) — a separate file buys little real isolation given this tight coupling, and risks violating the "No Hidden Shared Ownership" design principle by either exposing currently-internal `persist.c` state or duplicating its lock semantics.
|
||||
- ❌ Array submission isn't a new *domain* in this codebase's terms — it's a new WAL record shape within the existing job domain — so a new file would be an organizational mismatch, not a genuine boundary.
|
||||
|
||||
### Option C: Hybrid (extend in place, new functions not new domain files)
|
||||
Extend the six existing files (Option A's list) plus `controller.capnp`, adding clearly-named new functions (`restore_job_array`, `persist_submit_job_array_async`, `xcesched_push_jobarray`, `apiserver_push_jobarray`, `persist_apply_resource_fenced`/`_mark`/`_clear`) rather than modifying existing function bodies destructively. New *files* are created only where isolation is genuinely warranted: test files (`test_job_array_apply.c`, `test_job_array_fence.c`, `test_jobj_hardening.c`).
|
||||
- ✅ Every new function has one clear, single-file home matching its existing domain — no fragmentation, no fake isolation.
|
||||
- ✅ Genuine isolation exactly where it earns its cost (tests).
|
||||
- ❌ Touches 8 existing files across 2 languages (C controller/scheduler, Go apiserver) plus 1 schema file — coordination surface is wide even though each individual touch is small.
|
||||
- ❌ The "two live dispatchers" fact (`apply_committed_entry` vs. `apply_wal_entry`) must be correctly carried into task generation and implementation by every task that touches either — a real risk if that fact isn't threaded through explicitly (mitigated: design.md's Revalidation Triggers and File Structure Plan both name both dispatchers explicitly).
|
||||
|
||||
## Effort & Risk
|
||||
|
||||
- **Effort: L (1-2 weeks)** — significant functionality (new WAL type, apply-seam expansion, HA promotion-gate change, two new push consumers across two languages, five jobj.c fixes) spanning multiple files and one cross-language boundary, but no single piece requires inventing a new architectural primitive — every piece composes an already-proven existing mechanism (commit engine, hook system, RTTI object system, `Data` union).
|
||||
- **Risk: Medium** — no unknown technology and no foundational primitive being invented from scratch, but two of the touch points are genuinely HA-sensitive (`ha.c`'s promotion gate, `persist_apply_pump_run()`'s return-value handling) where a subtle mistake has correctness consequences beyond this feature. Mitigated by the design's scratch-build atomicity discipline and by full `/xce-test-ha` regression being a stated, non-optional gate (Requirement 8.2). Not High: nothing here requires new consensus/replication mechanics — those are consumed unchanged.
|
||||
|
||||
## Recommendation
|
||||
|
||||
**Option C (Hybrid)** is the right fit and is what `design.md` already selected — confirmed by this analysis, not contradicted by it. Option A alone would have been viable too (the "new components" in Option C are functions, not files, in every case except tests), so in practice Option C and a disciplined Option A converge here; the distinction matters mainly for the test files, where isolation is unambiguously worth it.
|
||||
|
||||
**Research items carried forward** (already flagged in `design.md`'s own Risk notes, repeated here for gap-analysis completeness):
|
||||
- Requirement 10.2's `jobsub_table` initialization-ordering root cause — confirmed unknown at design time, scoped to a bounded task-level investigation with an explicit boundary note (if the fix turns out to be outside `jobj.c`, split into its own task rather than silently widening scope).
|
||||
|
|
@ -66,6 +66,7 @@ The protocol **drive** is NOT fully event-driven. Root cause: `peer_ctrl_msg_han
|
|||
- [ ] **admin-config-tool** — **PROMOTED from backlog to spec 2026-07-06** (`/kiro-discovery`; brief at `.kiro/specs/admin-config-tool/brief.md`). **NEW: `xceadmin` Python CLI** for cluster operators — builds and deploys cluster configuration via direct TCP+TLS capnp wire to controller (NOT through apiserver). Two subcommands: **(a) `config build`** — directory scan: parse `configs/{hosts,queues,resources,users}.yaml`, validate against JSON schemas (`configs/schema/*.schema.json`), serialize `ClusterConfig` capnp struct to `cluster.bin` with SHA-256; **(b) `config push`** — zero-config dev (default `localhost:9991`, same as xce_test), single `--controller` flag for production, auto-discovers peers + primary from controller reply, sends `OP_upgradeConfig`, displays `{versionTag,ackedCount,error}`. **Single dependency: `controller.capnp`.** Pure Python (pycapnp + yaml + jsonschema + click), open-source ready (`pip install`). The tool is the client-side trigger for `config-upgrade-snapshot`'s controller-side path. MUST validate against reserved name `"lost_and_found"` for queue/host (defence-in-depth). xcecli comparison: `xcecli` = user tool (HTTP/HTTPS → apiserver → CGO → controller, JWT auth); `xceadmin` = operator tool (direct TCP+TLS → controller, zero-config, same trust model as controller peers). Shared future: `xcecommon` Python package with capnp schema + wire protocol reusable by both tools. Dependencies: `controller.capnp` schema only. **Blocks: nothing — ships independently.** Controller-side upgrade snapshot path (`config-upgrade-snapshot`) remains blocked on `entity-bearing-snapshots` (pre-existing). Next: `/kiro-spec-requirements admin-config-tool`.
|
||||
- [ ] lightweight rejoin re-attach (failover optimization) — **recorded 2026-06-16; deferred (YAGNI).** Today, on controller failover a reconnecting daemon does a full re-join and apiserver/scheduler receive **full** state (e.g. the whole job list), not a delta; for a large cluster this is sizable but rare. Decision (libxce-failover design review): accept full-state rejoin — it is simple, correct, and still cheaper than a full-history replace with no snapshot (cf. IBM LSF). Future optimization: a session-token "re-attach" hello that lets the new primary skip re-pushing already-replicated state, or a delta-only state transfer keyed on the consumer's last-known committed LSN. Delta reconciliation is non-trivial (the primary must know what each consumer already holds). Promote only if failover-rejoin latency/bandwidth is **measured** as a problem at scale. Dependencies: libxce-failover (rejoin hook), component-failover-adoption (daemon rejoin content), worker-rejoin-resync (worker job state).
|
||||
- [→] job-array-single-wal-record — **PROMOTED to a spec 2026-06-24** (`/kiro-discovery`; brief at `.kiro/specs/job-array-single-wal-record/brief.md`; now tracked under the *Raft Apply-Ordering & Hook Decoupling* phase above, sequenced after raft-apply-decoupling). Original note retained for provenance: Enhance controller & scheduler so a job-array submission persists, replicates, and pushes to the scheduler as a **single WAL record** rather than one record per array element. Today a job array fans out to N per-job state changes, so a 10k-element array means 10k WAL appends + 10k replicated commits + 10k scheduler pushes — amplifying commit-engine load, replication bandwidth, and failover replay time linearly in array size. Scope sketch: (a) controller writes one WAL entry describing the array (base spec + index range/count + per-index overrides) committed atomically through the commit barrier; (b) replication carries the single record so standbys reconstruct the whole array from one committed LSN; (c) scheduler receives the array as one push and expands locally. Must preserve per-element identity/state addressing after expansion (status, cancel, completion still operate per task id). Dependencies: ha-commit-engine + ha-commit-barrier-rollout (single atomic commit), ha-consistency-invariants (replicated scheduling state). Spec advised before implementation; promote via `/kiro-spec-init "job-array-single-wal-record"` when ready.
|
||||
- [ ] bulk job-array query/cancel — **recorded 2026-07-13, deferred out of `job-array-single-wal-record` scope by design.** A single client operation to query or cancel an entire array (not one element at a time) is explicitly out of scope for `job-array-single-wal-record` — see that spec's Boundary Context. Reason: array elements can be in heterogeneous states at request time (done/exited elements need no action; running elements need a worker kill signal; pending elements need a direct state transition to cancelled without ever running), which is a materially different bulk-operation problem from atomic array *submission* and needs its own requirements (partial-failure semantics, atomicity vs best-effort per element, etc.) rather than being folded into the submission spec. Dependencies: `job-array-single-wal-record` (per-element identity/addressing this would operate over). Promote via `/kiro-discovery` once job-array-single-wal-record ships.
|
||||
- [x] ~~WAL group-commit batching~~ — **recorded 2026-06-17 as deferred; ABSORBED INTO ha-commit-durability 2026-06-18 (design-review reversal).** The `/kiro-validate-design` review surfaced that the highest-volume WAL traffic is `WAL_MODE_RETAIN` facts (job start/finish/fail/rusage) — not array-collapsible and not covered by `job-array-single-wal-record` — so a pure per-entry synchronous design would penalize the dominant write path. Decision (with user): **do group-commit now** — repurpose `fsync_worker` from a timer into a demand-driven flusher (batch one `fdatasync` across pending appends, advance `durable_lsn`, wake commit-eval) as part of ha-commit-durability rather than deferring. No longer a separate backlog item.
|
||||
- [ ] primary step-down-on-quorum-loss — optional CP follow-on noted in ha-consistency-invariants brief (writes already blocked on quorum loss; step-down is an availability/clarity optimization).
|
||||
- [ ] read consistency / leader leases — Raft §8 read-only optimization; status queries may currently observe stale or provisional state (documented deviation in ha-consistency-invariants).
|
||||
|
|
@ -75,6 +76,7 @@ The protocol **drive** is NOT fully event-driven. Root cause: `peer_ctrl_msg_han
|
|||
|
||||
Smaller items that likely don't warrant a full spec; pick up opportunistically or bundle into adjacent spec work.
|
||||
|
||||
- [ ] **investigate: job_ctrl direct-mutate path (admin suspend/resume) — migrate to commit-then-apply?** — **recorded 2026-07-13, surfaced during job-array-single-wal-record's `persist_job_hook_fun` investigation.** `raft-apply-decoupling` deliberately scoped `job_ctrl`'s `mark_job_update`/`mark_job_fail` (admin SIGCONT/SIGSTOP) OUT-OF-BOUNDARY (task 4.1/6.1 Implementation Notes) — confirmed the ONLY remaining live call site that still mutates state directly, THEN fires the `AFTER_EVENT` hook, with `persist_job_hook_fun` doing the real WAL append + async commit from inside the hook (not through `apply_committed_entry()`). This is the same mutate-before-commit shape as the bug-795/828/851/1060 divergence family raft-apply-decoupling was built to eliminate for job submit + worker facts, narrowly scoped to admin suspend/resume. Not yet known to have caused a live incident — rare, synchronous, admin-only traffic. **Investigate before committing to a fix**: (a) actual blast radius if a commit fails/times out post-mutation (does it ever get observed/notified before commit — violates criterion 2.4-style provisional-state-export concerns?); (b) live-apply vs replay-apply asymmetry for this one WAL entry type — does replay actually diverge from live in any reachable scenario, or is it benign in practice; (c) cost of migrating vs. accepted-residual status quo. Promote to a spec (likely a small `raft-apply-decoupling` follow-up, not standalone) only if the investigation finds real risk, not proactively.
|
||||
- [ ] **WAL-trim cursor-bound guard (bug-1147; surfaced 2026-06-25 by raft-drive-async design review)** — `trim_wal_segments` (`persist.c:1374`) bounds deletion ONLY by the open segment (`if (seg_start_lsn < active_seg_lsn) unlink()`) — it consults **no** standby `acked_lsn`, **no** `committed_lsn`, **no** in-flight snapshot boundary. With no working snapshot backfill (`write_snapshot` emits a 16-byte sentinel only), a standby lagging by more than ~one open segment hits a log range that exists nowhere → permanent committed-prefix gap (the bug-795/828 family). This is **strategy-A retention discipline that should exist regardless**. Stopgap fix: compute `floor = min(all standby acked_lsn, in-flight snapshot boundaries)` under `p->lock` and refuse to delete at/above `floor`. **Folded into `raft-drive-async` task 3.2** (which moves checkpoint/trim onto a concurrent lane, worsening the race) — but stands alone if that slips. Does NOT fix empty/far-behind join (cursor=0); that needs `entity-bearing snapshots` (strategy B, backlog). Related: bug-795, bug-828, bug-1060.
|
||||
- [ ] OP_heartbeat peers cross-validation — include a hash of the local `--peers` string in heartbeat requests; log ERROR on mismatch between candidates. Detects misconfigured nodes at election time instead of silently risking dual-primary split-brain. _(FUTURE_WORK.md §3.)_
|
||||
- [ ] **peer-authentication-hardening** — **NEW 2026-07-06 from admin-config-tool discovery** (`/kiro-discovery`). XCE currently operates under a **trusted-LAN model** — any CA-signed SSL peer can connect to the controller and perform ANY operation. Three gaps identified: **(a) Admin→Controller**: `validate_host()` is a stub returning success for all peers (roadmap § below). `OP_upgradeConfig` has no real authorization gate today. **(b) Worker/Scheduler→Controller**: workers and schedulers connect via `controller_connector.c` using the same TLS context as controller peers, with no role differentiation — a compromised worker could send `OP_upgradeConfig` or read full config via `OP_replConfig`. **(c) Cross-component trust**: no distinction between controller peers, admin tools, workers, and schedulers — they share the same X.509 identity pool. **(d) xceadmin login (future)**: xceadmin currently uses zero-config TLS (same as xce_test). Production deployments likely need an admin login requirement — token-based session (`xceadmin login → token → config push`) or password-per-command. X.509 CN alone proves identity but not intent; a login step provides audit trail, session expiry, and operator accountability. **Needs design**: role-based X.509 CN validation (controller/admin/worker/scheduler roles), per-role OP code allowlists, admin login/session model, audit logging for admin operations. Separate from the existing `validate_host()` hardening (below) which is the minimal gate for `OP_upgradeConfig` specifically. Discuss + design before implementing; likely promotes to a spec via `/kiro-discovery`.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue