forked from lei.chen/myxce
feat(job-array-single-wal-record): scheduler-side local expansion of the compact push
Adds sched_on_job_array() (handler.c), a sibling case to Data_data_schedjobs decoding the new Data.jobArray variant and expanding it locally into N ordinary schedulable tasks via base_jobid + i, routed through the existing unmodified per-job pipeline (sched_on_jobs()). Each element gets its own deep clone (clone_job_submit_data/clone_job_startinfo) of the shared submit spec, since per-element teardown (free_Job_ptr) frees it independently and a shared pointer would double-free. Dedup reuses sched_get_job() against the same kvtab sched_add_job() guards, rather than a new mechanism. A confirmation log line fires once per successful expansion (skipped on a fully-duplicate redelivery). Adds a new task (5.3b) discovered during this task's review: the shared wire dispatcher extract_data() (src/lib/request.c) has no case for Data_data_jobArray and silently drops every real array push before any receiver's handler runs -- outside every current task's boundary, confirmed to affect the future API-server receive path (task 5.4) identically. design.md's File Structure Plan corrected to record this. User confirmed sequencing: fix it now, before 5.4. Task 5.3 of job-array-single-wal-record (Requirements 6.3, 6.4, 6.5, 8.4).
This commit is contained in:
parent
e0cec75284
commit
d16847185e
6 changed files with 526 additions and 6 deletions
|
|
@ -122,7 +122,7 @@
|
|||
- _Requirements: 6.2, 6.4_
|
||||
- _Boundary: Compact Scheduler/API-Server Push (controller-side apiserver push)_
|
||||
|
||||
- [ ] 5.3 (P) Scheduler-side local expansion
|
||||
- [x] 5.3 (P) Scheduler-side local expansion
|
||||
- Decode the compact push and expand it locally into N ordinary schedulable tasks using the same base-jobid-plus-index arithmetic the controller itself uses
|
||||
- Deduplicate by element identity so a redelivered push (e.g. after a scheduler reconnect) does not create duplicate tasks
|
||||
- Log one identifiable confirmation line (base jobid plus element count) once expansion completes
|
||||
|
|
@ -131,6 +131,15 @@
|
|||
- _Boundary: Compact Scheduler/API-Server Push (scheduler daemon receive path)_
|
||||
- _Depends: 5.1_
|
||||
|
||||
- [ ] 5.3b Fix the shared wire dispatcher to carry the compact array push
|
||||
- `extract_data()` (`src/lib/request.c`) is the shared reassembly step every inbound push (scheduler and API-server alike) passes through inside `vtask_entry()` (`src/lib/xcectx.c`) before a receiver's handler ever runs. Its dispatch switch has no case for the `Data_data_jobArray` variant (added task 1.2) and falls into `default: rc = -EINVAL`. Because `vtask_entry()`'s `if (callnext)` treats ANY nonzero return — including a negative errno — as "reassembly complete, proceed," this doesn't surface as an error: the real payload is freed and the receiver is silently handed an empty, zeroed `dataset_t` (`rows=NULL, nrows=0`), with no log anywhere in the chain (`vtask_entry()` discards the handler's own return value unconditionally)
|
||||
- Add a `case Data_data_jobArray:` to `extract_data()` that passes the single, unchunked `JobSubmit` descriptor straight through to the caller's `dataset_t` — no pagination/accumulation loop is needed (unlike the existing LIST-shaped cases), since this push is always exactly one descriptor per commit, matching the "wire shape note" both `xcesched_push_jobarray()` (task 5.1) and `apiserver_push_jobarray()` (task 5.2) already establish
|
||||
- Confirms both the scheduler (task 5.3, already landed) and the future API-server Go receiver (task 5.4) share this exact fix — this task only needs to land once
|
||||
- Observable: a synthetic `Data_data_jobArray` push driven through the REAL `vtask_entry()`/`extract_data()` path (not task 5.3's white-box `sched_handle_dataset()`-direct test) reaches the scheduler's `sched_on_job_array()` with the genuine payload intact, not an empty dataset
|
||||
- _Requirements: 6.1, 6.2, 6.4, 8.4_
|
||||
- _Boundary: shared wire transport (`src/lib/request.c`, `src/lib/xcectx.c`) — out of the Compact Scheduler/API-Server Push component's original File Structure Plan; discovered during task 5.3's review (2026-07-15), design.md corrected to record it_
|
||||
- _Depends: 5.1, 5.2_
|
||||
|
||||
- [ ] 5.4 (P) API-server-side local expansion
|
||||
- Decode the compact push and expand it locally into N per-element records using the same base-jobid-plus-index arithmetic
|
||||
- Deduplicate by element identity so a redelivered push does not create duplicate records
|
||||
|
|
@ -180,3 +189,5 @@
|
|||
- **Task 4.2**: New `_Atomic uint64_t apply_resource_fenced_lsn` field on `persist_t` (0 = clear, nonzero = the stuck LSN) plus `persist_apply_resource_fenced()`/`persist_mark_apply_resource_fenced()`/`persist_clear_apply_resource_fence()`, wired into `persist_apply_pump_run()`'s existing halt branch (engage) and unconditionally after every successful advance (clear). The unconditional clear is provably equivalent to "clear only on the fenced LSN's own successful retry" because `last_applied_lsn` has exactly 3 writers codebase-wide, the other two (`replay_wal`, `persist_recover`) are boot-only and temporally disjoint from the live pump, and a failed walk never advances past the stuck LSN — so the next success by construction IS that LSN's retry. **Known residual gap (out of this task's boundary, flagged by review, NOT yet fixed)**: `replay_wal()` (boot/restart replay, persist.c:3875-3888) still discards `apply_committed_entry()`'s failure return the same way the live pump did before task 4.1, and `persist_init()` unconditionally zeroes the fence field on every restart. A node that crashes while fenced and re-hits the identical resource-exhaustion condition during boot replay will silently pass it and look fully promotable while actually divergent — task 4.1/4.2's protection does not yet extend to the boot-replay path. This is a pre-existing gap (not introduced by this feature) but is now inconsistent with the live path's new guarantee; worth a dedicated follow-up task before anything downstream places full trust in this fence across a restart.
|
||||
- **Task 5.1**: New `xcesched_push_jobarray()` (`sched.c`) sends via direct `xce_request()`/`OPCode_push`/`Request_body_pushmsg` rather than the existing `xce_push_data_req()` helper — `xce_push_data_req()`'s dispatch switch hard-rejects (`-EINVAL`) anything outside jobs/workers/resources/resvals, so it cannot carry a single non-list `Data.jobArray` descriptor. design.md corrected to match (Dependencies + API Contract for the Compact Scheduler/API-Server Push component). Task 5.2's API-server push will hit the identical fork and should follow the same direct-`xce_request()` pattern rather than re-deriving it. One accepted, low-severity trade-off: the direct path does not go through `xce_push_data_req()`'s `checkf`-gated `check_controller`/`c->running` liveness check that every other push gets — a narrow shutdown-race behavioral difference from sibling pushes, not a new safety hazard; not fixed here since threading a liveness check into a single new call site duplicates rather than reuses the shared gate. `sched.c`/`dataex.c`/`egress.c` were never linked into `controller_unit_test` (two existing test files there ship their own fake `on_scheduler_join()`/`notify()`/`collect_confvars()` precisely because of this, and linking the real ones would collide) — task 5.1's tests live in a new standalone `sched_push_unit_test` binary instead; task 5.2 will likely face the same fork for `apiserver.c` and should check for an analogous existing-fakes collision before choosing a test-binary strategy.
|
||||
- **Task 5.2**: `apiserver_push_jobarray()` (`apiserver.c`) mirrors task 5.1's `xcesched_push_jobarray()` exactly — same field derivation, same direct-`xce_request()` transport, same `l->count > 1` discriminator — and additionally, correctly replicates `apiserver_push_joblist()`'s existing per-connected-server fan-out loop (`N_APISERVER_MAX` iteration, same lock discipline), independently verified against 2 simulated connected servers each receiving exactly one push. Confirmed the same `apiserver.c`/`dataex.c` link-collision task 5.1 hit for `sched.c` (a fake `notify()` already exists in `test_commit_barrier_reply.c`) — new standalone `apiserver_push_unit_test` binary added, correctly omitting `egress.c` since `apiserver_push_joblist()`, unlike `sched.c`'s push path, never references `job_submit_committed()`/`collect_jobs_committed()`. One low-severity, non-blocking nit (not fixed): the new test file's comment justifying its `apiserver_init()` idempotency guard is factually inaccurate — it claims a second `apiserver_init()` call would double-register hooks, but `xce_add_hook()` has deduped by `(fn,data)` since bug-845, so that specific claim is wrong; the guard itself is still correct/necessary for a different, real reason (avoids re-`mylock_init()`-ing an already-initialized mutex, UB).
|
||||
- **Task 5.3**: `sched_on_job_array()` (`handler.c`) decodes `Data_data_jobArray`, derives each element's jobid via `base_jobid + i`, and routes constructed elements through the unmodified per-job pipeline (`sched_on_jobs()`). Each element gets its OWN deep clone (`clone_job_submit_data`/`clone_job_startinfo`) of the shared submit spec — required because `sched_pend_update_killer()`'s per-element teardown (`free_Job_ptr`) frees each element's `submit` independently and a shared pointer across N elements would double-free; independently verified. Dedup reuses `sched_get_job()` against the same `sched->jobs` kvtab `sched_add_job()`'s `key_add` authoritatively guards — sound at the data-integrity level, but review found a narrow, non-corrupting TOCTOU: the dedup pre-check runs synchronously at receive time while the actual insert only happens later when `sched_update()` drains `pend_updates[0]`, so a redelivery landing before that drain can re-construct/re-enqueue a duplicate (harmless) object and double the confirmation log line — the same pre-existing, task-5.3-unrelated pattern as `sched_update_jobs()`'s already-ignored `key_add` `-EEXIST` return on the ordinary single-job path. Not fixed (pre-existing class of gap, narrow window, non-corrupting).
|
||||
**CRITICAL discovery during this task's review (2026-07-15), NOT a defect in task 5.3 itself**: the shared wire dispatcher `extract_data()` (`src/lib/request.c`) has no case for `Data_data_jobArray` and silently drops every real array push before ANY receiver's handler runs (see new task 5.3b, inserted immediately after this task per explicit user decision). Confirmed to affect task 5.4's future API-server receive path identically (both share the same `xcecli_ctx_init()` → `vtask_entry()` → `extract_data()` pipeline). Task 5.3's own tests are unaffected by this gap since they use the same established white-box convention as every sibling `handler.c` test (calling `sched_handle_dataset()` directly, bypassing the transport layer) — this is not a new testing shortcut, it just means task 5.3's passing tests do not, and were never intended to, prove real-wire delivery.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue