feat(job-array-single-wal-record): wire array expansion into apply_wal_entry
Adds case WAL_JOB_ARRAY_SUBMIT to apply_wal_entry() (standby catch-up dispatch), calling restore_job_array() hook-free, matching the existing WAL_JOB_SUBMIT case's shape exactly. Review found apply_wal_entry() has zero production call sites -- a 2026-07-10 refactor (ha_standby_advance_committed_and_apply) moved standby entity materialization onto the same apply_committed_entry() pump the primary uses; the code comment claiming otherwise predated that refactor and was never updated. Requirement 3.1/3.2/4.1 are fully delivered by task 3.2 alone. This task's wiring is kept as defensive parity with WAL_JOB_SUBMIT's existing, equally-dead case in the same switch, not as an independent delivery path -- design.md and tasks.md corrected accordingly. Task 3.3 of job-array-single-wal-record.
This commit is contained in:
parent
0c4573b54a
commit
b0c7cfe548
4 changed files with 260 additions and 12 deletions
|
|
@ -62,7 +62,7 @@ The brief's original framing (`job_submit()` looping `create_job()` per element)
|
|||
| Array submission WAL cost | `handle_jobsubmit()` → `build_job_submit_payloads()` builds N plain `xce_job_t` payloads (job.c:221) → `persist_submit_job_payloads_async()` appends N `WAL_JOB_SUBMIT` entries, registers N commit continuations (persist.c:4846) | Requirement 1: N WAL appends / N commit barriers must become 1 |
|
||||
| `create_job()` call site | Only inside the apply seam, via `restore_job()` (job.c:307) — never in `handle_jobsubmit` (removed by task 3.2) | None — this is already the correct location for this design's N-way expansion loop |
|
||||
| Client reply batching | Already one reply per request regardless of `array_size`, via `jobsubmit_async_ctx_t` (job.c:754) and `pending` refcounting | None — reused as-is; the array path sets `pending = 1` instead of `plist.count` |
|
||||
| Live apply dispatch | **Two** live dispatchers of `WAL_JOB_SUBMIT`, not one: `apply_committed_entry()` (persist.c:2721, primary live apply + boot replay) and `apply_wal_entry()` (persist.c:2574, standby catch-up/replicated-apply, explicitly not retired — persist.c:3722) | Requirement 3: both must expand an array record identically; today they share single-job construction only via `restore_job()` |
|
||||
| Live apply dispatch | **Correction (found during task 3.3 review, 2026-07-14):** `apply_wal_entry()` (persist.c:2574) has **zero production call sites** — a 2026-07-10 refactor (`ha_standby_advance_committed_and_apply`, ha.c:8024) moved standby entity materialization onto the SAME `apply_committed_entry()`/`persist_apply_pump_kick` pump the primary uses; the persist.c:3722 comment claiming `apply_wal_entry()` "remains the live standby catch-up dispatch" predates that refactor and was never updated. `apply_committed_entry()` (persist.c:2721) is the **only** live dispatcher — for primary live apply, boot replay, *and* standby catch-up. | Requirement 3/4.1 is fully delivered by task 3.2's `apply_committed_entry()` wiring alone. Task 3.3's parallel wiring into `apply_wal_entry()` is kept as defensive parity with `WAL_JOB_SUBMIT`'s existing (also-dead) case in the same switch, not as a requirement-delivery path. |
|
||||
| Live apply-pump failure handling | `persist_apply_pump_run()` discards `apply_committed_entry()`'s return value (`(void)apply_committed_entry(...)`, persist.c:3281) and unconditionally advances `last_applied_lsn` | Requirement 9.4/9.5: a resource-exhaustion apply failure would silently be skipped forever, never retried, never fenced |
|
||||
| Promotion-gate conditions | `ha.c` gates on `truncated`, `durable_floor` vs HWM, `apply_verdict == APPLY_DIVERGENT`, `self_restart_pending` — never on `last_applied_lsn` vs `committed_lsn` | Requirement 9.4: a node stuck on a resource-exhaustion apply failure looks fully promotable today |
|
||||
| Client-facing array wire format | `struct JobSubmit` (`controller.capnp:533`) already carries `(jobid, arraySize, submit: JobSubmitData)` | None — this is exactly the compact shape Requirement 1/6 need; reused verbatim, not redesigned |
|
||||
|
|
@ -273,7 +273,7 @@ int persist_submit_job_array_async(persist_t* p, uint64_t base_jobid,
|
|||
- Idempotent per index: if `jobid` already exists in `c->jobmgr`, that index is skipped, not treated as an error (handles a re-apply of an already-fully-applied record). If every index already exists, returns `-EEXIST` (mirrors `restore_job()`'s convention), and the caller fires no hook (Requirement 3.3).
|
||||
- Scratch-construct-then-bulk-insert, not construct-then-rollback: all `array_size` elements are built via `create_job()`-equivalent construction into a local scratch list first, **never touching `c->jobmgr` while the construction loop is running**. Only after every element in the scratch list succeeds does one final pass insert all of them into `c->jobmgr` under a single write-lock acquisition (one atomic visibility step, not per-element lock/unlock cycles — this is what makes the array invisible-then-visible rather than visible-then-rolled-back on the success path). This means there is no window during *construction*, success or failure, where a concurrent reader of `c->jobmgr` (a status query, a cancel request) can observe some array indices present and others absent. If construction fails at index `k` (resource exhaustion only — content-derived failures are impossible here because Requirement 9.1's pre-commit validation already ran), the `0..k-1` scratch objects are destroyed directly (no `c->jobmgr` mutation ever occurred, so no `key_del`/rollback race is possible) and the function returns a negative rc. A retry re-enters the loop from index 0 against a fresh scratch list, not from a saved offset.
|
||||
- **Residual gap in the bulk-insert step itself** (found during implementation, task 3.1 review round 1): the final insert-into-`c->jobmgr` pass is *not* actually failure-proof — `key_add()` can itself fail on allocation/growth exhaustion (`c->jobmgr`'s underlying table grows via `realloc()`, and no capacity-reservation primitive exists anywhere in this codebase's `kv.c`/`kvmgr.c`/`zarray.c` stack to pre-check this before mutating). Because the underlying table has no `key_del`, an already-inserted prefix `[0, inserted)` cannot be cleanly un-inserted if a later element in the same bulk pass fails. The implemented contract is therefore: `restore_job_array()` never *silently* claims success on a bulk-insert failure (it returns a negative errno, honestly), but a prefix of elements may be left genuinely, permanently visible in `c->jobmgr` while the overall call reports failure — and critically, that prefix is never included in `out_new_jobs`, so `HOOK_DETAIL_JOB_NEW` never fires for it, on this attempt or any later successful retry of the same LSN. Tasks 3.2/4.x must account for this: a bulk-insert-phase failure leaves elements that are individually addressable (status/cancel/completion work) but permanently un-pushed to the scheduler/API server unless something later re-derives and re-notifies them. This is a narrower, real exception to "atomically invisible-then-visible," not the general case — it fires only on late-stage resource exhaustion, not on ordinary construction failures (which remain fully atomic per the paragraph above).
|
||||
- Called identically from `apply_committed_entry()` (which additionally builds the N-element list and fires `HOOK_DETAIL_JOB_NEW` once, Requirement 3.4) and from `apply_wal_entry()` (hook-free, matching its existing single-job case) — this is the one place expansion logic exists (Requirement 3.1, 3.2).
|
||||
- Called identically from `apply_committed_entry()` (which additionally builds the N-element list and fires `HOOK_DETAIL_JOB_NEW` once, Requirement 3.4) and from `apply_wal_entry()` (hook-free, matching its existing single-job case) — this is the one place expansion logic exists (Requirement 3.1, 3.2). **Correction (2026-07-14, see the Existing Architecture Analysis table's "Live apply dispatch" row):** `apply_wal_entry()` has zero production call sites — `apply_committed_entry()` alone delivers Requirement 3.1/3.2/4.1 for live apply, boot replay, and standby catch-up. The `apply_wal_entry()` wiring is retained as defensive parity with `WAL_JOB_SUBMIT`'s existing (also-dead) case in the same switch, not as a requirement-delivery path.
|
||||
- Every constructed element is tagged with `submit_lsn = entry_lsn` (mirrors `restore_job()`, feeds the existing egress-watermark predicate unchanged).
|
||||
- Exposes an `XCE_TESTING`-guarded fault-injection seam (a forceable construction failure at a caller-chosen element index, compiled out in production builds, matching this codebase's existing `#ifdef XCE_TESTING`-guarded test-only-symbol convention per `structure.md`) — the concrete unit `test_job_array_apply.c` drives directly to satisfy Requirement 9.6's fault-injection scenario.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue