slurm: batch the queries and parallelize the writes in the GPU power/clock helpers

[100milliongold]

Problem

set_gpu_power_levels.sh and set_gpu_clocks.sh call nvidia-smi once per GPU to read the target value and once more to apply it, all serially. Both nvidia-smi -pl and nvidia-smi -ac take roughly a second per GPU, so on an 8-GPU node the two helpers together add about 8 s to the prolog of every job for which 50-exclusive-gpu runs. srun surfaces it as:

srun: Prolog hung on node <node>

Observed on DGX OS 7.5.0 (8× B300), Slurm 26.05.1.

Fix

• Read the values for all GPUs in a single --query-gpu call (the per-GPU -i loop was only needed because the value was read one at a time).
• Apply them in parallel, collecting each child's exit status so a failure on any GPU still fails the script.

The same values are written to the same GPUs; only the number of nvidia-smi invocations and their concurrency change. The default branch of set_gpu_clocks.sh already operated on all GPUs at once and is untouched.

Verification

Not yet timed with the patched scripts on hardware — the system where this was found has 50-exclusive-gpu removed from prolog.d (an 8-GPU node shared between jobs should not have every job reset limits and clocks on all GPUs). The serial cost is reproducible there: prolog took 6–8 s per job while the script was in place. Marked as draft for that reason; happy to run a timed before/after if that would help.

Related

The reason every job ran 50-exclusive-gpu in the first place is a separate defect in the exclusive-job detection, addressed in #1391.

[dholt]

The batched queries at set_gpu_power_levels.sh:21 and set_gpu_clocks.sh:13-14 run in process substitutions, whose failures are invisible to readarray and set -e. If a query fails with empty output, the loop is empty and the helper exits 0; partial output can update only some GPUs and also return success. Capture each query through a construct whose status can be checked, validate that all required per-GPU rows are present and aligned, and only then launch writes. Please cover empty, partial, and nonzero query results as required by the changed-path evidence.

---

