myxce/.kiro/specs/job-array-single-wal-record/research.md
Rongsong Shen b469b174a2 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.
2026-07-17 07:31:06 +00:00

31 KiB
Raw Blame History

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).