Skip to content

[https://nvbugs/6561778][fix] Fence all ranks before pytest launch in multi-node slurm_run.sh #17372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trtllm-agent wants to merge 1 commit into
base: main
Choose a base branch
from
+55 −1
Open

[https://nvbugs/6561778][fix] Fence all ranks before pytest launch in multi-node slurm_run.sh #17372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions jenkins/scripts/slurm_run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,61 @@ if [ "${SLURM_JOB_NUM_NODES:-1}" -eq 1 ] || \
done
fi

# The install lock in slurm_install.sh lives under $resourcePathNode (/tmp), so it
# is node-local: its wait loop only fences $SLURM_LOCALID peers on the same node,
# and a node can never observe another node's lock. Nothing else stops one node
# from reaching `eval $pytestCommand` below while another is still installing, and
# the per-rank work above skews the ranks further (non-zero ranks cover the
# coverage-config write with a blind `sleep 30`, and slurm_setup_runtime_env shells
# out to pip3). Pytest's first action is `import tensorrt_llm`, whose module-scope
# MPI collective must be entered by every rank; under --mpi=pmix -- added exactly
# when nodeCount > 1 -- that collective has a 300s fence timeout, so the skew
# aborts every rank instead of merely running late. Fence every rank on the shared
# $jobWorkspace so they enter pytest together.
slurm_wait_all_ranks() {
local numRanks="${SLURM_NTASKS:-1}"
if [ "$numRanks" -le 1 ] || [ -z "${jobWorkspace:-}" ]; then
return 0
fi

# Keyed per job *and* per step: $jobWorkspace outlives a single step, so
# markers from another job, or from an earlier step of this job, must not
# satisfy the count. Slurm assigns one step id per step across all of its
# nodes, so every rank of a step agrees on this path.
local readyDir="$jobWorkspace/run_ready_job_${SLURM_JOB_ID:-local}_step_${SLURM_STEP_ID:-0}"
mkdir -p "$readyDir"
touch "$readyDir/rank_${SLURM_PROCID}.ready"

# Bounded so a dead rank fails the stage loudly instead of hanging until the

@brnguyen2 brnguyen2 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stated justification for 3600s doesn't hold. slurm_install.sh has several retry_command --timeout 2700 calls plus a --timeout 1800 wget, so its worst-case budget is well over an hour, and the non-LOCALID-0 branch waits on the lock with no timeout at all. "Exceeds the 2700s pip3 retry budget" is a per-command bound, not a bound on install duration.

What 3600s actually bounds is arrival skew — each rank starts its own deadline after finishing its own install — which is the right thing to bound and comfortably above the ~10min skew in the bug. Worth saying that instead, since the current wording invites someone to "fix" the number against the wrong quantity later.

# partition walltime kills it; the ceiling exceeds the 2700s pip3 retry budget
# in slurm_install.sh so a merely slow rank still releases the barrier.
local timeoutSecs=3600
local deadline=$((SECONDS + timeoutSecs))
local markers ready
while true; do
# Counted with a glob rather than `ls | wc -l`: under `set -Eeuo pipefail` a
# failing `ls` propagates into the assignment and fires the ERR trap. The
# touch above guarantees at least one match, so no nullglob is needed.
markers=("$readyDir"/*.ready)
ready=${#markers[@]}
if [ "$ready" -ge "$numRanks" ]; then
return 0
fi
if [ "$SECONDS" -ge "$deadline" ]; then
echo "ERROR: rank ${SLURM_PROCID} timed out after ${timeoutSecs}s waiting for" \
"all $numRanks ranks to be ready; ready: $ready/$numRanks"
return 1
fi
# One rank reports progress; all of them would spam the log every 10s.

@brnguyen2 brnguyen2 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard doesn't do what the comment says. The script runs under set -xEeuo pipefail (line 4) and never disables tracing before here, so every rank already emits the full trace of the loop body — the glob assignment, both [ tests, the sleep — to the single #SBATCH --output= log every 10s. Suppressing the echo on non-zero ranks removes one line out of ~6 per rank per iteration.

Either wrap the loop in set +x / set -x (and keep the rank-0 echo as the only progress signal), or drop the guard and the comment.

if [ "$SLURM_PROCID" -eq 0 ]; then
echo "(Waiting for all $numRanks ranks to be ready) ready: $ready/$numRanks"
fi
sleep 10
done
}

slurm_wait_all_ranks

# Turn off "exit on error" so the following lines always run
set +e

Expand Down
1 change: 0 additions & 1 deletion tests/integration/test_lists/waives.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ accuracy/test_llm_api_autodeploy.py::TestQwen3_5_397B_MoE::test_bf16_small[4] SK
accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_fp8_blockscale[throughput] SKIP (https://nvbugs/6561775)

@brnguyen2 brnguyen2 Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal is the only user-visible part of the PR and it is the least-verified. The PR body says the fence "can't be exercised by the pytest verify run at all (a warm container never executes slurm_run.sh)" — so the code path being fixed was never executed end-to-end, only an extracted copy of the helper was.

Please post /bot run --extra-stage "GB200-8_GPUs-2_Nodes-PyTorch-Post-Merge-1" and link the green run before merging; otherwise this re-enables a test in post-merge on a mechanism argument alone.

accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_fp8_blockscale[throughput_mtp] SKIP (https://nvbugs/6428101)
accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_fp8_blockscale[throughput_mtp_trtllm] SKIP (https://nvbugs/6426868)
accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_nvfp4_multi_gpus[latency] SKIP (https://nvbugs/6561778)
accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_nvfp4_multi_gpus[latency_adp_lmtp] SKIP (https://nvbugs/6561777)
accuracy/test_llm_api_pytorch.py::TestDeepSeekR1::test_nvfp4_multi_gpus[throughput_pp4_mtp] SKIP (https://nvbugs/6481323)
accuracy/test_llm_api_pytorch.py::TestDeepSeekV32::test_dsa_host_cache_offload[host_cache_offload_mtp1] SKIP (https://nvbugs/6384357)
Expand Down
Loading