Automated triage review (agent-generated on the maintainer's behalf; a human maintainer decides merges).

[100milliongold]

Thanks — the process-substitution point is correct, and the failure modes were worse than "invisible": all four of them returned success.

What changed (commit c95d3a2):

• The query result now goes through a file so its exit status can be checked.
• The index is selected alongside the values (--query-gpu=index,<field>), so a write targets the GPU that nvidia-smi reported instead of assuming the array subscript equals the GPU index.
• Every row is parsed and range-checked, and the row count is compared against nvidia-smi -L, before the first write is launched. A partial result is rejected rather than silently applied.
• set_gpu_clocks.sh selected clocks.max.mem and clocks.max.sm in two separate queries; if they returned different row counts, ${maxMEM[$i]} was empty for the trailing GPUs and produced an -ac ,1980 argument. Both values now come from the same query, so they cannot drift out of alignment.

Coverage of the cases you asked for, verified on a DGX B300 (Ubuntu 24.04, bash 5.2) with an nvidia-smi stub that honours --query-gpu and records writes instead of performing them. Identical results for both helpers:

case	before	after
ok (8 GPUs)	rc=0, 8 writes	rc=0, 8 writes
empty output	rc=0, 0 writes	rc=1, 0 writes
partial output (3 of 8)	rc=0, 3 writes	rc=1, 0 writes
nonzero exit	rc=0, 0 writes	rc=1, 0 writes
non-numeric value	rc=0, 8 writes (N/A passed through)	rc=1, 0 writes

One limitation of the stub is worth stating plainly: it records writes rather than performing them, so the non-numeric row shows 8 writes for the old code. On real hardware nvidia-smi -pl N/A fails and wait would surface rc=1 — but only after eight bad invocations. The new code rejects the row before the first one.

The ok case is unchanged, so the parallel-write speedup this branch adds is preserved.

[dholt]

The checked primary queries and explicit child waits address the earlier failure-handling concerns. Two issues remain in the GPU helper templates:

• Their Bash array-length expressions are interpreted as Jinja comment starts when Ansible renders the files, producing a missing-comment-end error. Protect the literal Bash expressions and add a test that renders the actual templates.
• The secondary nvidia-smi -L | grep -c pipeline still loses the producer's exit status. Partial output followed by a nonzero exit can pass validation and reach GPU writes. Check that command's status before parsing/counting, with a regression for partial output plus failure.

These are specific to the changed helpers; the separate epilog ownership work is tracked in #1407.

[100milliongold]

Both remaining points are addressed in 50ad7d0.

1. Jinja comment collision

Reproduced first, parsing the templates exactly as they stood on this branch:

FAIL  set_gpu_power_levels.sh: line 59: Missing end of comment tag
FAIL  set_gpu_clocks.sh:       line 53: Missing end of comment tag

Neither file uses Jinja at all ({{ and {% both appear zero times), so rather than adding escaping machinery to a shell script, the row count is now kept in a variable incremented as rows are read:

rows=0
...
    indexes+=("$index")
    limits+=("$limit")
    rows=$((rows + 1))
done < "$tmp"

if [ "$rows" -ne "$expected" ]; then

After the change, both parse:

OK    set_gpu_power_levels.sh
OK    set_gpu_clocks.sh

2. Test that renders the actual templates

scripts/deepops/check-template-syntax.py walks roles/*/templates and parses every file. It only parses, never renders, so it needs no variables and no inventory — which also means it runs where the slurm role's molecule scenario cannot (the role is excluded from the Docker matrix in .github/workflows/molecule.yml).

Current tree:

skip  nvidia-dcgm-exporter/dcgm-exporter.yml.j2: No filter named 'zip_longest'.
skip  prometheus-node-exporter/node-exporter.yml.j2: No filter named 'zip_longest'.
skip  prometheus-slurm-exporter/slurm-exporter.yml.j2: No filter named 'zip_longest'.
skip  slurm/etc/slurm/slurm.conf: No filter named 'ternary'.
skip  slurm/etc/munge/munge.key.j2: No filter named 'password_hash'.

82 templates parsed, 5 skipped, 0 failed

Unknown filters raise TemplateAssertionError, which subclasses TemplateSyntaxError; they are matched by exact type and reported as skipped, so Ansible-provided filters do not turn into false failures while real syntax errors still fail the run. Run against this branch before the fix, it exits 1 on the two files above.

I did not wire it into a workflow in this PR — say the word if you would rather have it as a step in ansible-lint-roles.yml (ansible, and therefore Jinja, is already installed there) and I will add it.

3. nvidia-smi -L | grep -c status

if ! gpu_list=$(nvidia-smi -L); then
    echo "$0: could not list the GPUs" >&2
    exit 1
fi
expected=$(printf '%s\n' "$gpu_list" | grep -c '^GPU ') || expected=0
if [ "$expected" -eq 0 ]; then
    echo "$0: nvidia-smi -L reported no GPUs" >&2
    exit 1
fi

The zero-GPU case still exits 1, as it did when grep -c's exit 1 drove the old if !.

Evidence

nvidia-smi replaced by a stub. The dangerous shape is a listing that emits every line and then exits nonzero: the count matches the query rows, so the old validation had nothing to object to.

Case A — nvidia-smi -L prints 8 GPUs then exits 15; the value query returns 8 well-formed rows:

	exit	GPU writes
before, set_gpu_power_levels.sh	0	8
after, set_gpu_power_levels.sh	1	0
before, set_gpu_clocks.sh	0	8
after, set_gpu_clocks.sh	1	0

Case B — everything healthy (8 GPUs listed, exit 0):

	exit	GPU writes
after, set_gpu_power_levels.sh	0	8
after, set_gpu_clocks.sh	0	8

My first attempt at case A had the listing emit only 2 of 8 lines, which the old code already caught on the row-count mismatch — that shape was not the hole. The table above is the corrected one.

[dholt]

The revision fixes the two previously requested helper issues. CI now flags the syntax checker's implicit autoescape=False on the Jinja environment.

This helper only compiles templates and does not render HTML, so I do not see an HTML-output sink here. Making autoescape=True explicit is safe for this syntax-only use and does not change how Ansible renders deployment templates. Please apply the small suggestion and rerun CI to clear the CodeQL check.

[dholt]

Suggested change

	env = Environment(loader=FileSystemLoader(templates))
	env = Environment(loader=FileSystemLoader(templates), autoescape=True)

[100milliongold]

Applied in 76b30885. Verified with jinja2 3.1.6 that the checker's output is byte-identical before and after: 82 templates parsed, 5 skipped, 0 failed in both runs.

[100milliongold]

Applied in 76b30885.

I checked that it is inert here rather than taking it on the reasoning alone. With jinja2 3.1.6, running the checker before and after the change gives byte-identical output — diff reports no lines:

82 templates parsed, 5 skipped, 0 failed

Same five skips in both runs (zip_longest x3, ternary, password_hash), same zero failures. That is what I would expect from your point that the helper never renders: autoescape only changes the code the compiler emits for output nodes, and get_template() stops at compilation.

One process note against myself: my first attempt at this comparison used a sed that silently failed, so I was diffing the unmodified file against itself. The numbers above are from the run after confirming git diff showed the one-line change.

[dholt]

The current revision resolves the requested query-status, listing-status, template-rendering and syntax-checker findings. Independent review found no remaining functional blocker.

Validation: actual Ansible rendering of both helpers; 78 inert helper invocations including historical negative controls, sparse/reordered indices, malformed/failed queries and listings, all child-failure positions, and complete child waits. A separate 33-case rendered-helper run and focused role syntax check also passed. The syntax checker reports 82 parsed, 5 unknown-filter skips, 0 failures. All 15 current-head CI checks are green, and the earlier CodeQL thread is answered.

This validates the error handling and rendered scripts, not real-GPU timing or deployment compatibility. Retaining these isolated regressions in CI is a nonblocking follow-up. The unrelated epilog ownership work remains separate.