lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(resreq-maxtasks-wiring): add task-count derivation helper

Browse source 
Adds apply_job_task_count() to overwrite a standalone primary resreq's
minTasks/maxTasks with a job's real requested task count, no-op for
COMPOUND/ALTER nested primaries, with a defensive min>max clamp.
Closes the gap that lets scheduler-allocation's multi-host allocator
ever be reached (bug-1770). Wiring at live ingest call sites follows
in subsequent tasks.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg
This commit is contained in:
Rongsong Shen 2026-09-06 03:26:32 +00:00
parent ecfb0a3c64
commit 799a71ecb1
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
10 changed files with 918 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

321
.kiro/specs/resreq-maxtasks-wiring/design.md Normal file
View file

@ -0,0 +1,321 @@
## Overview
This feature closes bug-1770: a job's real requested task count is already correctly threaded from the apiserver all the way to `xce_sched_job_t.mintasks/maxtasks`, but the parsed resource request the scheduler's allocator dispatch actually reads (`xce_primary_resreq_t.minTasks/maxTasks`) is unconditionally hardcoded to one task. This makes the scheduler-allocation feature's already-implemented, already-tested, already-soak-validated multi-host allocation logic (`parjob_allocator`) permanently unreachable from any real job submission today.
**Users**: cluster users submitting multi-task jobs, and cluster operators relying on existing single-task jobs continuing to behave identically.
**Impact**: changes the resource request the scheduler dispatches on from an always-`1` constant to a real, per-job derived value for standalone primary resource requests, at every point a resreq is parsed from a job's submitted or persisted state. No existing component's ownership, wire format, or public interface changes shape — this is a targeted derivation fix plus the validation needed to trust it.
### Goals
- A standalone primary resource request's `minTasks`/`maxTasks` reflect the job's real requested task count, at fresh ingest and at snapshot restore, and remain correct across a subsequent resource-request update (a guarantee this feature relies on an existing structural property for, rather than actively enforcing — see Existing Architecture Analysis).
- A job requiring more tasks than one host can provide is genuinely evaluated for, and where appropriate allocated across, multiple hosts, through the real scheduler dispatch path.
- A committed multi-host allocation is proven, end to end, to actually launch and run on real (or virtual) workers.
- Zero behavioral change for any job with a requested task count of one, and zero behavioral change for compound/alternative resource requests.
### Non-Goals
- Implementing `span[ptile=N]` resreq-grammar semantics (remains parsed and discarded, unchanged).
- Wiring task count into primary resource requests nested inside a compound/alternative resource request (bug-1768 already makes their resource checking meaningless regardless of task count).
- Fixing the snapshot-restore `resreq_fp` computation gap (bug-1772, tracked separately) or its sibling in `make_sched_job()` (bug-1774) — the latter is dead code today (confirmed: `make_sched_job()`'s entire call chain, via `sched_update_jobs()`/`sched_update()`, has zero live callers anywhere in this codebase), so this feature does not wire it at all, not merely defer fixing it.
- Fixing a job's resource-request update never taking effect for scheduling purposes for any field (bug-1773) — this feature's own task-count guarantee across an update holds for that same structural reason, not because this feature actively re-wires anything on update.
- Modifying the controller's or worker's existing multi-host launch code (`mk_launchinfo()`, `job_start_remote_task()`, `create_mpi_cmdline()`) — this design depends on and validates that code, but does not change it.
- Fixing the pre-existing per-host live memory-usage reconciliation limitation for multi-host jobs.
## Boundary Commitments
### This Spec Owns
- The derivation that sets a standalone primary resource request's `minTasks`/`maxTasks` from the job's real requested task count, at every live call site whose parsed resreq actually reaches scheduler dispatch.
- The defensive invariant that an invalid (`minTasks > maxTasks`) pair cannot reach allocator dispatch, regardless of upstream validation.
- A new automated end-to-end test proving a real multi-task job, submitted through the normal submission path, is genuinely allocated across multiple hosts by the scheduler and actually runs to completion.
### Out of Boundary
- `span[...]` resreq-grammar syntax (parsed, discarded, unchanged).
- Primary resource requests nested inside a compound/alternative resource request (bug-1768).
- The snapshot-restore `resreq_fp` gap (bug-1772).
- Per-host live memory-usage reconciliation for multi-host jobs (pre-existing, documented limitation).
- The scheduler's plan-generation/allocation algorithm itself (owned by `scheduler-allocation`; this spec only makes it reachable).
- The controller/worker multi-host launch mechanism's internal implementation (pre-existing; this spec validates it, does not modify it).
### Allowed Dependencies
- `scheduler-allocation`'s `parjob_allocator`/`seqjob_allocator`/`alloc_cache` — consumed as-is; this spec must not modify their dispatch logic.
- The apiserver's existing `checkNumProcessors()` validation and `backend/convert.go`'s `mintasks`/`maxtasks` clamp — relied upon as the primary validation layer; this spec adds a redundant defensive check, not a replacement.
- The controller/worker's existing multi-host launch code (`mk_launchinfo()`, `job_start_remote_task()`, `create_mpi_cmdline()`) — read-only dependency for the new end-to-end test.
### Revalidation Triggers
- Any change to `parse_resreq_primary()`'s or `parse_resreq()`'s signature, or to `xce_primary_resreq_t`'s struct shape.
- Any change to how a resreq's top-level tag distinguishes a standalone primary from a `COMPOUND`/`ALTER` tree.
- Any change to `job_submit_data_t.mintasks/maxtasks` or its Go-side validation (`checkNumProcessors()`, `convert.go`'s clamp).
- Any change to `scheduler-allocation`'s dispatch gate (`sched_make_primary_alloc_()`'s `maxTasks == 1` branch) or to bug-1768's compound-nested resource-checking gap, since a future fix there would change what "out of boundary" means for compound resource requests.
## Architecture
### Existing Architecture Analysis
- `parse_resreq_primary(xce_primary_resreq_t*, char*, var, char**)` and `parse_resreq(xce_resreq_t*, char*, var, char**)` (`src/lib/resreq.c`) take no job-context parameter — they cannot see a job's requested task count. Six existing test call sites invoke the bare production signature directly with no job object in scope, ruling out a signature change as the fix mechanism.
- Five call sites in total reach this parser in the source tree, but only **two are reachable in production today**. The scheduler's actual dispatch reads `job->schedobj->u.primary->resreq` (`scheduler.c:315`), a pointer set exactly once, by whichever function constructs `job->schedobj`. Exactly three functions ever do this (`job->schedobj =` grepped exhaustively): `core_build_job()` (`core_events.c`), `make_sched_job()` (`schedobj.c`), and the snapshot-restore path (`snapshot_load.c`). Of these, `make_sched_job()` is **dead code**: its only caller, `sched_update_jobs()`, is only called from inside `sched_update()`'s switch, and `sched_update()` (`scheduler.c`) has zero live callers anywhere in this codebase — confirmed by exhaustive grep and corroborated by the codebase's own pre-existing comments describing it as dead/superseded, and by the identical, already-fixed sibling case in bug-1740. `core_build_job()` (fresh submission, via the driver-translated `XCE_CORE_EVENT_JOBS_PUSH` path) and the `snapshot_load.c` restore path are the two genuinely live schedobj-construction sites, and are this spec's only implementation targets.
- The remaining two of the five parser call sites are job-update handlers (`sched_update_existing_job()` in `schedobj.c`, the `JOB_UPDATE` handler in `core_events.c`) that replace `job->resreq` but never rebuild `job->schedobj`. `sched_update_existing_job()` is itself dead code (its sole caller is also inside `sched_update()`'s dead switch). `core_events.c`'s `JOB_UPDATE` handler IS live (reached via the driver-translated path, confirmed by tracing `main.c` → `driver.c`'s event translation → `xce_core_push_events()`) — but it never affects scheduling regardless, since `job->schedobj` isn't rebuilt there either, and no live feature in this codebase today ever actually changes an existing job's `submit->resreq` (grepped the whole controller: nothing mutates it post-submission). Applying this feature's helper at either update call site would be dead code with zero observable effect. Requirement 1.4's guarantee (task count unaffected by an update) already holds today for this structural reason, independent of this feature. The update path never taking effect for scheduling at all, for any field, is a real, separately-tracked, pre-existing defect (bug-1773).
- **"Snapshot restore" (Requirement 1.3/1.4's `snapshot_load.c` site) is live only via the simulation/replay driver, not live-cluster crash recovery.** `xce_sched_core_load_snapshot()`'s only real caller anywhere in `src/` is `sim.c`'s `sim_locate_and_load_snapshot()`, reachable only through `main.c`'s `--sim FILE` offline-replay flag; normal-mode `main.c` has no snapshot-load call or flag at all. A live scheduler restart (including post-failover, e.g. `run-ha-soak.sh`'s own scheduler-restart step) starts with empty in-memory state and rejoins via `on_scheduler_join()` → the same live `core_build_job()` path fresh ingest uses — it never touches `snapshot_load.c`. This means the HA soak (Requirement 5.2) provides zero exercise of this call site; Requirement 5.6 validates it separately. The fix is still worth wiring here — `--sim` mode drives the identical `parjob_allocator`/`seqjob_allocator` dispatch per this project's own driver-equivalence invariant, so a saved snapshot's task count needs to be correct for simulation/tuning fidelity — but this is a genuinely different (and narrower) liveness scope than "post-restart/post-failover" phrasing might suggest, and bug-1772's own description has been corrected accordingly.
- A single job's resreq parse tree can contain more than one primary when `COMPOUND`/`ALTER` syntax is used; `parse_resreq_primary()` already runs once per primary node in the tree, uniformly. Only the job's own top-level standalone primary (tag `XCE_RESREQ_PRIMARY`) is in this spec's boundary — a nested primary inside `COMPOUND`/`ALTER` must be left exactly as today.
- `job_submit_data_t.mintasks/maxtasks` (the job-level source of truth) is already correctly threaded end-to-end from the apiserver's `num_processors`/`maxnum_processors` fields, validated twice (`checkNumProcessors()`, `convert.go`'s clamp) before reaching the C layer, and already correctly round-tripped through the WAL/snapshot wire format. This spec's boundary starts exactly where that correct value stops propagating.
### Project Invariant Cross-Check
`core_events.c` (home of `core_build_job()`, one of this spec's real call sites) is part of `sched_core_isolation_lib` — the "one core, two consumers" layer shared between the real driver (`driver.c`) and the simulation driver (`sim_loop.c`), per this project's own scheduler-allocation task 5.2 lesson (a fix applied only to the real-driver path there caused a real driver/simulation divergence that corrupted `xce_sched_sim_driver_t` state). **Confirmed, not merely assumed**: `driver.c` and `sim_loop.c` both build `XCE_CORE_EVENT_JOBS_PUSH` events and call the same shared `xce_core_push_events()` (`core_events.c`), whose `JOBS_PUSH` case calls `core_build_job()` directly — no parallel, sim-specific job constructor exists anywhere. Wiring the fix in `core_build_job()` therefore applies identically to real and simulated job ingest with no divergence risk. `test_maxtasks_wiring_ingest.c` still includes a regression test exercising this path through `sim_loop.c`'s own entry point, to keep this confirmed property under test rather than resting on a one-time design-time check.
### Architecture Pattern & Boundary Map
```mermaid
graph TB
ApiServer[Apiserver job submission] --> SubmitData[job_submit_data_t mintasks maxtasks]
SubmitData --> Ingest[Scheduler ingest call sites]
Ingest --> ParseResreq[parse_resreq existing parser]
ParseResreq --> Helper[apply_job_task_count new helper]
Helper --> Primary[xce_primary_resreq_t minTasks maxTasks]
Primary --> Dispatch[Scheduler dispatch existing]
Dispatch --> SeqJob[seqjob_allocator existing]
Dispatch --> ParJob[parjob_allocator existing]
ParJob --> Commit[Multi host allocation committed]
Commit --> Controller[Controller mk_launchinfo existing]
Controller --> Worker[Worker job_start_remote_task existing]
Worker --> Running[Job reaches RUN then DONE]
```
**Architecture Integration**:
- Selected pattern: a single new helper function, invoked by each of the two ingest call sites whose result actually reaches dispatch (`core_build_job()` and the snapshot-restore path), immediately after its existing `parse_resreq()`/`parse_resreq_primary()` call returns.
- Domain boundaries: `src/lib/resreq.c` owns the new helper and its tag-gating/clamp logic; each scheduler-side call site owns invoking it with whichever job-context task count is already in scope there.
- Existing patterns preserved: this mirrors the codebase's own existing idiom of copying a field from `submit->` onto a target struct immediately after that struct is built — already done identically for `job->mintasks/maxtasks` itself at both of these sites.
- New component rationale: one shared helper avoids duplicating the `COMPOUND`/`ALTER` tag-check twice and gives one independently unit-testable unit for the only real logic this fix introduces.
- Steering compliance: no new WAL/snapshot field is introduced (see Data Models); `resreq.c` is a shared C library untouched by the scheduler-core isolation boundary; the fix does not alter `scheduler-allocation`'s dispatch or allocation logic, only what reaches it.
### Technology Stack
| Layer | Choice / Version | Role in Feature | Notes |
|-------|------------------|------------------|-------|
| Shared C library | Existing C (gnu17), `src/lib/resreq.c` | New `apply_job_task_count()` helper | No new dependency |
| Scheduler (C) | Existing C, `src/scheduler/src/` | Two live ingest call-site wiring | `core_events.c`, `snapshot_load.c` |
| Test infrastructure | Existing `xce_test` debug CLI + `xcecd_virtual` | New end-to-end test | Extends the pattern already used by `xce-test-resources`/`xce-test-ha` |
## File Structure Plan
### Modified Files
- `src/lib/resreq.c` — add `apply_job_task_count()`; `parse_resreq_primary()`'s existing `minTasks = 1; maxTasks = 1;` hardcode is left in place as the correct default for a freshly-parsed resreq before the helper is applied, and as the permanent behavior for nested `COMPOUND`/`ALTER` primaries the helper deliberately skips.
- `src/include/resreq.h` — declare `apply_job_task_count()`.
- `src/scheduler/src/core_events.c` — call the helper after `core_build_job()`'s `parse_resreq()` call, using `j->submit->mintasks/maxtasks`. `core_events.c`'s `JOB_UPDATE` handler is NOT modified — it's live, but `job->schedobj` (what dispatch actually reads) is never rebuilt by an update, so applying the helper to the freshly-parsed `job->resreq` there would be dead code (see Existing Architecture Analysis and bug-1773).
- `src/scheduler/src/snapshot_load.c` — call the helper after the restore path's `parse_resreq()` call, using `job->mintasks/maxtasks` (already restored earlier in the same function, before this call).
- `src/scheduler/src/schedobj.c` is NOT modified — `make_sched_job()` and `sched_update_existing_job()` are both dead code (confirmed: unreachable via `sched_update()`, which has zero live callers), so wiring either would have no production effect.
- `src/scheduler/testsuite/test_parjob_e2e_multislot.c` — remove the `PATCH_MAXTASKS_FOR_BUG_1770` workaround now that real ingest correctly wires `maxTasks`, converting it to drive the same scenario through unmodified real ingest.
### New Files
- `src/lib/testsuite/test_resreq_task_count.c` — unit tests for `apply_job_task_count()` in isolation.
- `src/scheduler/testsuite/test_maxtasks_wiring_ingest.c` — white-box tests confirming each of the two wired ingest call sites applies the wiring correctly (including the fresh-ingest-vs-snapshot-restore comparative assertion for Requirement 5.6), plus the structural regression test for Requirement 1.4.
- `src/worker/testsuite/test_worker_launch_type_dispatch.c` — white-box test targeting `job_start_remote_task()`'s launch-type gate (`controller_connector.c`) first: confirms it correctly no-ops (defers to MPI's own remote-launch mechanism) for an MPI launch type, and correctly fans the job out via XCE's own per-host dispatch for the default launch type — this is the actual decision point Requirement 4.4 depends on, and today has zero test coverage of any kind. Secondarily confirms `job_start_local_task()`'s branch into `create_mpi_cmdline()` builds the right host list and command. Neither requires a real MPI runtime installed in the test environment (Requirement 4.4).
- `.claude/skills/xce-test-multihost/` — new skill (`SKILL.md` + driver script), following `xce-test-resources`'s single-controller-node pattern: deploys one controller/scheduler plus two named `xcecd_virtual` workers, submits a real multi-task job via `jobsub`, and verifies genuine multi-host `RUN`-then-`DONE` execution on both hosts, plus one array-job variant.
## System Flows
```mermaid
sequenceDiagram
participant User
participant Api as Apiserver
participant Sched as Scheduler ingest
participant Alloc as Scheduler dispatch
participant Ctrl as Controller
participant W1 as Worker host A
participant W2 as Worker host B
User->>Api: submit job with -n 4,6
Api->>Api: checkNumProcessors validate
Api->>Sched: job_submit_data_t mintasks maxtasks
Sched->>Sched: parse_resreq existing
Sched->>Sched: apply_job_task_count new
Sched->>Alloc: standalone primary minTasks maxTasks real value
Alloc->>Alloc: maxTasks greater than one route to parjob_allocator
Alloc->>Ctrl: commit multi host allocation
Ctrl->>W1: launch info host A share
Ctrl->>W2: launch info host B share
W1-->>Ctrl: task started
W2-->>Ctrl: task started
Ctrl-->>Sched: job running
Sched-->>User: job reaches DONE
```
Key decision visible in the diagram: `apply_job_task_count()` sits strictly between the existing parser and the existing dispatch gate — nothing downstream of it (allocator, controller, worker) changes; nothing upstream of it (apiserver validation, `job_submit_data_t` wiring) changes.
## Requirements Traceability
| Requirement | Summary | Components | Interfaces | Flows |
|-------------|---------|-------------|------------|-------|
| 1.1 | Real task count reaches the standalone primary | `TaskCountWiring`, `IngestTaskCountWiring` | `apply_job_task_count` | Sequence diagram, steps 1-6 |
| 1.2 | Default (one task) behavior unchanged | `TaskCountWiring` | `apply_job_task_count` | — |
| 1.3 | Jobs restored via the simulation/replay driver's snapshot loading derive the same value as fresh ingest | `IngestTaskCountWiring` (snapshot_load.c site) | `apply_job_task_count` | — |
| 1.4 | A resource-request update does not change the job's dispatched task count | `test_maxtasks_wiring_ingest.c` (structural regression test; no production change needed — see Existing Architecture Analysis and bug-1773) | — | — |
| 1.5 | Invalid pair cannot reach allocator dispatch | `TaskCountWiring` | `apply_job_task_count` (clamp) | — |
| 2.1 | Candidate plans evaluated for maxTasks > 1 | Existing `scheduler-allocation` dispatch (unmodified) | — | Sequence diagram, step 7 |
| 2.2 | Multi-host allocation committed when satisfiable | Existing `parjob_allocator` (unmodified) | — | Sequence diagram, step 8 |
| 2.3 | maxTasks == 1 unaffected | Existing `seqjob_allocator` (unmodified) | — | — |
| 3.1 | Identical single-task results before/after | `test_maxtasks_wiring_ingest.c` | — | — |
| 3.2 | No cluster-wide behavior change while maxTasks always 1 | `test_maxtasks_wiring_ingest.c` | — | — |
| 3.3 | Compound-nested primaries unaffected | `TaskCountWiring`'s tag gate | `apply_job_task_count` | — |
| 4.1 | Controller ensures every host has launch info | Existing controller (unmodified), validated by `xce-test-multihost` | — | Sequence diagram, steps 8-10 |
| 4.2 | Worker starts tasks on every allocated host | Existing worker (unmodified), validated by `xce-test-multihost` | — | Sequence diagram, steps 9-12 |
| 4.3 | Scheduler reports running via existing mechanism | Existing scheduler (unmodified), validated by `xce-test-multihost` | — | Sequence diagram, step 13 |
| 4.4 | Non-default launch type honored | `WorkerLaunchTypeCoverage` | — | — |
| 5.1 | Existing suites pass unmodified | Full regression run | — | — |
| 5.2 | HA soak shows no regression (does not exercise the simulation/replay driver's snapshot-loading path — see 5.6) | Existing `xce-test-ha` (unmodified) | — | — |
| 5.3 | Real multi-task job proven end to end | `xce-test-multihost` | — | Full sequence diagram |
| 5.4 | No cache collision across differing task counts, scoped to the ingest path with a correct fingerprint | `test_resreq_task_count.c` / `scheduler-allocation`'s `alloc_cache` tests (fresh-ingest, `core_build_job()`, only; snapshot-restore is the documented bug-1772 exception — it never computes `resreq_fp`, confirmed live and unfixed by this feature; `make_sched_job()`'s path is dead code and out of consideration entirely) | — | — |
| 5.5 | Multi-task array job proven end to end | `xce-test-multihost` (array-job scenario) | — | Full sequence diagram, array variant |
| 5.6 | Simulation/replay-loaded snapshot derives the same task count as fresh submission | `test_maxtasks_wiring_ingest.c` (snapshot-restore site, comparative assertion against fresh-ingest for equivalent input) | `apply_job_task_count` | — |
## Components and Interfaces
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts |
|-----------|--------------|--------|---------------|---------------------------|-----------|
| `TaskCountWiring` | Shared C library (`src/lib`) | Apply a job's real task count onto its resreq's standalone primary | 1.1, 1.2, 1.5, 3.3 | None (pure function) | Service |
| `IngestTaskCountWiring` | Scheduler (`src/scheduler/src`) | Invoke the helper at every ingest call site whose result actually reaches dispatch | 1.1, 1.3 | `TaskCountWiring` (P0) | Service |
| `xce-test-multihost` | Test infrastructure | Prove real multi-host allocation and execution end to end | 4.1, 4.2, 4.3, 5.3, 5.5 | Existing controller/worker/scheduler binaries (P0) | Batch |
| `WorkerLaunchTypeCoverage` | Worker (`src/worker/src`) | Close a pre-existing, zero-coverage gap in the worker's launch-type dispatch decision | 4.4 | `controller_connector.c` (P0) | Service |
### Shared C Library (`src/lib`)
#### TaskCountWiring
| Field | Detail |
|-------|--------|
| Intent | Overwrite `minTasks`/`maxTasks` on a resreq's own top-level primary with the job's real requested task count, leaving nested/compound primaries untouched. |
| Requirements | 1.1, 1.2, 1.5, 3.3 |
**Responsibilities & Constraints**
- Sole owner of the tag-gate distinguishing a standalone primary (`XCE_RESREQ_PRIMARY`) from a `COMPOUND`/`ALTER` tree.
- Sole owner of the defensive `minTasks <= maxTasks` clamp.
- Never allocates memory, never mutates `select`/`rusages`/`rusage_hash`/`span`/`same`/`affinity`.
**Dependencies**
- Inbound: `IngestTaskCountWiring` — the two live scheduler-side call sites (P0).
**Contracts**: Service [x]
##### Service Interface
```c
/*
* Overwrite minTasks/maxTasks on resreq's own top-level primary with the
* job's real requested task count. No-op if resreq is NULL or if its
* top-level tag is not XCE_RESREQ_PRIMARY -- a COMPOUND or ALTER resreq's
* nested primaries are left exactly as parse_resreq_primary() set them.
*/
void apply_job_task_count(xce_resreq_t* resreq,
uint32_t mintasks,
uint32_t maxtasks);
```
- Preconditions: `resreq` has already been successfully returned by `parse_resreq()`/`parse_resreq_primary()`.
- Postconditions: if `resreq->tag == XCE_RESREQ_PRIMARY`, its `primary->minTasks`/`primary->maxTasks` equal `mintasks`/`maxtasks`, with `maxtasks` raised to `mintasks` if `mintasks > maxtasks` (mirroring `convert.go`'s clamp direction exactly). Otherwise `resreq` is unchanged.
- Invariants: idempotent — calling it twice with the same inputs produces the same result.
- Type note: parameters are `uint32_t`, matching `job_submit_data_t.mintasks/maxtasks` and `xce_sched_job_t.mintasks/maxtasks` (both `uint32_t`, confirmed in `types.h`) — the values every call site actually holds. `xce_primary_resreq_t.minTasks/maxTasks` (the assignment target) are plain `int` (`resreq.h:239-240`); this function performs a deliberate, documented narrowing conversion at the write, not a silent one. Safe for realistic task counts (well under `INT_MAX`), and no existing code path today assigns across this `int`/`uint32_t` boundary, so this is a new, one-time, explicitly acknowledged conversion rather than a pattern already relied upon elsewhere.
**Implementation Notes**
- Integration: called once, immediately after each of the two live ingest `parse_resreq()`/`parse_resreq_primary()` call sites returns successfully.
- Validation: `test_resreq_task_count.c` covers the standalone-primary case, the `COMPOUND`/`ALTER` no-op case, and the defensive clamp case.
- Risks: none identified — the "undiscovered sixth call site" risk originally recorded in `research.md` has since been independently re-verified twice (exhaustive grep, no additional caller found either time).
### Scheduler (`src/scheduler/src`)
#### IngestTaskCountWiring
| Field | Detail |
|-------|--------|
| Intent | Invoke `apply_job_task_count()` at every ingest call site whose result actually reaches dispatch, using whichever job-context task count is already in scope there. |
| Requirements | 1.1, 1.3 |
**Responsibilities & Constraints**
- Two call sites, each a single new line: `core_build_job()` (`core_events.c`); the restore path (`snapshot_load.c`).
- `schedobj.c` is untouched entirely: `make_sched_job()` and `sched_update_existing_job()` are both dead code (unreachable via `sched_update()`, which has zero live callers).
- `core_events.c`'s `JOB_UPDATE` handler is deliberately NOT touched either — it's live, but rebuilds `job->resreq` and never `job->schedobj`, so the actually-dispatched object never sees the update regardless of whether this helper runs there (bug-1773).
**Dependencies**
- Outbound: `TaskCountWiring` (P0).
**Contracts**: Service [x]
##### Service Interface
No new interface — this is two one-line call-site additions to existing functions, not a new function.
**Implementation Notes**
- Integration: ordering matters only in that the call must happen after `parse_resreq()`/`parse_resreq_primary()` returns and before any downstream code (e.g. `core_build_primary_schedobj()`'s `resreq_fp` computation) reads `minTasks`/`maxTasks` — confirmed both wired sites already satisfy this ordering naturally.
- Validation: `test_maxtasks_wiring_ingest.c` white-box tests each site directly.
- Risks: none identified; each site's change is independently reviewable and revertable.
### Worker (`src/worker/src`)
#### WorkerLaunchTypeCoverage
| Field | Detail |
|-------|--------|
| Intent | Close a pre-existing, zero-coverage gap in the worker's launch-type dispatch decision, adjacent to (but not caused by) this feature's own fix. |
| Requirements | 4.4 |
**Responsibilities & Constraints**
- Does not modify `controller_connector.c` — pure test addition against existing, unmodified production code.
- Primary target: `job_start_remote_task()`'s launch-type gate (correct no-op for an MPI launch type, deferring to MPI's own remote-launch mechanism; correct fan-out via XCE's per-host dispatch for the default type) — the real decision point Requirement 4.4 depends on, with no existing coverage of any kind.
- Secondary target: `job_start_local_task()`'s branch into `create_mpi_cmdline()`, confirming the built host list and command are correct.
**Dependencies**
- External: `controller_connector.c`'s existing, unmodified launch-dispatch functions (P0).
**Contracts**: Service [x]
##### Service Interface
No new interface — a white-box test file (`#include`-ing `controller_connector.c` directly, the same technique already established by `test_controller_connector.c`) exercising existing static functions.
**Implementation Notes**
- Integration: no production code touched.
- Validation: `test_worker_launch_type_dispatch.c`.
- Risks: none — this is a pure test addition against unmodified code, adjacent-gap discovery rather than a fix this feature's own change requires.
### Test Infrastructure
#### xce-test-multihost
| Field | Detail |
|-------|--------|
| Intent | Prove a real multi-task job is genuinely allocated across multiple hosts by the scheduler's own decision, and actually runs to completion. |
| Requirements | 4.1, 4.2, 4.3, 5.3, 5.5 |
**Responsibilities & Constraints**
- Deploys a single controller/scheduler node (no HA needed for this proof) plus two named `xcecd_virtual` instances, each sized so neither alone can satisfy the test job's requested task count.
- Drives submission via plain `jobsub`, never `xce_test`'s `jobrun` manual-allocation bypass — the scheduler's own decision to span hosts is what this test exists to prove.
- Follows the pre-flight zombie-process/port check convention already established for `xce-test-ha`.
**Dependencies**
- External: real `xce_controller`, `xce_scheduler`, `xcecd_virtual` binaries (P0, unmodified by this spec).
**Contracts**: Batch [x]
##### Batch / Job Contract
- Trigger: manual invocation (`bash .claude/skills/xce-test-multihost/run.sh`), same convention as `xce-test-ha`/`xce-test-resources`.
- Input / validation: none beyond the built binaries; the script deploys its own fixture cluster.
- Output / destination: pass/fail verdict line matching the existing skills' `[xce-test-*] PASS/FAIL` convention, plus per-host worker log confirmation.
- Idempotency & recovery: each run deploys a fresh, uniquely-named cluster instance; no shared state across runs.
**Implementation Notes**
- Integration: reuses `run-ha-soak.sh`'s `submit_worker_job()`/`verify_worker_rejoin_resync()` polling idiom (real job, poll `jobinfo` for `RUN` then `DONE`, grep each worker's own log) rather than inventing a new verification mechanism.
- Validation: two scenarios — a plain multi-task job, and a multi-task job submitted as part of an array job (Requirement 5.5).
- Risks: soak-style process/port flakiness, mitigated by the same pre-flight check `xce-test-ha` already established.
## Data Models
### Domain Model
No new entities or fields. `xce_primary_resreq_t.minTasks`/`.maxTasks` (existing fields) transition from a compile-time constant to a per-job derived value.
### Persistence / Save-Restore Completeness
Per `persistence.md`'s "Snapshot Save/Load Field-Completeness" section: `minTasks`/`maxTasks` are **not** independently persisted in the WAL or snapshot wire format. `snapshot_wire.c` persists `job_submit_data_t.mintasks/maxtasks` (the job-level source of truth, already correctly round-tripped, confirmed unchanged by this design); `snapshot_load.c` restores that value and then calls the same `parse_resreq()` chain fresh — the exact derivation this spec fixes. There is no field being newly persisted, so there is no new round-trip gap to enumerate: fixing the shared derivation fixes fresh-ingest and snapshot-restored jobs identically, by construction.
## Error Handling
### Error Categories and Responses
**Invalid task-count pair** (`minTasks > maxTasks` reaching `apply_job_task_count()`): defensive-only, since the apiserver already guarantees this invariant twice before any job reaches the C layer. `maxtasks` is raised to `mintasks`, mirroring `convert.go`'s clamp direction exactly — no new job-rejecting error path is introduced.
**`mintasks == 0`**: not separately validated by this feature, since it's unreachable from either live call site (both `convert.go` and `xce_test`'s `jobsub` independently clamp `mintasks` to at least 1 before a job ever reaches the C layer). A genuinely-zero pair (only reachable via a hand-crafted `--sim` snapshot bypassing both guards) degrades gracefully through `scheduler-allocation`'s own pre-existing `maxTasks <= 1` bail in `parjob_allocator.c` — the job pends as unsatisfiable rather than reaching an inconsistent dispatch state.
## Testing Strategy
- **Unit Tests** (`test_resreq_task_count.c`): standalone primary receives the applied task count; `COMPOUND`/`ALTER`-tagged resreq is left untouched; `minTasks > maxTasks` is defensively clamped.
- **Integration Tests** (`test_maxtasks_wiring_ingest.c`): each of the two live ingest call sites applies the wiring correctly (fresh ingest via `core_build_job()`, and the simulation/replay driver's snapshot-loading path via `snapshot_load.c`); the snapshot-loading test asserts the same task count is derived as an equivalent fresh-ingest input would produce, satisfying Requirement 5.6's comparative claim directly; confirms `sim_loop.c`'s job-ingest path reaches the same `core_build_job()`/`core_events.c` call site as the real driver, so simulated and real job ingest apply the fix identically with no driver/simulation divergence; a structural regression test drives a real resource-request update (Requirement 1.4) and confirms the job's dispatched task count is unchanged, proving the guarantee holds via `job->schedobj` never being rebuilt (bug-1773) rather than via any new code this feature adds. `test_parjob_e2e_multislot.c` is updated to drive its multi-host scenario through unmodified real ingest, closing the loop on the bug it originally characterized.
- **E2E Tests** (`xce-test-multihost`): a real multi-task job submitted via plain `jobsub` against two real workers is genuinely split across both hosts by the scheduler's own decision and reaches `DONE` on both (4.1-4.3); a second scenario repeats this for a multi-task array job (5.5).
- **Non-default launch type** (`test_worker_launch_type_dispatch.c`, white-box): confirms `job_start_remote_task()`'s launch-type gate correctly no-ops for an MPI launch type and correctly fans out via XCE's own per-host dispatch for the default type — the actual decision point Requirement 4.4 depends on, currently with zero coverage of any kind — and confirms `job_start_local_task()`'s branch into `create_mpi_cmdline()` builds the correct host list and command (4.4), without requiring a real MPI runtime in the test environment. The E2E harness above only exercises the default launch path, whose availability is guaranteed.
- **Regression**: full existing scheduler/controller/worker test suites re-run unmodified (5.1); existing `xce-test-ha` sequential and chaotic soak re-run unmodified as non-regression evidence (5.2); a cache-key collision test (reusing `scheduler-allocation`'s `alloc_cache` test conventions) confirms two otherwise-identical jobs with different task counts don't collide in the unsatisfiable-resreq cache, for the fresh-ingest path (`core_build_job()`) where `resreq_fp` is correctly computed (5.4) — the snapshot-restore path is explicitly excluded from this test per the documented, pre-existing bug-1772 exception (it never computes `resreq_fp`, confirmed live and unfixed by this feature), not silently skipped; `make_sched_job()`'s path is dead code and not a live production concern at all.

57
.kiro/specs/resreq-maxtasks-wiring/requirements.md Normal file
View file

@ -0,0 +1,57 @@
# Requirements Document
## Introduction
Cluster users can already request a job spanning multiple tasks through the existing job-submission API and CLI (a minimum/maximum task count, e.g. `xcecli jobs submit -n 4,6`), and that requested count is correctly carried all the way into the scheduler's own job record. However, the parsed resource request the allocator dispatch actually consults is unconditionally fixed at a single task regardless of what was requested, so no job today can ever be evaluated for, or allocated across, multiple hosts -- it silently behaves as a single-task job no matter what a user asked for. This defect (bug-1770) was discovered while validating the scheduler-allocation feature, which already implements, tests, and soak-validates real multi-host allocation logic that currently has no way to ever run in production. This feature wires a job's actually-requested task count through to the resource request the allocator reads, so that a job requiring more tasks than a single host can provide is genuinely evaluated for, and where appropriate allocated across, multiple hosts -- and so that such an allocation decision results in the job actually launching and running correctly.
## Boundary Context
- **In scope**: standalone primary resource requests (a job's own resource request, not one nested inside a compound resource request); the existing job-level task-count submission path; fresh job ingest and job ingest from a saved snapshot loaded by the simulation/replay driver; end-to-end verification that a resulting multi-host allocation decision launches and runs correctly on the worker side.
- **Out of scope**: resource-request-string-level task-count syntax (`span[...]`), which remains parsed and silently discarded exactly as today, unchanged by this feature; primary resource requests nested inside a compound resource request, which have a separate, already-tracked gap (bug-1768) that makes their resource checking meaningless regardless of task count; the pre-existing snapshot-restore resource-request-fingerprint gap (bug-1772), tracked separately; a sibling fingerprint gap in a job-construction code path confirmed to have no live caller in this codebase today (bug-1774, dead-code-only, no current production impact); per-host live memory-usage reconciliation for a multi-host job, which is an existing, documented limitation this feature does not change; a job's resource-request update never actually taking effect for scheduling purposes for any field, not only task count (bug-1773) — this feature's own task-count guarantee holds across an update for that same structural reason, not because this feature actively re-wires anything on update.
- **Adjacent expectations**: this feature depends on the scheduler-allocation feature's already-implemented multi-host allocation and unsatisfiable-resource-request cache continuing to behave as specified; it also depends on the controller and worker's existing multi-host job-launch capability (remote task dispatch and message-passing-style launch), which predates this feature and has not previously been exercised by a real scheduler decision.
## Requirements
### Requirement 1: Job-level task count reaches the resource request the allocator dispatches on
**Objective:** As a cluster user, I want the task count I request for my job to determine how the scheduler evaluates and allocates it, so that requesting multiple tasks isn't silently ignored.
#### Acceptance Criteria
1. When a job is submitted with a requested minimum and maximum task count greater than one, the Scheduler shall set the job's standalone primary resource request's minimum and maximum task count to the requested values.
2. When a job is submitted without an explicit task count, or with a task count of one, the Scheduler shall set the job's standalone primary resource request's minimum and maximum task count to one, matching today's behavior.
3. When a job is reconstructed from a saved snapshot loaded by the simulation/replay driver, the Scheduler shall derive the same standalone primary resource request task count it would have derived from a fresh submission of that same job.
4. When a job's resource request is updated, the Scheduler shall continue dispatching that job using the task count it derived at ingest, regardless of the update.
5. If a job's requested minimum task count would exceed its requested maximum task count at the point the Scheduler derives the primary resource request, the Scheduler shall not let an invalid task-count pair reach the allocator dispatch.
### Requirement 2: Multi-host allocation is genuinely reachable when needed
**Objective:** As a cluster user submitting a job that needs more task slots than any single host can provide, I want the scheduler to actually consider spanning multiple hosts, so that my job isn't wrongly treated as unsatisfiable or restricted to a single host.
#### Acceptance Criteria
1. When a standalone primary resource request's maximum task count is greater than one, the Scheduler shall evaluate candidate allocation plans ranging from a single host up to as many hosts as the requested task count could need.
2. When a candidate allocation plan can be satisfied by available hosts, the Scheduler shall commit a job allocation spanning the hosts in that plan, with each host's own task count recorded correctly.
3. When a standalone primary resource request's maximum task count is exactly one, the Scheduler shall dispatch the job through the same single-host allocation path used today, unaffected by this feature.
### Requirement 3: No regression to existing single-task job behavior
**Objective:** As a cluster operator, I want every job that does not request more than one task to behave exactly as it does today, so that this fix introduces no regression to existing production traffic.
#### Acceptance Criteria
1. The Scheduler shall produce identical host-selection and allocation results for a single-task job before and after this feature, given identical worker and resource state.
2. While no job in a cluster requests more than one task, the Scheduler's observable dispatch, allocation, and pending behavior shall be unchanged by this feature.
3. The Scheduler shall not change how a primary resource request nested inside a compound resource request is evaluated or allocated.
### Requirement 4: A committed multi-host allocation results in correct job execution
**Objective:** As a cluster user, I want a job allocated across multiple hosts to actually launch and run correctly, so that requesting multiple tasks results in real distributed execution rather than only a scheduling decision.
#### Acceptance Criteria
1. When the Scheduler commits a job allocation spanning more than one host, the Controller shall ensure every allocated host has the information it needs to start its own share of the job's tasks.
2. When a job's allocation spans more than one host, the Worker shall start the job's tasks on every allocated host.
3. When a multi-host job's tasks all start successfully, the Scheduler shall report the job as running through the same job-status mechanism used for single-host jobs.
4. If a job's submission specifies a non-default launch type (for example, an MPI launch type), the Worker shall launch the job using that launch type's existing mechanism across all allocated hosts.
### Requirement 5: Non-regression and end-to-end validation
**Objective:** As a cluster operator, I want this feature validated against the same regression standard as the scheduler-allocation feature it depends on, so that I can trust it in production, in HA failover scenarios, and in the simulation/replay tooling used for offline tuning and debugging.
#### Acceptance Criteria
1. The existing scheduler, controller, and worker test suites shall continue to pass unmodified in assertions, adjusted only where they directly exercised the always-one-task-count behavior this feature replaces.
2. A full HA soak (sequential and chaotic regression) shall show no new job loss, divergence, or liveness stall attributable to this feature. (The HA soak restarts and rejoins a scheduler through the same live ingest path fresh submission uses; it does not exercise the simulation/replay driver's snapshot-loading path, which is validated separately by AC6.)
3. A real multi-task job shall be driven through the full path from submission to worker execution at least once as part of validating this feature, including confirmation that it reaches a running state and completes.
4. For every job ingest path where this feature computes a resource-request fingerprint correctly, a job with a different requested task count than another otherwise-identical job shall not produce an unsatisfiable-resource-request cache hit against that other job's cached outcome. (The simulation/replay driver's snapshot-loading path has a separately-tracked, pre-existing fingerprint gap, bug-1772, that this feature does not fix; that path is explicitly excluded from this guarantee until bug-1772 is resolved.)
5. A real multi-task job submitted as part of an array job submission shall be driven through the full path from submission to worker execution at least once, and shall receive the same per-element task count wiring as a non-array job would.
6. A real multi-task job's saved snapshot, loaded through the simulation/replay driver, shall be shown to derive the same standalone primary resource request task count as it would through fresh submission.

115
.kiro/specs/resreq-maxtasks-wiring/research.md Normal file
View file

@ -0,0 +1,115 @@
# Research & Design Decisions
## Summary
- **Feature**: `resreq-maxtasks-wiring`
- **Discovery Scope**: Extension (existing system) — light discovery
- **Key Findings**:
- `parse_resreq_primary()`/`parse_resreq()` take no job-context parameter at all, and 6 existing test call sites invoke them directly with the bare production signature — an "add a parameter" fix would break every one of them.
- There are 5 real (non-test) call sites, not 3: fresh ingest (`core_build_job()`, `make_sched_job()`), snapshot restore (`snapshot_load.c`), and two previously-uncounted job-update handlers (`sched_update_existing_job()`, `core_events.c`'s `JOB_UPDATE` handler) that also re-parse a job's resreq.
- A single job's resreq parse tree can contain multiple primaries when `COMPOUND`/`ALTER` syntax is used; only the job's own top-level standalone primary (`tag == XCE_RESREQ_PRIMARY`) should receive the job's task count — nested primaries must stay untouched to satisfy Requirement 3.3's non-regression guarantee.
- `minTasks`/`maxTasks` are not independently persisted in the WAL/snapshot wire format — they are always re-derived at parse time from `job_submit_data_t.mintasks/maxtasks`, which is already correctly round-tripped. Fixing the derivation fixes both fresh-ingest and snapshot-restored jobs with no new wire schema.
- `xcecd_virtual`'s multi-host relay code (`job_start_remote_task()`, `create_mpi_cmdline()`, `handle_runtasks()`) is completely unguarded by `VIRTUAL_CLUSTER` — it is byte-identical to the real worker's code, and multiple named virtual-worker instances can already run concurrently. No new worker-side test infrastructure is needed.
- Today, the only way to produce a real `nhosts > 1` dispatch end-to-end is `xce_test`'s `jobrun` debug bypass, which hand-constructs the allocation and skips the scheduler's own decision entirely. Once this spec's fix lands, a plain `jobsub -n <count>` against a real scheduler with multiple real workers should produce a genuine multi-host split on its own — proving that is the actual point of this spec's E2E test, not exercising the pre-existing worker-side code (which was never broken).
- Of the 5 parser call sites, only 2 are reachable in production today: `core_build_job()` and the snapshot-restore path. `make_sched_job()` (and its own caller chain, `sched_update_jobs()`/`sched_update()`) is dead code — confirmed by exhaustive grep and by pre-existing comments already in the codebase, and matching the identical, already-fixed sibling case bug-1740 established for `sched_apply_job_rusage()`. `sched_update_existing_job()` is dead for the same reason. `core_events.c`'s `JOB_UPDATE` handler is live but functionally inert for scheduling (bug-1773) and, separately, never receives a genuinely-changed resreq today since no live feature mutates a job's resreq post-submission.
- "Snapshot restore" (`snapshot_load.c`) is live only through the `--sim FILE` offline simulation/replay driver (`sim.c`'s `sim_locate_and_load_snapshot()`), not through any live-cluster crash-recovery or post-failover path. A live scheduler restart rejoins via `on_scheduler_join()` → the same `core_build_job()` path fresh ingest uses, never touching `snapshot_load.c`. This means the HA soak provides zero coverage of that call site; a dedicated comparative test (Requirement 5.6) closes the gap. bug-1772's own original phrasing ("post-restart or post-failover restore") was corrected to reflect this.
- Separately, "write-ahead-log restore" (Requirement 1.3's original wording) turned out not to be a third code path at all: a controller-restart's WAL-replayed job state reaches the scheduler via `on_scheduler_join()` → the identical `core_build_job()` path fresh ingest uses (confirmed by tracing `job.c`'s `restore_job()`/`restore_job_array()` → `sched.c`'s `on_scheduler_join()` → `xcesched_push_jobs()` → `handler.c`'s `sched_on_jobs()` → the same driver-translated ingest). Requirement 1.3 was reworded to refer only to the simulation/replay driver's snapshot loading, since that's the only genuinely distinct second ingest path that exists.
## Research Log
### `parse_resreq_primary()` call graph and fix-shape constraints
- **Context**: Requirements demand the job's real task count reach `xce_primary_resreq_t.minTasks/maxTasks`; needed the exact current code shape to pick a fix mechanism.
- **Sources Consulted**: Direct reads of `src/lib/resreq.c`, `src/include/resreq.h`, `src/scheduler/src/core_events.c`, `src/scheduler/src/schedobj.c`, `src/scheduler/src/snapshot_load.c`, and every test file calling either function.
- **Findings**:
- `parse_resreq_primary(xce_primary_resreq_t*, char*, var, char**)` and `parse_resreq(xce_resreq_t*, char*, var, char**)` (`resreq.c:391-392`, `:448`) take no job object.
- 5 real call sites: `core_build_job()` (`core_events.c:247`), `make_sched_job()` (`schedobj.c:182`), snapshot restore (`snapshot_load.c:438`), `sched_update_existing_job()` (`schedobj.c:638`), and the `JOB_UPDATE` handler (`core_events.c:1221`).
- At every ingest call site, the job's real `mintasks`/`maxtasks` is available in scope at or immediately after the parse call, either via `j->submit->mintasks/maxtasks` (or `wj->submit->`) or, at the two update sites, via the already-resolved `job->mintasks/maxtasks` (the update handlers never refresh task count from `updated->submit`, so re-applying the job's existing value is the conservative, scope-preserving choice).
- 6 test call sites (`test_resreq.c` x3, `test_core_events.c:1520`, `test_scheduler_cache_dispatch_compound.c:312/315`, `test_snapshot_load_allocator_regression.c:206`) call the bare 4-arg signature directly with no job context — ruling out any signature change.
- `xce_primary_resreq_t` (`resreq.h:238-252`) has no field or comment documenting the relationship between its own `minTasks/maxTasks` and the job-level pair — this duplication (one job, potentially several primaries in a `COMPOUND`/`ALTER` tree, each historically hardcoded to 1/1) is confirmed by `scheduler.c:192-196`'s comment that `parse_resreq_primary()` runs "uniformly for EVERY primary in a resreq's parse tree... not only top-level ones."
- **Implications**: The fix must be an overwrite performed by each caller immediately after the parse call returns (not a parser-signature change), gated on the resreq's top-level tag being `XCE_RESREQ_PRIMARY` so `COMPOUND`/`ALTER` nested primaries are left exactly as today.
### Test infrastructure for a real multi-host execution proof
- **Context**: Requirement 4/5 need an automated test proving a real multi-host job launches and runs, but the controller/worker multi-host code has zero existing automated coverage.
- **Sources Consulted**: `.claude/skills/xce-test-ha/run-ha-soak.sh`, `.claude/skills/xce-deploy-ha/SKILL.md`, `.claude/skills/xce-test-resources/`, `src/worker/meson.build`, `src/worker/src/controller_connector.c`, `src/controller/testsuite/clean_failover_preservation.sh`, `src/controller/testsuite/test.c`'s `cmd_jobrun`.
- **Findings**:
- `xcecd_virtual` is the same `xcecd_srcs` as the real worker, built only with `-DVIRTUAL_CLUSTER`; that macro gates identity/signal/timing shortcuts, never the multi-host relay code path. Multiple named virtual-worker instances can run concurrently (unlike real `xcecd`, which is single-instance per host).
- `run-ha-soak.sh`'s `submit_worker_job()`/`verify_worker_rejoin_resync()` already proves the idiom for "submit real job via real `jobsub`, poll for real `RUN` then `DONE` against a real worker" — single-host only, but directly reusable as a pattern.
- `xce_test`'s REPL (including `jobrun`) is already driven non-interactively via piped stdin in three existing scripts — no new automation mechanism needed.
- No existing automated (meson/Unity) test spawns real, separate controller+worker processes; the one script that spawns real controller processes (`clean_failover_preservation.sh`) never starts a worker and isn't wired into meson.
- **Implications**: Build a new, small standalone script (same shape as `run-resource-verify.sh`/`clean_failover_preservation.sh`, single controller node — no HA needed for this proof), deploying two named `xcecd_virtual` instances and driving a real `jobsub`. Critically, drive it via plain `jobsub -n <count>`, not `jobrun`'s manual-allocation bypass — proving the *scheduler itself* decides to split across hosts is the actual deliverable of this spec; the worker-side launch code is pre-existing and unmodified.
### Persistence field-completeness (per `persistence.md`'s review requirement)
- **Context**: `xce_primary_resreq_t.minTasks/maxTasks` transitions from a compile-time constant to a real, job-derived value — steering requires an explicit per-field save/restore audit for any such change.
- **Findings**: `minTasks`/`maxTasks` are never independently serialized to the WAL or snapshot wire format. `snapshot_wire.c` persists `job_submit_data_t.mintasks/maxtasks` (the job-level source of truth) directly; `snapshot_load.c` restores it and then calls the same `parse_resreq()` chain fresh, which is exactly the derivation this spec fixes.
- **Implications**: No new wire field, no new snapshot schema, no round-trip gap introduced. Fixing the shared derivation function's callers fixes fresh-ingest and snapshot-restored jobs identically and simultaneously — this satisfies the persistence steering's field-completeness bar by construction, not by omission.
## Design Decisions
### Decision: Overwrite-after-parse via a shared helper, not a parser-signature change
- **Context**: `xce_primary_resreq_t.minTasks/maxTasks` must reflect the job's real task count.
- **Alternatives Considered**:
1. Add job-context parameters to `parse_resreq_primary()`/`parse_resreq()` — rejected: breaks 6 existing test call sites that call the bare signature directly, and doesn't cleanly express "only the top-level standalone primary, never nested ones."
2. Duplicate an inline overwrite at each of the 5 call sites — rejected: repeats the `COMPOUND`/`ALTER` tag-check five times (violates Simplification), harder to keep consistent.
3. **Selected**: a single new helper, `apply_job_task_count()`. Originally scoped to all 5 real call sites; a later design review found 2 of the 5 (the job-update handlers) don't affect dispatch at all (`job->schedobj` is never rebuilt on update — see the dedicated Decision below and bug-1773), and a follow-up review found a 3rd (`make_sched_job()`) is dead code entirely (unreachable via `sched_update()`, which has zero live callers — see the dedicated Decision below and bug-1774's correction), narrowing this to the 2 live ingest call sites whose result actually reaches dispatch: `core_build_job()` and the snapshot-restore path.
- **Rationale**: Matches the codebase's own existing idiom (an un-abstracted field-copy from `submit->` onto the target struct right after it's built, already done 5 times for `job->mintasks/maxtasks` itself) while centralizing the one piece of real logic (the tag-gate and the defensive clamp) in one testable place.
- **Trade-offs**: Five call sites still each need one new line invoking the helper — irreducible given the parser's job-context-free signature, but each line is trivial and independently reviewable.
- **Follow-up**: `test_parjob_e2e_multislot.c` (scheduler-allocation feature)'s `PATCH_MAXTASKS_FOR_BUG_1770` workaround becomes obsolete once this fix lands and should be removed in favor of driving that test through real ingest — closing the loop in the same test that originally characterized the bug.
### Decision: E2E proof drives real `jobsub`, not the `jobrun` debug bypass
- **Context**: Need an automated test proving real multi-host execution; `jobrun` already provides a way to force a multi-host dispatch today.
- **Alternatives Considered**:
1. Use `jobrun`'s manual `-m host:ntasks` allocation construction — proves the controller/worker wire path but bypasses the scheduler's own decision, which is not what this spec changes.
2. **Selected**: plain `jobsub -n <mintasks,maxtasks>` against two real workers sized so neither alone can satisfy the request.
- **Rationale**: This spec's entire value is that the scheduler *itself* now correctly decides to span multiple hosts. A test that hand-constructs the allocation would pass identically whether or not this spec's fix exists, proving nothing about it.
- **Trade-offs**: None material — `jobsub`'s real dispatch path already includes everything `jobrun` would need to prove, once the fix lands.
### Decision: Scheduler-side min/max clamp is defensive-only, not a new rejection path
- **Context**: Requirement 1.4 requires the Scheduler not let an invalid task-count pair reach allocator dispatch; upstream (apiserver) already guarantees `mintasks <= maxtasks` twice before any job reaches the C layer.
- **Selected Approach**: `apply_job_task_count()` clamps `maxtasks` up to `mintasks` if violated (mirroring `convert.go`'s exact clamp direction), rather than rejecting the job.
- **Rationale**: Matches this project's "graceful degradation over complete failure" principle and does not introduce a new failure mode for a codepath that should be unreachable today; still gives Requirement 1.4 a real, testable invariant rather than one vacuously true by construction.
### Decision: Job-update call sites are out of implementation scope
- **Context**: An independent design review found that `job->schedobj->u.primary->resreq` — the object the scheduler actually dispatches on — is set once at ingest and never rebuilt by either job-update handler, which only replace `job->resreq` itself.
- **Alternatives Considered**:
1. Apply the helper at both update call sites anyway, for `job->resreq`'s own internal consistency — rejected: it would be dead code with zero effect on dispatch, and could mislead a future reader into thinking it matters.
2. Fix the underlying gap (rebuild `job->schedobj` on update) as part of this spec — rejected: this is a substantially larger, unrelated fix (an update to ANY field, not just task count, never takes effect for scheduling today) that belongs in its own spec.
3. **Selected**: drop the two update-site changes entirely; reword Requirement 1.4 to describe the guarantee that already holds structurally; add a regression test proving it; log the underlying gap separately (bug-1773).
- **Rationale**: Matches the Simplification design principle — no production code should exist that has no observable effect. The regression test still gives Requirement 1.4 real, ongoing coverage without shipping dead code.
- **Follow-up**: bug-1773 (job-update never taking effect for scheduling, any field) is a real, independently valuable fix for a future spec.
### Decision: `make_sched_job()` is dead code — dropped from implementation scope entirely, not merely deferred
- **Context**: An independent review initially found `make_primary_schedobj()` (backing `make_sched_job()`) never computes `resreq_fp`, identical to bug-1772's snapshot-restore gap, and logged it as bug-1774 with Requirement 5.4 scoped to exclude that path. A SECOND, follow-up independent review then established that `make_sched_job()`'s entire call chain is unreachable in production: its only caller, `sched_update_jobs()`, is only called from inside `sched_update()`'s switch, and `sched_update()` (`scheduler.c`) has zero live callers anywhere in this codebase — confirmed by exhaustive grep (only the declaration, definition, and doc-comments describing it as dead/superseded exist) and corroborated by the identical, already-fixed sibling case in bug-1740 (`sched_apply_job_rusage()`, reachable only via the same dead path, fixed by moving the logic to the live `core_events.c` path instead).
- **Alternatives Considered**:
1. Fix `make_sched_job()`'s `resreq_fp` gap now, since this spec already touches this exact class of code — rejected even before the dead-code finding, to stay consistent with the user's own prior scoping decision for bug-1772's sibling instance; rejected even more strongly once dead-code status was confirmed, since fixing unreachable code has zero production value.
2. Wire `apply_job_task_count()` into `make_sched_job()` anyway, for code-path completeness — rejected: it would be dead code applying a fix to dead code, the same "no production code with no observable effect" concern that excluded the job-update call sites.
3. **Selected**: drop `make_sched_job()`/`schedobj.c` from this spec's implementation scope entirely (not merely exclude it from one test's guarantee); correct bug-1774's original "confirmed live" claim (retracted, tagged dead-code/severity-low); move Requirement 5.4's actual exception to bug-1772 (the snapshot-restore gap, which IS live) instead.
- **Rationale**: Matches the Simplification design principle even more directly than the job-update decision above — there is no reason to wire, test, or even discuss a fix for code with zero live callers, beyond noting its existence for whoever eventually revives or removes it.
- **Follow-up**: bug-1774 remains open as a low-priority, dead-code-only finding; bug-1772 (now Requirement 5.4's real exception) should ideally be picked up alongside bug-1767's broader recommended remediation (share one real schedobj-construction implementation across the live call sites, rather than maintaining independent, drift-prone copies — including the dead one, if it's ever revived).
### Decision: Requirement 1.3 reworded, new Requirement 5.6 added, to accurately scope "snapshot restore"
- **Context**: A third independent review, applying the same liveness rigor now applied to `make_sched_job()`, found `snapshot_load.c`'s restore path is live only via the `--sim` offline simulation/replay driver — not live-cluster crash recovery or post-failover restart, which instead rejoins through the identical `core_build_job()` path fresh ingest uses. This meant Requirement 5.2's HA soak was implicitly (and wrongly) credited with covering this call site, and Requirement 1.3's "write-ahead-log restore" phrasing implied a third code path that doesn't actually exist.
- **Alternatives Considered**:
1. Leave the wording as-is, since the implementation plan (wire `apply_job_task_count()` into `snapshot_load.c`) doesn't change — rejected: a reader would reasonably conclude the HA soak validates this path, which it does not, understating an actual test-coverage gap.
2. **Selected**: reword Requirement 1.3 to name the simulation/replay driver specifically (dropping "write-ahead-log restore," which turned out to describe the same live path as fresh ingest, not a distinct one); add Requirement 5.6 requiring a direct comparative proof (snapshot-loaded task count equals what fresh ingest would derive for the same input); clarify Requirement 5.2's AC text that the HA soak doesn't exercise this path; correct bug-1772's own trigger description in the buglog.
- **Rationale**: The fix and its value (correct task count for `--sim`/tuning fidelity, per this project's driver-equivalence invariant) are unchanged — only the test-coverage claim needed correcting. An honest AC set with a real, explicit line of coverage is better than an implicit, incorrect one.
- **Follow-up**: none — `test_maxtasks_wiring_ingest.c`'s existing white-box test for the snapshot-restore call site already provides a natural home for the new comparative assertion; no new test file or infrastructure is needed.
### Decision: white-box MPI-launch test targets `job_start_remote_task()`'s gate, not `create_mpi_cmdline()` alone
- **Context**: A fourth independent review found the originally-planned test target, `create_mpi_cmdline()`, is a pure command-string builder one layer below the actual decision point. `job_start_remote_task()` (`controller_connector.c`, the real entry point `handle_startjob()` calls for `nhosts > 1`) has its own independent launch-type gate deciding whether to defer to MPI's own remote-launch mechanism or fan out via XCE's per-host dispatch — the actual behavior Requirement 4.4 depends on, with zero existing test coverage of any kind.
- **Selected Approach**: retarget `test_worker_launch_type_dispatch.c` primarily at `job_start_remote_task()`'s gate (correct no-op for MPI launch types, correct fan-out for the default type), secondarily at `job_start_local_task()`'s branch into `create_mpi_cmdline()`.
- **Rationale**: A regression in `job_start_remote_task()`'s gate would cause a real multi-host MPI job to be double-launched or wrongly launched — testing only the command-string builder one layer down would not catch that.
- **Follow-up**: none — same file, same white-box `#include` technique already precedented by `test_controller_connector.c`.
## Risks & Mitigations
- Note (non-blocking, documentation-only): `apply_job_task_count()`'s clamp never separately validates `mintasks == 0`. Confirmed unreachable from either real live call site (`convert.go`'s and `xce_test`'s `jobsub`'s own independent `mintasks == 0 -> 1` clamps both predate this feature), and even a hand-crafted `--sim` snapshot with a genuinely-zero `mintasks`/`maxTasks` degrades gracefully via `scheduler-allocation`'s own pre-existing `maxTasks <= 1` bail in `parjob_allocator.c` (a job pends as unsatisfiable, no crash). Not a defect; worth a one-line mention in Error Handling for a future reader's benefit, not a required fix.
- ~~Risk: a sixth, undiscovered call site to `parse_resreq()`/`parse_resreq_primary()` exists that this design missed~~ — RESOLVED: independently re-verified twice by separate design-review passes (exhaustive grep each time), no additional caller found beyond the original 5. No longer an open risk, though the implementation task should still re-grep once more immediately before wiring as cheap insurance.
- ~~Risk: driver/simulation divergence in `core_events.c`~~ — RESOLVED during design review: independently confirmed `driver.c` and `sim_loop.c` both reach `core_build_job()` through the same shared `xce_core_push_events()`, no parallel sim-specific job constructor exists. No longer an open risk.
- Risk (discovered during design review, now mitigated): the job-update call sites don't affect scheduling at all, because `job->schedobj` is never rebuilt on update — logged as bug-1773, removed from this spec's implementation scope, Requirement 1.4 reworded and given a structural regression test instead.
- Risk (discovered during design review, now mitigated): initially thought `make_sched_job()`'s ingest path had the same `resreq_fp` gap as bug-1772 (logged as bug-1774) and needed excluding from Requirement 5.4's test — a follow-up review found `make_sched_job()` is dead code entirely (unreachable in production), so it's dropped from implementation scope outright; Requirement 5.4's real exception is bug-1772 (snapshot-restore), corrected accordingly.
- Risk: the new E2E script's two-virtual-worker deployment introduces soak-infrastructure flakiness (per this project's documented zombie-process/port-conflict soak gotchas) — Mitigation: follow the same pre-flight process/port check convention already established for `xce-test-ha`.
- Risk: fixing task-count wiring exposes bug-1768's compound-nested resource-checking gap or bug-1771's fcfs NULL-queue crash under new, previously-unreachable conditions — Mitigation: both remain explicitly out of this spec's boundary; the new tests target standalone primaries and array jobs only, matching Requirement 3.3's scope.
## References
- `.wolf/buglog.json` bug-1770, bug-1768, bug-1769, bug-1771, bug-1772, bug-1773, bug-1774 — prior findings this spec builds on or explicitly defers. bug-1773 and bug-1774 were discovered during this spec's own design review; bug-1772 and bug-1774 both received a trigger/liveness correction during the same review (bug-1774's original "confirmed live" claim retracted; bug-1772's "post-restart or post-failover" phrasing corrected to "the `--sim` simulation/replay driver").
- Minor, non-blocking cleanup noted but out of this spec's file boundary: `core.c:354` has a stale comment referring to `make_sched_job()` as "still-live" — harmless (prose, not code) but inaccurate after this review's dead-code finding; worth fixing whenever that file is next touched.
- `.kiro/specs/scheduler-allocation/` — the dependency spec whose allocator logic this fix makes reachable.
- `.kiro/steering/persistence.md` — "Snapshot Save/Load Field-Completeness" section, cross-checked above.

25
.kiro/specs/resreq-maxtasks-wiring/spec.json Normal file
View file

@ -0,0 +1,25 @@
{
"feature_name": "resreq-maxtasks-wiring",
"created_at": "2026-09-05T07:18:28Z",
"updated_at": "2026-09-05T19:52:00Z",
"language": "en",
"phase": "tasks-generated",
"approvals": {
"requirements": {
"generated": true,
"approved": true
},
"design": {
"generated": true,
"approved": true
},
"tasks": {
"generated": true,
"approved": true
}
},
"ready_for_implementation": false,
"related_specs": ["scheduler-allocation"],
"related_bugs": ["bug-1770"],
"motivation": "scheduler-allocation's task 5.3/6.2 review confirmed parjob_allocator's entire multi-host combinatorial allocation path (implemented, reviewed, and soak-tested by that feature) has zero live production reachability today, because no real job submission can ever produce xce_primary_resreq_t.maxTasks > 1. This spec exists to close that gap."
}

89
.kiro/specs/resreq-maxtasks-wiring/tasks.md Normal file
View file

@ -0,0 +1,89 @@
# Implementation Plan
- [x] 1. Implement and test the task-count derivation helper
- Overwrite a standalone (top-level) primary resource request's minimum/maximum task count with the job's real requested minimum/maximum task count
- Leave a compound/alternative resource request's nested primary resource requests untouched (no-op) — only the job's own top-level standalone primary is in scope
- Defensively clamp an invalid pair by raising the maximum to the minimum when the requested minimum exceeds the requested maximum, mirroring the apiserver's own clamp direction
- Write unit tests covering: a standalone primary receiving the applied task count, a compound/alternative resreq being left untouched, and the minimum-exceeds-maximum clamp
- Observable: a passing unit test suite demonstrates all three cases (override, no-op, clamp)
- _Requirements: 1.1, 1.2, 1.5, 3.3_
- _Boundary: TaskCountWiring_
- [ ] 2. Core: wire the derived task count into live scheduler ingest paths and close the adjacent worker launch-type gap
- [ ] 2.1 (P) Wire fresh job-ingest to apply the job's real requested task count
- Call the new helper immediately after fresh job ingest's existing resource-request parse, using the job's real submitted minimum/maximum task count already in scope there
- Observable: a job's dispatched resource request (the one the allocator actually reads) reflects the real requested task count immediately after fresh ingest, not the previous always-one-task default
- _Requirements: 1.1_
- _Boundary: core_events.c (core_build_job)_
- [ ] 2.2 (P) Wire simulation/replay snapshot-restore ingest to apply the same real task count
- Call the new helper immediately after the snapshot-restore path's existing resource-request parse, using the job's already-restored minimum/maximum task count
- Observable: a job reconstructed from a saved snapshot via the simulation/replay driver has its dispatched resource request reflect the same real task count fresh ingest would produce
- _Requirements: 1.3_
- _Boundary: snapshot_load.c_
- [ ] 2.3 (P) Close the worker's non-default launch-type coverage gap
- Write a white-box test proving the worker's per-host launch dispatch correctly defers to an MPI launch type's own remote-launch mechanism instead of fanning the job out itself
- Write a white-box test proving the same dispatch correctly fans a default-launch-type job out across every allocated host itself
- Write a white-box test proving the local-launch path's MPI command-line construction builds the correct host list and command, without requiring a real MPI runtime in the test environment
- Observable: all three launch-type dispatch tests pass against the existing, unmodified worker launch code, closing a gap that previously had zero coverage
- _Requirements: 4.4_
- _Boundary: test_worker_launch_type_dispatch.c_
- [ ] 3. Integration: verify the ingest wiring end-to-end
- [ ] 3.1 (P) Verify both live ingest call sites, snapshot-vs-fresh parity, and single-task-job no-regression
- Write a white-box test proving fresh job-ingest's dispatched resource request reflects the real requested task count for a multi-task job
- Write a white-box test proving snapshot-restore ingest derives the identical task count an equivalent fresh submission would, directly satisfying the comparative claim between the two paths
- Write a test proving a job requesting exactly one task (or no explicit count) produces identical host-selection and dispatch behavior through this wiring as it did before this feature
- Observable: all three tests pass, together proving both live call sites are wired correctly and that no job requesting one task observes any behavior change
- _Requirements: 1.1, 1.3, 3.1, 3.2, 5.6_
- _Boundary: test_maxtasks_wiring_ingest.c_
- _Depends: 2.1, 2.2_
- [ ] 3.2 Verify driver/simulation ingest parity, update-immunity, and no cache collision across task counts
- Write a regression test proving job ingest driven through the simulation driver's own entry point reaches the identical shared ingest code fresh (real-driver) ingest uses, applying the fix identically with no driver/simulation divergence
- Write a structural regression test proving that after a job is ingested and its resource request is then updated, the job's already-dispatched task count remains unchanged by that update
- Write a test submitting two otherwise-identical jobs through fresh ingest that differ only in requested task count, confirming an unsatisfiable outcome cached for one does not produce a cache hit for the other
- Observable: all three tests pass, proving real/simulated ingest parity, that an update never silently changes an already-dispatched task count, and that differing task counts never collide in the unsatisfiable-resource-request cache on the fresh-ingest path
- _Requirements: 1.4, 5.4_
- _Boundary: test_maxtasks_wiring_ingest.c_
- _Depends: 2.1, 2.2_
- [ ] 3.3 (P) Retire the multi-host allocator characterization test's manual workaround
- Replace the existing multi-host allocator test's manual task-count patch with real ingest of an equivalent multi-task job, exercising the same scenario end to end through the now-fixed wiring
- Observable: the updated test passes while driving its scenario entirely through unmodified real ingest, with the workaround removed
- _Requirements: 2.1, 2.2_
- _Boundary: test_parjob_e2e_multislot.c_
- _Depends: 2.1_
- [ ] 4. Validation: real multi-host end-to-end proof
- [ ] 4.1 Build the multi-host end-to-end test harness
- Add a new test skill that deploys one controller/scheduler node plus two named virtual workers, each sized so neither alone can satisfy the scenario's requested task count
- Follow the existing soak-test convention for pre-flight zombie-process/port checks and a pass/fail verdict line
- Observable: the new harness deploys its fixture cluster and tears it down cleanly, with no scenario wired in yet
- _Requirements: 4.1, 4.2, 4.3, 5.3_
- _Boundary: xce-test-multihost_
- _Depends: 2.1, 2.2_
- [ ] 4.2 Prove a plain multi-task job is genuinely scheduled and completes across multiple hosts
- Submit a real multi-task job through normal job submission (not a manual-allocation bypass) and poll until it reaches a running state, then confirm it reaches completion
- Confirm each allocated host's own log shows its share of the job's tasks actually starting
- Observable: a PASS verdict confirms the scheduler's own decision spans both hosts and the job completes successfully on both
- _Requirements: 4.1, 4.2, 4.3, 5.3_
- [ ] 4.3 Prove a multi-task array-job element receives identical wiring and completes across multiple hosts
- Submit a multi-task job as one element of an array-job submission and repeat the same running-then-complete verification as 4.2
- Confirm the array element's per-element task count wiring matches what an equivalent non-array submission would receive
- Observable: a PASS verdict for the array-job scenario, with the same task-count wiring and multi-host completion as the non-array case
- _Requirements: 5.5_
- [ ] 5. Validation: full non-regression confirmation
- [ ] 5.1 Run and confirm the full existing scheduler, controller, and worker test suites
- Re-run the complete existing suites and confirm every assertion still passes unmodified except for the intentional test changes made in this feature's own tasks
- Observable: a clean full-suite pass recorded as this feature's non-regression evidence
- _Requirements: 2.3, 5.1_
- [ ] 5.2 Run and confirm the existing HA soak shows no regression
- Re-run the existing sequential and chaotic HA soak unmodified and confirm no new job loss, divergence, or liveness stall attributable to this feature
- Observable: a clean soak pass recorded as this feature's HA non-regression evidence
- _Requirements: 5.2_

12
src/include/resreq.h
View file

@ -316,4 +316,16 @@ extern int calc_wanted_global_resvals(xvec_idval_t*, var, xlist_t*,
extern int calc_wanted_host_resvals(xvec_idval_t*, var, xlist_t*,
rusage_per_type_t, int, int);
/*
* Overwrite minTasks/maxTasks on resreq's own top-level primary with the
* job's real requested task count. No-op if resreq is NULL or if its
* top-level tag is not XCE_RESREQ_PRIMARY -- a COMPOUND or ALTER resreq's
* nested primaries are left exactly as parse_resreq_primary() set them.
* If mintasks > maxtasks, maxtasks is defensively raised to mintasks
* before assignment (mirrors the apiserver's own convert.go clamp
* direction). Idempotent; never allocates memory.
*/
extern void apply_job_task_count(xce_resreq_t* resreq, uint32_t mintasks,
uint32_t maxtasks);
#endif

1
src/lib/meson.build
View file

@ -160,6 +160,7 @@ libxce_test_src = ['testsuite/test.c',
'testsuite/test_xvec.c',
'testsuite/test_resreq.c',
'testsuite/test_resreq_fingerprint.c',
'testsuite/test_resreq_task_count.c',
'testsuite/test_resmgr_fixture.c',
'testsuite/test_select.c',
'testsuite/test_bitset.c',

34
src/lib/resreq.c
View file

@ -482,6 +482,40 @@ int parse_resreq(xce_resreq_t* resreq, char* data, var resmgr, char** errmsg)
return rc;
}
/*
* TaskCountWiring (resreq-maxtasks-wiring spec): overwrite a standalone
* primary resreq's minTasks/maxTasks with the job's real requested task
* count. A COMPOUND/ALTER resreq's nested primaries are left exactly as
* parse_resreq_primary() set them -- only the job's own top-level
* standalone primary is in scope (design.md's TaskCountWiring component;
* requirements 1.1, 1.2, 1.5, 3.3).
*
* Narrowing conversion note: mintasks/maxtasks are uint32_t, matching
* job_submit_data_t.mintasks/maxtasks (the caller's own source of truth);
* xce_primary_resreq_t.minTasks/maxTasks are plain int. This is a
* deliberate, one-time, acknowledged narrowing assignment -- safe for
* realistic task counts (well under INT_MAX) -- not a mismatch to "fix"
* by changing either type.
*/
void apply_job_task_count(xce_resreq_t* resreq, uint32_t mintasks,
uint32_t maxtasks)
{
if (resreq == NULL || resreq->tag != XCE_RESREQ_PRIMARY) {
return;
}
/* Defensive clamp: mirror convert.go's own clamp direction -- raise
* the max, never lower the min. The apiserver already guarantees
* mintasks <= maxtasks twice before a job reaches this layer; this is
* a redundant safety net, not a new job-rejecting error path. */
if (mintasks > maxtasks) {
maxtasks = mintasks;
}
resreq->u.primary->minTasks = (int)mintasks;
resreq->u.primary->maxTasks = (int)maxtasks;
}
void kill_primary_resreq(xce_primary_resreq_t* resreq)
{
int i;

2
src/lib/testsuite/test.c
View file

@ -23,6 +23,7 @@ extern void test_limit(void);
extern void test_xvec(void);
extern void test_resreq(void);
extern void test_resreq_fingerprint(void);
extern void test_resreq_task_count(void);
extern void test_select(void);
extern void test_bitset(void);
extern void test_kv(void);
@ -147,6 +148,7 @@ int main(int argc, char* argv[])
TEST(test_xvec);
TEST(test_resreq);
TEST(test_resreq_fingerprint);
TEST(test_resreq_task_count);
TEST(test_select);
TEST(test_bitset);
TEST(test_kv);

262
src/lib/testsuite/test_resreq_task_count.c Normal file
View file

@ -0,0 +1,262 @@
/* test_resreq_task_count.c — resreq-maxtasks-wiring spec, task 1
* (TaskCountWiring component).
*
* Unit tests for apply_job_task_count() (src/lib/resreq.c), the pure
* helper that overwrites a standalone primary resreq's minTasks/maxTasks
* with the job's real requested task count, per design.md's
* TaskCountWiring component. Covers the three cases the task requires:
* - a standalone primary resreq receives the applied task count
* (Requirements 1.1, 1.2)
* - a COMPOUND/ALTER resreq's nested primaries are left untouched
* (Requirement 3.3)
* - an invalid (minTasks > maxTasks) pair is defensively clamped by
* raising maxTasks to minTasks, mirroring convert.go's clamp
* direction (Requirement 1.5)
*/
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unity/unity.h>
#include "resreq.h"
#include "utils.h"
/* ── standalone primary: overwritten with the requested task count ─────── */
void test_apply_job_task_count_standalone_primary_override(void)
{
char* resreqString = "rusage[mem=20]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_t resreq;
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_PRIMARY);
/* Sanity: parser's own hardcoded default before the helper ever runs. */
TEST_ASSERT_EQUAL_INT(1, resreq.u.primary->minTasks);
TEST_ASSERT_EQUAL_INT(1, resreq.u.primary->maxTasks);
apply_job_task_count(&resreq, 4, 6);
TEST_ASSERT_EQUAL_INT(4, resreq.u.primary->minTasks);
TEST_ASSERT_EQUAL_INT(6, resreq.u.primary->maxTasks);
/* Idempotency: calling again with identical inputs must not change
* the result (design.md's TaskCountWiring invariant). */
apply_job_task_count(&resreq, 4, 6);
TEST_ASSERT_EQUAL_INT(4, resreq.u.primary->minTasks);
TEST_ASSERT_EQUAL_INT(6, resreq.u.primary->maxTasks);
kill_resreq(&resreq);
FREEUP(errmsg);
}
/* A default/single-task request (Requirement 1.2) must still land on 1/1 --
* exercised explicitly rather than merely relying on the parser's own
* pre-helper default, since the helper is expected to be applied
* unconditionally at every live ingest call site regardless of the
* requested count. */
void test_apply_job_task_count_default_single_task(void)
{
char* resreqString = "rusage[mem=20]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_t resreq;
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_PRIMARY);
apply_job_task_count(&resreq, 1, 1);
TEST_ASSERT_EQUAL_INT(1, resreq.u.primary->minTasks);
TEST_ASSERT_EQUAL_INT(1, resreq.u.primary->maxTasks);
kill_resreq(&resreq);
FREEUP(errmsg);
}
/* ── COMPOUND/ALTER: nested primaries left exactly as parsed (no-op) ────── */
void test_apply_job_task_count_compound_noop(void)
{
char* resreqString = "2*{rusage[mem=100:duration=20:decay=1]} + "
"4*{rusage[mem=80:duration=30:decay=1]}";
char* errmsg = NULL;
int rc = 0;
xce_resreq_t resreq;
int count;
int before_min[8];
int before_max[8];
int i;
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_COMPOUND);
count = resreq.u.compound->primaries.count;
TEST_ASSERT_TRUE(count > 0);
TEST_ASSERT_TRUE(count <= 8);
/* Snapshot whatever parse_resreq() actually produced for each nested
* primary -- the "N*{...}" compound syntax sets a nested primary's
* minTasks/maxTasks to N (not the bare-rusage default of 1), so the
* no-op guarantee is "unchanged from parse", not "equal to 1". */
for (i = 0; i < count; i++) {
xce_primary_resreq_t* p = (xce_primary_resreq_t*)xlist_get(
&(resreq.u.compound->primaries), i);
TEST_ASSERT_NOT_NULL(p);
before_min[i] = p->minTasks;
before_max[i] = p->maxTasks;
}
apply_job_task_count(&resreq, 4, 6);
/* No-op: tag unchanged, every nested primary left exactly as
* parse_resreq() set it. Only the job's own top-level standalone
* primary is in this helper's scope (Requirement 3.3). */
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_COMPOUND);
for (i = 0; i < count; i++) {
xce_primary_resreq_t* p = (xce_primary_resreq_t*)xlist_get(
&(resreq.u.compound->primaries), i);
TEST_ASSERT_NOT_NULL(p);
TEST_ASSERT_EQUAL_INT(before_min[i], p->minTasks);
TEST_ASSERT_EQUAL_INT(before_max[i], p->maxTasks);
}
kill_resreq(&resreq);
FREEUP(errmsg);
}
void test_apply_job_task_count_alter_noop(void)
{
char* resreqString
= "{2*{rusage[mem=100:duration=20:decay=1]} + "
"4*{rusage[mem=80:duration=30:decay=1]}} || "
"{3*{rusage[mem=150:duration=15:decay=1]} + "
"6*{rusage[mem=120:duration=20:decay=1]}} ";
char* errmsg = NULL;
int rc = 0;
xce_resreq_t resreq;
int ncompounds;
int before_min[8][8];
int before_max[8][8];
int nprimaries[8];
int i;
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_ALTER);
ncompounds = resreq.u.alter->compounds.count;
TEST_ASSERT_TRUE(ncompounds > 0);
TEST_ASSERT_TRUE(ncompounds <= 8);
/* Snapshot whatever parse_resreq() actually produced for every nested
* primary across every compound branch -- the "N*{...}" compound
* syntax sets a nested primary's minTasks/maxTasks to N (not the
* bare-rusage default of 1), so the no-op guarantee is "unchanged
* from parse", not "equal to 1". */
for (i = 0; i < ncompounds; i++) {
xce_compound_resreq_t* c
= (xce_compound_resreq_t*)xlist_get(&(resreq.u.alter->compounds), i);
int j;
TEST_ASSERT_NOT_NULL(c);
nprimaries[i] = c->primaries.count;
TEST_ASSERT_TRUE(nprimaries[i] <= 8);
for (j = 0; j < nprimaries[i]; j++) {
xce_primary_resreq_t* p
= (xce_primary_resreq_t*)xlist_get(&(c->primaries), j);
TEST_ASSERT_NOT_NULL(p);
before_min[i][j] = p->minTasks;
before_max[i][j] = p->maxTasks;
}
}
apply_job_task_count(&resreq, 4, 6);
/* No-op: tag unchanged, every nested primary left exactly as
* parse_resreq() set it (Requirement 3.3). */
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_ALTER);
for (i = 0; i < ncompounds; i++) {
xce_compound_resreq_t* c
= (xce_compound_resreq_t*)xlist_get(&(resreq.u.alter->compounds), i);
int j;
TEST_ASSERT_NOT_NULL(c);
for (j = 0; j < nprimaries[i]; j++) {
xce_primary_resreq_t* p
= (xce_primary_resreq_t*)xlist_get(&(c->primaries), j);
TEST_ASSERT_NOT_NULL(p);
TEST_ASSERT_EQUAL_INT(before_min[i][j], p->minTasks);
TEST_ASSERT_EQUAL_INT(before_max[i][j], p->maxTasks);
}
}
kill_resreq(&resreq);
FREEUP(errmsg);
}
/* ── defensive clamp: minTasks > maxTasks must not reach dispatch ───────── */
void test_apply_job_task_count_clamps_min_exceeds_max(void)
{
char* resreqString = "rusage[mem=20]";
char* errmsg = NULL;
int rc = 0;
xce_resreq_t resreq;
memset(&resreq, 0x0, sizeof(resreq));
rc = parse_resreq(&resreq, resreqString, NULL, &errmsg);
TEST_ASSERT_TRUE(rc >= 0);
TEST_ASSERT_TRUE(resreq.tag == XCE_RESREQ_PRIMARY);
/* mintasks (6) > maxtasks (4): defensively raise maxtasks to
* mintasks, mirroring convert.go's own clamp direction exactly. */
apply_job_task_count(&resreq, 6, 4);
TEST_ASSERT_EQUAL_INT(6, resreq.u.primary->minTasks);
TEST_ASSERT_EQUAL_INT(6, resreq.u.primary->maxTasks);
kill_resreq(&resreq);
FREEUP(errmsg);
}
/* ── defensive: NULL resreq must be a safe no-op, never a crash ─────────── */
void test_apply_job_task_count_null_resreq_is_noop(void)
{
apply_job_task_count(NULL, 4, 6);
}
void test_resreq_task_count(void)
{
printf("\n>> resreq task count test\n\n");
RUN_TEST(test_apply_job_task_count_standalone_primary_override);
RUN_TEST(test_apply_job_task_count_default_single_task);
RUN_TEST(test_apply_job_task_count_compound_noop);
RUN_TEST(test_apply_job_task_count_alter_noop);
RUN_TEST(test_apply_job_task_count_clamps_min_exceeds_max);
RUN_TEST(test_apply_job_task_count_null_resreq_is_noop);
}