From 1e9627467ad708189c39f7cc8ff5bee4ef3cd747 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 6 Sep 2026 09:38:08 +0000 Subject: [PATCH] feat(resreq-maxtasks-wiring): prove array-job element spans hosts too Extends xce-test-multihost with a second scenario: a 2-element array job (jobsub -a 2) where each element independently requests the same nproc+1 task count as the plain-job scenario. Drives element [0] through the identical RUN-then-DONE, both-hosts-started proof, and confirms its jobinfo Slots: field matches the plain job's exactly (Requirement 5.5). Traced the array-expansion path (handler.c's sched_on_job_array()/clone_job_submit_data()) to confirm no array-specific shortcut bypasses apply_job_task_count() -- each element flows through the same core_build_job() ingest as a standalone job. Fixes a real bug (bug-1775) found while building this: an array element's bracketed jobid (e.g. "[0]") was misparsed by grep -E's bracket-expression syntax, causing a false negative in the per-host verification. Added ere_escape(), a no-op for plain (non-bracketed) jobids. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg --- .claude/skills/xce-test-multihost/SKILL.md | 158 ++++++++++---- .../run-multihost-verify.sh | 192 ++++++++++++++++-- .kiro/specs/resreq-maxtasks-wiring/tasks.md | 2 +- .wolf/buglog.json | 87 ++++++++ 4 files changed, 373 insertions(+), 66 deletions(-) diff --git a/.claude/skills/xce-test-multihost/SKILL.md b/.claude/skills/xce-test-multihost/SKILL.md index 154e175e..c6acf7cb 100644 --- a/.claude/skills/xce-test-multihost/SKILL.md +++ b/.claude/skills/xce-test-multihost/SKILL.md @@ -1,41 +1,43 @@ --- name: xce-test-multihost -description: "End-to-end multi-host job allocation proof: deploys one controller/scheduler node plus two named xcecd_virtual workers (each sized so neither alone can satisfy the test job's requested task count), submits a real multi-task job via plain jobsub, and confirms the scheduler's own decision genuinely splits it across both hosts, which both reach DONE. Usage: bash .claude/skills/xce-test-multihost/run-multihost-verify.sh [--keep-running] [--dir DIR]" +description: "End-to-end multi-host job allocation proof: deploys one controller/scheduler node plus two named xcecd_virtual workers (each sized so neither alone can satisfy the test job's requested task count), submits a real multi-task job via plain jobsub AND as one element of a 2-element array job via jobsub -a N, and confirms the scheduler's own decision genuinely splits both across both hosts, which both reach DONE. Usage: bash .claude/skills/xce-test-multihost/run-multihost-verify.sh [--keep-running] [--dir DIR]" model: haiku --- # XCE Multi-Host Allocation Test Harness -## Status: tasks 4.1 + 4.2 -- harness plus a real plain-job scenario; 4.3 (array-job variant) not yet wired in +## Status: tasks 4.1 + 4.2 + 4.3 -- harness plus BOTH the plain-job and array-job scenarios `run-multihost-verify.sh` implements the resreq-maxtasks-wiring spec's task 4.1 scope (deploy a single controller/scheduler node plus two independently-named `xcecd_virtual` workers, confirm all four roles come up -and join) **plus** task 4.2's scenario on top of it: once the fixture is up -(`HARNESS_READY`), it submits ONE real multi-task job via plain `jobsub`, -polls `jobinfo` for `RUN` then `DONE`, confirms BOTH named workers' -own logs independently show them starting their own share of the job's -tasks, and emits a PASS/FAIL verdict reflecting that real outcome -- not -just "harness stood up". **Not yet implemented**: task 4.3 (the same -scenario submitted as one element of an array job via `jobsub -a N`) -- -see the `EXTENSION POINT for task 4.3` comment block near the bottom of -`run-multihost-verify.sh`. +and join) **plus** task 4.2's plain-job scenario **plus** task 4.3's +array-job scenario, run back-to-back in the same invocation: once the +fixture is up (`HARNESS_READY`), it (1) submits ONE real multi-task job via +plain `jobsub`, polls `jobinfo` for `RUN` then `DONE`, confirms BOTH named +workers' own logs independently show them starting their own share of the +job's tasks; then (2) submits a 2-element array job via `jobsub -a 2` +requesting the SAME per-element task count, drives element `[0]` through +the identical `RUN`-then-`DONE`/both-hosts-started proof, and confirms that +element's own `jobinfo` `Slots:` line matches the plain job's exactly. A +single combined PASS/FAIL verdict reflects both scenarios' real outcome -- +not just "harness stood up". -This closes bug-1770's own end-to-end validation gap for the plain-job path -(this revision): the scheduler-allocation feature already implements, +This closes bug-1770's own end-to-end validation gap for both the plain-job +and array-job paths: the scheduler-allocation feature already implements, unit-tests, and soak-validates real multi-host allocation logic -(`parjob_allocator`) that, until resreq-maxtasks-wiring's own fix lands, has -no way to ever be reached by a real job submission. This skill now proves -- +(`parjob_allocator`) that, until resreq-maxtasks-wiring's own fix landed, had +no way to ever be reached by a real job submission. This skill proves -- with a real controller, a real scheduler, and real (virtual) workers, driven -through plain `jobsub` -- that a job requiring more tasks than one host can -provide is genuinely split across two hosts by the scheduler's own decision, -and actually runs to completion on both. The array-job variant of this same -proof (Requirement 5.5) remains open until task 4.3 lands. +through plain `jobsub` and `jobsub -a N` alike -- that a job requiring more +tasks than one host can provide is genuinely split across two hosts by the +scheduler's own decision, and actually runs to completion on both, whether +submitted standalone or as one element of an array (Requirement 5.5). Traces to `.kiro/specs/resreq-maxtasks-wiring/design.md`'s `xce-test-multihost` component (Requirements 4.1, 4.2, 4.3, 5.3, 5.5). -## What this skill does today (tasks 4.1 + 4.2) +## What this skill does today (tasks 4.1 + 4.2 + 4.3) 1. **Pre-flight zombie-process/port check** -- following the convention already established by `xce-test-ha`'s `run-ha-soak.sh` (a fixed, reused @@ -104,20 +106,45 @@ Traces to `.kiro/specs/resreq-maxtasks-wiring/design.md`'s path's own jobid-tagged evidence line (`"Job ntasks is: "` from the startjob path, or `"run task: jobid = "` from the relayed runtasks path) on each worker's own `$TOP/logs/xcecd-.log`, since - which host takes which path is the allocator's own internal choice. -8. **Report a real PASS/FAIL verdict** reflecting the job's actual outcome - -- not just "harness stood up": + which host takes which path is the allocator's own internal choice. The + jobid is escaped (`ere_escape()`) before being embedded in the `grep -E` + pattern -- needed for task 4.3's bracketed array-element jobids (see + below), a no-op for a plain jobid. +8. **(Task 4.3) Repeat the same proof for a multi-task job submitted as one + element of an array job** -- immediately after the plain-job scenario + above passes, submit a 2-element array job via `jobsub -a 2` requesting + the SAME per-element task count (`-n ,`, reusing the same + `nproc`-derived sizing), then drive element `[0]` (addressed as + `[0]`, the same bracketed-string convention `xce-test-ha`'s + `run-ha-soak.sh` array-soak already established -- `jobinfo`'s existing + argument parser reconstructs `base+i` from it, no new `xce_test` command + needed) through the identical `RUN`-then-`DONE` / both-hosts-started + proof. Confirmed LIVE in `src/scheduler/src/handler.c`'s + `sched_on_job_array()`/`make_array_element()` that an array element is + NOT a special-cased ingest path: each element gets its own independent + `clone_job_submit_data()` deep copy of the identical submit spec, fed + through the SAME `core_events.c` `JOBS_PUSH` -> `core_build_job()` call + site a standalone job uses -- so an array element genuinely receives the + same per-element task-count wiring a non-array job would (Requirement + 5.5). This is also checked empirically, not just asserted by + construction: the array element's own `jobinfo` `Slots:` line + (`job->submit->mintasks/maxtasks`) is compared byte-for-byte against the + plain job's, and the run FAILs if they differ. +9. **Report a real, COMBINED PASS/FAIL verdict** reflecting both scenarios' + actual outcome -- not just "harness stood up": ``` [xce-test-multihost] HARNESS_READY controller=up scheduler=up workers=2/2 top=/home/claude/tmp/xce-test-multihost ... - [xce-test-multihost] PASS: job 0-9-1788684444-366142 (23 tasks, sleep 5s) submitted via plain jobsub, reached RUN then DONE; mh-worker-1 and mh-worker-2 each independently logged starting their own share of its tasks -- the scheduler's own decision genuinely spans both hosts and the job completed on both + [xce-test-multihost] PASS: plain job 0-13-1788686453-901606 (23 tasks, sleep 5s) submitted via plain jobsub AND array-job element 0-4-1788686458-524288[0] (base=0-4-1788686458-524288, 2 elements, 23 tasks, sleep 5s) submitted via jobsub -a 2 both reached RUN then DONE; mh-worker-1 and mh-worker-2 each independently logged starting their own share of both jobs' tasks -- the scheduler's own decision genuinely spans both hosts for BOTH the plain-job and array-job cases, and the array element's task-count wiring (Slots: 23,23) matches the plain job's exactly ``` - Any structural failure (a role never confirming joined, a missing - binary, a pre-flight conflict) OR job-scenario failure (submission - rejected, RUN/DONE never reached within its bounded timeout, either - worker never logging its own share starting) is a `FAIL:` line + - non-zero exit instead, via the shared `fail()` helper. -9. **Clean up** every process this run started -- unconditionally, on + (Verbatim from a real local run.) Any structural failure (a role never + confirming joined, a missing binary, a pre-flight conflict) OR + job-scenario failure in EITHER scenario (submission rejected, RUN/DONE + never reached within its bounded timeout, either worker never logging + its own share starting, the array element's task-count wiring not + matching the plain job's) is a `FAIL:` line + non-zero exit instead, via + the shared `fail()` helper. +10. **Clean up** every process this run started -- unconditionally, on success or failure alike -- unless `--keep-running` was passed. Log files under the deploy directory are always preserved regardless of that flag, matching `xce-test-resources`'s own documented divergence from @@ -153,7 +180,10 @@ bash .claude/skills/xce-test-multihost/run-multihost-verify.sh [--keep-running] `wait-primary` budget per `xce_test` session), `$XCE_MULTIHOST_JOB_SLEEP` (default `5` -- seconds the test job's `sleep ` command runs), `$XCE_MULTIHOST_RUN_TIMEOUT` (default `30`), `$XCE_MULTIHOST_DONE_TIMEOUT` - (default `$XCE_MULTIHOST_JOB_SLEEP + 30`). + (default `$XCE_MULTIHOST_JOB_SLEEP + 30`), `$XCE_MULTIHOST_ARRAY_SIZE` + (default `2` -- task 4.3's array-job scenario: element count for + `jobsub -a N`; only element `[0]` is driven through the full proof + regardless of this value). Must be invoked under an environment with `nix` on `PATH` -- the script shells out to `nix develop` itself for the steps that need it. @@ -234,15 +264,52 @@ on both --`"Starting worker as user ... to process command: ..."`, `xcecd.c` ~54 -- lacks the jobid tag, so it is not used as the primary signal, but corroborates the two above when inspecting a log by hand.) -## Extension point for task 4.3 +`verify_task_started_on_host()`'s jobid is passed through `ere_escape()` +before being embedded in the `grep -E` alternation above (task 4.3): an +array-element jobid string like `[0]` contains `[`/`]`, which `grep +-E` would otherwise parse as a one-character bracket-expression class +(matching a literal `0`) rather than the literal 3-character substring +`[0]` -- silently breaking the match for every array element. Reproduced +this exact false negative live before adding the fix, then confirmed +`ere_escape()` corrects it; a no-op for a plain (non-array) jobid, which +contains no ERE metacharacters. -`run-multihost-verify.sh` has a clearly marked `EXTENSION POINT for task -4.3` comment block (near the bottom of the file, after -`run_multihost_job_scenario()`) documenting exactly where and how to add a -second, array-job scenario -- the same job submitted via `jobsub -a N`, -same `jobinfo_of()`/`wait_for_job_state()` polling idiom, confirming the -array element receives the same per-element task-count wiring a non-array -job would (Requirement 5.5). +## Array-job scenario (task 4.3) + +After the plain-job scenario passes, `run_multihost_array_job_scenario()` +submits a `$ARRAY_SIZE`-element (default `2`) array job via +`submit_multihost_array_job()` (`jobsub -a $ARRAY_SIZE`, composed with the +SAME `-n ,` flag the plain job used), then drives element +`[0]` -- addressed via `array_element_id()`'s `"[0]"` string, the +identical convention `xce-test-ha`'s `run-ha-soak.sh` array-soak already +established (`jobinfo`'s existing `parse_jobid`/`REGEX_JOBARRAY_ID` +reconstructs `base+i` from it; no new `xce_test` command needed) -- through +the SAME `RUN`-then-`DONE`/both-hosts-started proof +`run_multihost_job_scenario()` uses for the plain job. + +**Why an array element genuinely gets the same wiring, not a shortcut**: +confirmed live in `src/scheduler/src/handler.c`'s `sched_on_job_array()`/ +`make_array_element()` that array-element ingest is not special-cased -- +each element gets its own independent `clone_job_submit_data()` deep copy +of the array submission's single, shared `job_submit_data_t` (never a +shared pointer), then flows through `sched_on_jobs()` into the SAME +`XCE_SCHED_OP_UPDATE_JOB` pending-update path an ordinary single-job push +uses, draining into the SAME `core_events.c` `JOBS_PUSH` case -> +`core_build_job()` call site this feature's `apply_job_task_count()` is +wired into. So Requirement 5.5's "same per-element task-count wiring" claim +holds by construction of the production code being driven, not merely by +this script's own assumption -- and `run_multihost_array_job_scenario()` +also checks it empirically, by comparing the array element's own `jobinfo` +`Slots:` line (`job->submit->mintasks/maxtasks`) byte-for-byte against the +plain job's, failing the run if they ever differ. + +Live evidence from a real run (`base=0-4-1788686458-524288`, `ntasks=23`): +`xcecd-mh-worker-1.log` logged `"Job 0-4-1788686458-524288[0] ntasks is: +12"` (the startjob path) and `xcecd-mh-worker-2.log` logged `"run task: +jobid = 0-4-1788686458-524288[0]"` (the relayed runtasks path) -- the exact +same two-path split `run_multihost_job_scenario()`'s own plain job +exhibits, confirming the array element got a genuine, independent 2-host +allocation decision, not a copy of the plain job's. ## Known pre-existing constraints this skill inherits @@ -268,10 +335,13 @@ job would (Requirement 5.5). This document describes design.md's `xce-test-multihost` component (Components and Interfaces -> Test Infrastructure), covering Requirements -4.1, 4.2, 4.3, 5.3, 5.5. Task 4.1 built the deploy/teardown harness; task -4.2 (this revision) drives a real plain multi-task job through it end to -end, satisfying 4.1, 4.2, 4.3, and 5.3 with live evidence. Requirement 5.5 -(the same proof for an array-job submission) remains open until task 4.3 -lands -- see the `EXTENSION POINT for task 4.3` block. See +4.1, 4.2, 4.3, 5.3, 5.5. Task 4.1 built the deploy/teardown harness; task 4.2 +drove a real plain multi-task job through it end to end, satisfying 4.1, +4.2, 4.3, and 5.3 with live evidence; task 4.3 (this revision) adds the +array-job scenario on top, satisfying Requirement 5.5 with live evidence too +(a real multi-task job submitted as one element of an array job, driven +through the full path to worker execution, with its per-element task-count +wiring confirmed to match a non-array job's). All five requirements this +component covers now have live end-to-end evidence. See `.kiro/specs/resreq-maxtasks-wiring/requirements.md` and `design.md` for the full requirement text. diff --git a/.claude/skills/xce-test-multihost/run-multihost-verify.sh b/.claude/skills/xce-test-multihost/run-multihost-verify.sh index 67abadf3..17d382f2 100755 --- a/.claude/skills/xce-test-multihost/run-multihost-verify.sh +++ b/.claude/skills/xce-test-multihost/run-multihost-verify.sh @@ -29,15 +29,37 @@ # PASS/FAIL oracle line now reflects this job's real outcome, not just # harness-ready. # -# Task 4.3 (the same scenario submitted as one element of an array job via -# `jobsub -a N`) is NOT implemented by this revision -- see the "EXTENSION -# POINT for task 4.3" comment block near the bottom of this file. +# Task 4.3 (THIS revision) adds a SECOND scenario on top of 4.2's: the same +# proof, submitted as element [0] of a 2-element array job via `jobsub -a N` +# instead of a plain single-job submission. Confirmed LIVE via +# src/scheduler/src/handler.c's sched_on_job_array()/make_array_element() +# that an array element is NOT a special-cased or shortcut ingest path -- +# each element gets its own independent clone_job_submit_data() deep copy of +# the identical job_submit_data_t the array's single `jobsub -a N -n X,X` +# invocation captured, and is fed through sched_on_jobs() into the SAME +# XCE_SCHED_OP_UPDATE_JOB pending-update queue an ordinary single-job push +# uses, which drains into the SAME core_events.c JOBS_PUSH case -> +# core_build_job() call site this feature's apply_job_task_count() is wired +# into. So an array element genuinely receives the identical per-element +# task-count wiring a standalone job would (Requirement 5.5) -- this is a +# structural fact about the production code, not an assumption this script +# takes on faith; run_multihost_array_job_scenario() below also confirms it +# empirically by comparing jobinfo's own `Slots:` line between the plain job +# and the array element. Also fixed, as part of adding this: verify_task_ +# started_on_host()'s grep pattern now escapes the jobid before embedding it +# in an ERE alternation -- an array element's bracketed id (e.g. "[0]") +# would otherwise have its "[0]" misparsed by `grep -E` as a one-character +# bracket-expression class instead of a literal 3-character substring, +# silently breaking the match (confirmed live: reproduced the exact false +# negative, then confirmed ere_escape() fixes it, before wiring it in below). # # Requirements this task helps satisfy: 4.1 (controller ensures every # allocated host has launch info), 4.2 (worker starts tasks on every # allocated host), 4.3 (scheduler reports running via the existing # job-status mechanism), 5.3 (a real multi-task job driven end-to-end at -# least once, confirmed reaching RUN then completing). See +# least once, confirmed reaching RUN then completing), 5.5 (the same proof +# for a multi-task job submitted as part of an array job, receiving the same +# per-element task-count wiring a non-array job would). See # .kiro/specs/resreq-maxtasks-wiring/{requirements,design}.md. # # ── Pattern provenance (read these before changing this file) ────────────── @@ -142,6 +164,12 @@ WAITMS="${XCE_MULTIHOST_WAITMS:-5000}" JOB_SLEEP_SECS="${XCE_MULTIHOST_JOB_SLEEP:-5}" RUN_TIMEOUT_SECS="${XCE_MULTIHOST_RUN_TIMEOUT:-30}" DONE_TIMEOUT_SECS="${XCE_MULTIHOST_DONE_TIMEOUT:-$((JOB_SLEEP_SECS + 30))}" +# Task 4.3's array-job scenario: element count for `jobsub -a N`. 2 is enough +# to prove the array-submission path (Requirement 5.5 only requires "at +# least once"); only element [0] is verified end-to-end below (design.md's +# xce-test-multihost Implementation Notes: "a multi-task job submitted as +# part of an array job" -- singular, not "every element of"). +ARRAY_SIZE="${XCE_MULTIHOST_ARRAY_SIZE:-2}" # Single-node cluster, deliberately -- design.md's own "no HA needed for this # proof" call for this component. Single, fixed port so the pre-flight @@ -543,6 +571,55 @@ submit_multihost_job() { return 0 } +# Task 4.3: submits a $3-element ARRAY job via `jobsub -a N`, composing the +# SAME `-n ,` task-count flag submit_multihost_job() above +# uses. `-n`/`-a` compose independently in cmd_jobsub (test.c ~2239-2288): +# `-n` parses into local `min`/`max`, unconditionally assigned to +# `submit.mintasks`/`submit.maxtasks` (test.c ~2317-2318) BEFORE `-a`'s +# `arsize` is ever read into `jobsubmit.array_size` (test.c ~2348) -- one +# shared job_submit_data_t, sized once, then fanned out. Confirmed live in +# the scheduler's own array-expansion code +# (src/scheduler/src/handler.c's sched_on_job_array()/make_array_element()): +# EVERY element gets its own independent clone_job_submit_data() deep copy +# of that identical submit spec (never a shared pointer -- each element's +# xce_job_t is freed independently), then flows through sched_on_jobs() into +# the SAME XCE_SCHED_OP_UPDATE_JOB pending-update path an ordinary +# single-job push uses, which drains into the SAME core_events.c JOBS_PUSH +# case -> core_build_job() call site this feature's apply_job_task_count() +# is wired into -- no array-specific ingest shortcut of any kind. So each +# element genuinely receives the SAME per-element task-count wiring a +# standalone job would (Requirement 5.5), by construction of the production +# code this test drives, not merely by this script's own assumption. +# Echoes the captured BASE jobid (bracket-less -- cmd_jobsub prints +# fmt_jobid(array_jobid(jobid)), the array-tag-stripped form, same as +# run-ha-soak.sh's own submit_array()) on success; returns 1 (echoing +# nothing, diagnostic on stderr) on a rejected submit. +submit_multihost_array_job() { + local ntasks="$1" sleep_secs="$2" arsize="$3" out base + out="$(printf 'set masters %s\nset myname mh-verify\nclient\nwait-primary %s\njobsub -u root -q normal -c "sleep %s" -n %s,%s -a %s\nquit\n' \ + "$MASTERS" "$WAITMS" "$sleep_secs" "$ntasks" "$ntasks" "$arsize" | xtest)" + base="$(printf '%s\n' "$out" | sed -n 's/^jobsubmit ok: JobId: \(.*\)$/\1/p')" + if [ -z "$base" ]; then + log "array job submission failed -- raw xce_test output: $(printf '%s' "$out" | tr '\n' ' ')" >&2 + return 1 + fi + echo "$base" + return 0 +} + +# Per-element addressable jobid string for array element $2 (0-based) of +# base jobid $1 -- IDENTICAL string-concatenation convention to +# run-ha-soak.sh's own array_element_id(): no new xce_test command needed, +# since `jobinfo -j`/`-a`'s existing argument parser (utils.c parse_jobid, +# REGEX_JOBARRAY_ID) already reconstructs base_jobid+i from the bracketed +# "[i]" string form -- the identical arithmetic restore_job_array() +# (controller/job.c) itself uses. jobinfo_of()/wait_for_job_state()/ +# verify_task_started_on_host() below all work UNCHANGED against this +# string: none of them bash-glob-expand it (always interpolated via printf +# %s into an xce_test session line, or -- verify_task_started_on_host() +# only -- through ere_escape() first before use in a grep -E pattern). +array_element_id() { echo "$1[$2]"; } + # Poll jobinfo_of() every 1s (up to $3 seconds) until job $1 reaches state # $2. Echoes the observed state on success; returns 1 on timeout (same # jobinfo_of()+sed state-extraction+1s-sleep-loop idiom as run-ha-soak.sh's @@ -561,6 +638,21 @@ wait_for_job_state() { return 1 } +# Escape ERE metacharacters in $1 so it can be embedded in a `grep -E` +# pattern as a LITERAL substring match (task 4.3). Needed because an +# array-element jobid (e.g. "12-3-1700000000-32768[0]") contains `[`/`]` -- +# without escaping, grep -E parses "[0]" as a one-character bracket- +# expression class matching a literal '0', NOT the literal 3-character +# substring "[0]", so verify_task_started_on_host()'s jobid-tagged log-line +# match would silently and always miss for every array element (reproduced +# and confirmed live before adding this fix). A plain (non-array) jobid +# contains no ERE metacharacters today (fmt_jobid's format is digits and +# hyphens only), so this is a byte-identical no-op for task 4.2's existing +# plain-job scenario -- pure defense-in-depth there, load-bearing here. +ere_escape() { + printf '%s' "$1" | sed -e 's/[][\.^$(){}?+*|]/\\&/g' +} + # Per-host "this host actually started its share of THIS job's tasks" # evidence (task 4.2's Observable). Confirmed LIVE (this task's own first # validation run) that the controller does NOT send an independent per-host @@ -596,9 +688,14 @@ wait_for_job_state() { # synchronously as part of the SAME async task that gets this job to RUN, so # it is expected to already be present by the time wait_for_job_state() # above observes RUN -- this wait is a safety margin, not the primary gate). +# $jid is generic -- works UNCHANGED for either a plain jobid (task 4.2) or a +# bracketed array-element jobid string like "[0]" (task 4.3): passed +# through ere_escape() above before being embedded in the grep -E pattern, +# so the alternation matches it as a literal substring either way. verify_task_started_on_host() { - local name="$1" jid="$2" - wait_for_log "worker '$name' started its share of job $jid's tasks" "Job $jid ntasks is:|run task: jobid = $jid" 15 "$TOP/logs/xcecd-$name.log" + local name="$1" jid="$2" ejid + ejid="$(ere_escape "$jid")" + wait_for_log "worker '$name' started its share of job $jid's tasks" "Job $ejid ntasks is:|run task: jobid = $ejid" 15 "$TOP/logs/xcecd-$name.log" } # Orchestrates the whole task 4.2 scenario: size the job so it must span @@ -637,20 +734,72 @@ run_multihost_job_scenario() { MULTIHOST_JOB_NTASKS="$ntasks" } -# ═══════════════════════════════════════════════════════════════════════════ -# EXTENSION POINT for task 4.3 (NOT implemented by this task -- 4.2 above -# covers a plain job only). -# -# Task 4.3 (the same scenario, as one element of an array-job submission via -# `jobsub -a N`): add a second scenario function alongside -# run_multihost_job_scenario() above, following the SAME polling idiom -# (submit_multihost_job() would need an `-a N` variant, or a sibling -# function built the same way; jobinfo/wait_for_job_state() work unchanged -# against a per-element jobid string, same array-element-addressing -# convention run-ha-soak.sh's own array_element_id()/verify_array_elements() -# already establish), confirming the array element receives the SAME -# per-element task-count wiring a non-array job gets (Requirement 5.5). -# ═══════════════════════════════════════════════════════════════════════════ +# Orchestrates the task 4.3 scenario: the SAME job-outcome proof as +# run_multihost_job_scenario() above, but submitted as element [0] of a +# $ARRAY_SIZE-element array job via `jobsub -a N` (Requirement 5.5). Run +# AFTER the plain-job scenario has already passed (main(), below), and +# deliberately reuses this scenario's own compute_task_count() sizing so the +# array element requests the IDENTICAL per-element task count the plain job +# did -- this is what makes the closing jobinfo `Slots:` comparison below a +# genuine like-for-like check, not just two arbitrary numbers. Only element +# [0] is driven through RUN/task-start/DONE (design.md's Implementation +# Notes: "a multi-task job submitted as part of an array job" -- "at least +# once" per Requirement 5.5's own wording, not "every element"). Hard-fails +# (via the shared fail() helper) on the first violated step, same discipline +# as run_multihost_job_scenario() above. +run_multihost_array_job_scenario() { + local ntasks base eid + + ntasks="$(compute_task_count)" + log "array-job scenario: submitting a $ARRAY_SIZE-element array job via jobsub -a $ARRAY_SIZE, each element independently requesting $ntasks tasks (-n $ntasks,$ntasks, \"sleep $JOB_SLEEP_SECS\") -- the SAME per-element task count the plain-job scenario above requested" + base="$(submit_multihost_array_job "$ntasks" "$JOB_SLEEP_SECS" "$ARRAY_SIZE")" \ + || fail "array-job submission failed -- see the diagnostic line just above" + + eid="$(array_element_id "$base" 0)" + log "submitted array job base=$base -- verifying element $eid (index 0 of $ARRAY_SIZE) end to end; polling for RUN (timeout ${RUN_TIMEOUT_SECS}s)" + + wait_for_job_state "$eid" "RUN" "$RUN_TIMEOUT_SECS" >/dev/null \ + || fail "array element $eid never reached RUN within ${RUN_TIMEOUT_SECS}s -- the scheduler's own allocation decision never dispatched it" + log "confirmed: array element $eid reached RUN" + + log "confirming each allocated host's own log shows its share of array element $eid's tasks actually starting" + verify_task_started_on_host "$WORKER1_NAME" "$eid" \ + || fail "array element $eid: '$TOP/logs/xcecd-$WORKER1_NAME.log' never logged starting a share of this element's tasks" + verify_task_started_on_host "$WORKER2_NAME" "$eid" \ + || fail "array element $eid: '$TOP/logs/xcecd-$WORKER2_NAME.log' never logged starting a share of this element's tasks -- the allocation did not genuinely span both hosts" + log "confirmed: BOTH $WORKER1_NAME and $WORKER2_NAME independently logged starting their own share of array element $eid's tasks -- the array element received the same genuine 2-host allocation decision the plain job did" + + wait_for_job_state "$eid" "DONE" "$DONE_TIMEOUT_SECS" >/dev/null \ + || fail "array element $eid reached RUN and both hosts started their share, but never reached DONE within ${DONE_TIMEOUT_SECS}s" + log "confirmed: array element $eid reached DONE -- completed successfully on both hosts" + + # Requirement 5.5's "same per-element task count wiring" claim, checked + # concretely rather than merely asserted by this script's own + # construction: compare this element's own jobinfo `Slots:` line + # (print_jobinfo(), test.c: "Slots: %d,%d" from + # job->submit->mintasks/maxtasks -- the job-level source of truth + # apply_job_task_count() derives the actually-dispatched + # xce_primary_resreq_t.minTasks/maxTasks FROM, per design.md's Existing + # Architecture Analysis) against the plain job's. Both were submitted + # with the identical -n $ntasks,$ntasks above; this assertion proves the + # array-element ingest path (sched_on_job_array()/make_array_element()'s + # clone_job_submit_data(), confirmed live in handler.c -- see this + # file's top-of-file comment) actually PRESERVED that value into the + # element's own independent xce_job_t end to end through to jobinfo, + # rather than this script merely trusting its own inputs. + local plain_slots array_slots + plain_slots="$(jobinfo_of "$MULTIHOST_JOB_ID" | sed -n 's/^Slots: //p')" + array_slots="$(jobinfo_of "$eid" | sed -n 's/^Slots: //p')" + [ -n "$plain_slots" ] && [ -n "$array_slots" ] \ + || fail "could not read jobinfo's Slots: field for the task-count wiring comparison (plain job='$plain_slots' array element='$array_slots')" + [ "$plain_slots" = "$array_slots" ] \ + || fail "Requirement 5.5 violated: array element $eid's task-count wiring (Slots: $array_slots) does not match the non-array job's (Slots: $plain_slots)" + log "confirmed: array element $eid's task-count wiring (Slots: $array_slots) matches the non-array job's (Slots: $plain_slots) exactly -- Requirement 5.5's 'same per-element task count wiring' proven" + + MULTIHOST_ARRAY_BASE_ID="$base" + MULTIHOST_ARRAY_ELEMENT_ID="$eid" + MULTIHOST_ARRAY_NTASKS="$ntasks" +} main() { parse_args "$@" @@ -671,8 +820,9 @@ main() { log "HARNESS_READY controller=up scheduler=up workers=2/2 top=$TOP" run_multihost_job_scenario + run_multihost_array_job_scenario - log "PASS: job $MULTIHOST_JOB_ID ($MULTIHOST_JOB_NTASKS tasks, sleep ${JOB_SLEEP_SECS}s) submitted via plain jobsub, reached RUN then DONE; $WORKER1_NAME and $WORKER2_NAME each independently logged starting their own share of its tasks -- the scheduler's own decision genuinely spans both hosts and the job completed on both" + log "PASS: plain job $MULTIHOST_JOB_ID ($MULTIHOST_JOB_NTASKS tasks, sleep ${JOB_SLEEP_SECS}s) submitted via plain jobsub AND array-job element $MULTIHOST_ARRAY_ELEMENT_ID (base=$MULTIHOST_ARRAY_BASE_ID, $ARRAY_SIZE elements, $MULTIHOST_ARRAY_NTASKS tasks, sleep ${JOB_SLEEP_SECS}s) submitted via jobsub -a $ARRAY_SIZE both reached RUN then DONE; $WORKER1_NAME and $WORKER2_NAME each independently logged starting their own share of both jobs' tasks -- the scheduler's own decision genuinely spans both hosts for BOTH the plain-job and array-job cases, and the array element's task-count wiring (Slots: $MULTIHOST_ARRAY_NTASKS,$MULTIHOST_ARRAY_NTASKS) matches the plain job's exactly" exit 0 } diff --git a/.kiro/specs/resreq-maxtasks-wiring/tasks.md b/.kiro/specs/resreq-maxtasks-wiring/tasks.md index 3a3651c8..fc47dbad 100644 --- a/.kiro/specs/resreq-maxtasks-wiring/tasks.md +++ b/.kiro/specs/resreq-maxtasks-wiring/tasks.md @@ -71,7 +71,7 @@ - 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 +- [x] 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 diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 8335402b..e89b1ade 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -20992,6 +20992,93 @@ "related_bugs": [], "occurrences": 1, "last_seen": "2026-09-04T00:00:00Z" + }, + { + "id": "bug-1772", + "timestamp": "2026-09-05T00:00:00Z", + "error_message": "OPEN, NOT A CRASH -- pre-existing, out-of-scope gap discovered during requirements scoping for the resreq-maxtasks-wiring spec (bug-1770's fix). snapshot_build_primary_schedobj() (src/scheduler/src/snapshot_load.c:81-96) never computes primary->resreq_fp at all, unlike core_build_primary_schedobj() (src/scheduler/src/core_events.c:130-135), which sets resreq_fp.min_tasks/max_tasks/select_node_id/rusage_hash from the parsed xce_primary_resreq_t immediately after parsing. Every job reconstructed from a snapshot (post-restart or post-failover restore) therefore gets a zeroed resreq_fp -- the same class of gap as bug-1767's broader 'recurring schedobj-reconstruction-duplication' pattern (snapshot_load.c independently re-implements core_events.c's schedobj construction rather than sharing it, and each independent copy has repeatedly drifted field-by-field). TRIGGER CORRECTION (2026-09-05, resreq-maxtasks-wiring design review): this entry's original phrasing ('post-restart or post-failover restore') overstates the live trigger. xce_sched_core_load_snapshot()'s ONLY real (non-test) caller anywhere in src/ is sim.c's sim_locate_and_load_snapshot(), reachable only via 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 restart_scheduler_for_new_primary()) starts with empty in-memory state and rejoins via on_scheduler_join() -> the same core_build_job() path fresh ingest uses -- never touching snapshot_load.c. The accurate trigger is: a job in a .replay-recorded snapshot loaded by the --sim simulation/replay driver, not any live-cluster crash-recovery or failover scenario. This does not change the bug's validity or fix (the gap is real for --sim/replay fidelity, which matters per this project's driver-equivalence invariant since --sim drives the identical allocator dispatch) -- it only corrects when it's actually reachable, which matters for scoping test coverage (an HA soak does NOT exercise this path at all).", + "root_cause": "snapshot_load.c's snapshot_build_primary_schedobj() is a hand-duplicated, field-for-field port of core_events.c's core_build_primary_schedobj() (kept independent because core_build_primary_schedobj() is static and the two files are compiled as separate objects -- see bug-1767's own root-cause text for the deliberate-duplication rationale). The port was written before task 1.2 (scheduler-allocation feature) added resreq_fp/hash_rusages() wiring to the core_events.c original, and the snapshot_load.c copy was never updated to match. Effect: a snapshot-restored job's unsatisfiable-resreq cache fingerprint (used by scheduler-allocation's alloc_cache.c) is always the zero value, which is a valid-looking but WRONG fingerprint -- it could produce a false cache hit against an unrelated job that also happens to have a zeroed fingerprint (extremely unlikely today since real jobs always populate resreq_fp correctly at first ingest, but a snapshot-restored job's own fingerprint stays wrong for its entire post-restore lifetime, and any code that later relies on comparing/keying by resreq_fp for a restored job silently gets 0/0/0/0). Discovered while scoping resreq-maxtasks-wiring (bug-1770's fix spec) because that fix will make resreq_fp carry real, meaningful, non-degenerate maxTasks values for the first time -- increasing the practical consequence of this gap versus its current all-zero-everywhere baseline.", + "fix": "NOT FIXED -- logged for its own follow-up, deliberately kept out of the resreq-maxtasks-wiring spec's scope (per that spec's own requirements-phase scoping decision, 2026-09-05) to avoid absorbing an unrelated schedobj-reconstruction-duplication gap into a narrowly-scoped bug fix. Fix should add the same resreq_fp computation core_build_primary_schedobj() does (or better, per bug-1767's own recommended remediation, export core_build_primary_schedobj()/core_build_schedobj() and have snapshot_load.c call the real function instead of maintaining a third independent duplicate).", + "tags": [ + "scheduler", + "snapshot-restore", + "resreq-fingerprint", + "cache", + "pre-existing", + "schedobj-duplication", + "severity-medium" + ], + "related_bugs": [ + "bug-1767", + "bug-1770" + ], + "occurrences": 1, + "last_seen": "2026-09-05T00:00:00Z" + }, + { + "id": "bug-1773", + "timestamp": "2026-09-05T00:00:00Z", + "error_message": "OPEN, NOT A CRASH -- pre-existing, out-of-scope gap discovered during design review for the resreq-maxtasks-wiring spec (bug-1770's fix). A job's resource-request UPDATE (JOB_UPDATE core event / sched_update_existing_job()) never takes effect for scheduling purposes, for ANY field -- not just task count. sched_update_existing_job() (src/scheduler/src/schedobj.c:597-656) and core_events.c's JOB_UPDATE handler (src/scheduler/src/core_events.c:1200-1234) both do `free(job->resreq); job->resreq = newreq;` (a shallow free via FREEUP, utils.h:114) but never touch job->schedobj. ADDENDUM (2026-09-05, independent follow-up investigation): sched_update_existing_job() itself is dead code (unreachable -- its sole caller is scheduler.c's sched_update(), which has zero live callers, same root architecture as bug-1740). core_events.c's JOB_UPDATE handler IS live (traced main.c -> driver.c's xce_sched_driver_translate_pending_events() -> xce_core_push_events() -> the JOB_UPDATE case), reached routinely by ordinary job-lifecycle notifications (state/rusage/postdone), not only by a genuine resource-request change -- but grepping the whole controller finds no code path that ever mutates an existing job's submit->resreq after initial submission, so this live handler's parse_resreq() branch always re-parses the job's original, unchanged resreq string today. No real 'update a job's resource request' feature exists yet in this codebase; this bug describes what WOULD happen if one were added without also fixing job->schedobj reconstruction.", + "root_cause": "CONFIRMED via direct code read: the scheduler's actual dispatch reads job->schedobj (scheduler.c:483, sched_make_allocation(sched, job->schedobj)), not job->resreq. sched_make_primary_allocation() (scheduler.c:315) reads xce_primary_resreq_t* primary = obj->u.primary->resreq -- i.e. job->schedobj->u.primary->resreq, a pointer set exactly once, at ingest, by core_build_primary_schedobj()/make_primary_schedobj() (primary->resreq = resreq, aliasing job->resreq->u.primary at that moment). job->schedobj is built ONLY in core_build_job() (core_events.c:259-265) and make_sched_job() (schedobj.c:194-202) -- confirmed via grepping every `job->schedobj =` assignment in the scheduler; neither update path is among them. So after any resreq update, job->schedobj->u.primary->resreq keeps pointing at the ORIGINAL, ingest-time xce_primary_resreq_t forever, completely disconnected from the freshly-parsed newreq the update path installed into job->resreq. Any change an operator makes to a pending job's select/rusage/task-count via a resource-request update is silently invisible to the scheduler's own dispatch decision -- only a job's original, at-submission resreq is ever actually evaluated.", + "fix": "NOT FIXED -- discovered during resreq-maxtasks-wiring's design review (2026-09-05); deliberately kept out of that spec's scope (it is not about task-count wiring, and fixing it changes long-standing update semantics far beyond this feature's boundary). Needs its own dedicated bugfix task/spec: the update paths must rebuild (or re-point) job->schedobj's primary->resreq to the newly-parsed newreq, mirroring how core_build_job()/make_sched_job() construct it at ingest, probably by extracting a shared 'build/refresh primary schedobj from a resreq' helper both ingest and update can call.", + "tags": [ + "scheduler", + "job-update", + "resreq", + "schedobj", + "pre-existing", + "severity-high", + "resreq-maxtasks-wiring-discovery" + ], + "related_bugs": [ + "bug-1770" + ], + "occurrences": 1, + "last_seen": "2026-09-05T00:00:00Z" + }, + { + "id": "bug-1774", + "timestamp": "2026-09-05T00:00:00Z", + "error_message": "OPEN, NOT A CRASH, DEAD-CODE-ONLY -- pre-existing gap discovered during design review for the resreq-maxtasks-wiring spec, sibling to bug-1772, CORRECTED 2026-09-05: this entry originally claimed make_sched_job()'s ingest path was \"confirmed live, not legacy\" -- that claim was WRONG and has been retracted (see root_cause). make_primary_schedobj() (src/scheduler/src/schedobj.c:45-63, backing make_sched_job()) never computes primary->resreq_fp, unlike core_build_primary_schedobj() (src/scheduler/src/core_events.c:130-135) which sets resreq_fp.min_tasks/max_tasks/select_node_id/rusage_hash from the parsed xce_primary_resreq_t immediately after parsing -- but make_sched_job() itself is unreachable in production today (see root_cause), so this gap currently has ZERO live impact.", + "root_cause": "CONFIRMED via direct code read that make_primary_schedobj() calloc's the primary schedobj and leaves resreq_fp all-zero, with no equivalent computation anywhere in the function -- identical failure shape to bug-1772 (snapshot_load.c's snapshot_build_primary_schedobj() has the same gap), a third instance of the recurring schedobj-reconstruction-duplication pattern already tracked by bug-1767. HOWEVER, independently re-verified 2026-09-05 (two separate investigations, exhaustive grep of every call site of sched_update(), sched_update_jobs(), and make_sched_job() across src/ outside testsuite/): sched_update(xce_scheduler_t*) (scheduler.c:1200) has ZERO live callers anywhere in this codebase -- only its own declaration/definition and doc-comments describing it as dead/superseded exist (matching bug-1740's already-established finding for the sibling sched_apply_job_rusage() case, same root architecture). sched_update_jobs() (scheduler.c:1109) is called only from inside sched_update()'s switch, and make_sched_job() (schedobj.c:153) is called only from sched_update_jobs() -- so the ENTIRE chain is unreachable in production. The real, live production job-ingest paths are exactly two: core_build_job() (core_events.c, fresh submission via the driver-translated XCE_CORE_EVENT_JOBS_PUSH path) and snapshot_load.c's restore path -- both confirmed live by the same grep rigor. This entry's original \"confirmed live, not legacy\" parenthetical was an unverified, incorrect aside and is hereby retracted; make_sched_job()'s resreq_fp gap is real but currently dead-code-only, with no production consequence unless/until something makes this path reachable again.", + "fix": "NOT FIXED, and NOT CURRENTLY WORTH FIXING given zero live impact -- discovered during resreq-maxtasks-wiring's design review (2026-09-05), corrected same day once the dead-code status was established. resreq-maxtasks-wiring's own Requirement 5.4 does NOT need an exception for this path (it can be excluded from consideration entirely, not merely excluded-and-documented, since it never ingests any real job) -- that spec's actual Req 5.4 exception is for bug-1772's snapshot-restore gap, which IS live. If make_sched_job()'s dead code path is ever revived or repurposed in a future change, revisit this fix then: add the same resreq_fp computation core_build_primary_schedobj() does to make_primary_schedobj(), or better, per bug-1767's broader recommended remediation, share one real schedobj-construction implementation across all three call sites instead of maintaining three independent, drift-prone copies (two live, one dead).", + "tags": [ + "scheduler", + "resreq-fingerprint", + "dead-code", + "schedobj-duplication", + "pre-existing", + "severity-low", + "resreq-maxtasks-wiring-discovery", + "corrected" + ], + "related_bugs": [ + "bug-1767", + "bug-1770", + "bug-1772" + ], + "occurrences": 1, + "last_seen": "2026-09-05T00:00:00Z" + }, + { + "id": "bug-1775", + "timestamp": "2026-09-06T00:00:00Z", + "error_message": "xce-test-multihost's run-multihost-verify.sh (resreq-maxtasks-wiring task 4.3, array-job scenario): verify_task_started_on_host()'s grep -E pattern embedded a raw jobid string directly in an extended-regex alternation. A bracketed array-element jobid (e.g. \"[0]\") has its \"[0]\" misparsed by grep -E as a one-character bracket-expression class (matching a literal '0') instead of the literal 3-character substring \"[0]\" -- causing a false negative: `grep -qE 'Job base[0] ntasks is:'` does not match the literal log line `\"Job base[0] ntasks is: 5\"`.", + "root_cause": "Extended regular expressions treat unescaped `[...]` as a bracket-expression (character class), not a literal substring. Array-element jobids introduced by task 4.3 are the first jobid format in this harness containing ERE metacharacters (plain jobids are digits/hyphens only); the pre-existing plain-job code path never hit this because grep -E on a digits-and-hyphens-only string has no metacharacters to misparse.", + "fix": "FIXED (2026-09-06, resreq-maxtasks-wiring task 4.3): added ere_escape() to run-multihost-verify.sh, applied to any jobid string before use in a grep -E pattern (escapes '[' and ']', the only metacharacters a jobid can contain per fmt_jobid_ex/REGEX_JOBARRAY_ID's digit/hyphen/bracket-only format). Confirmed a byte-identical no-op for plain (non-array) jobids. Reproduced the false negative standalone before the fix and confirmed the escaped pattern matches correctly after, independently re-verified by review.", + "tags": [ + "test-infrastructure", + "shell-script", + "regex", + "array-job", + "xce-test-multihost", + "severity-low" + ], + "related_bugs": [ + "bug-1770" + ], + "occurrences": 1, + "last_seen": "2026-09-06T00:00:00Z" } ] }