Validate vLLM Router on GB200: DEP4, DEP8, and 1P/2D - #2549
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
cquil11
added
agentx-fast
|
Claude finished @cquil11's task in 7m 29s —— View job PR Review
Summary: No blocking issues found; two 🟡 warnings posted inline. First, the unscoped --- · Branch |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| case "$MODEL_PREFIX" in | ||
| minimaxm2.5|minimaxm3|kimik2.5|kimik3|qwen3.5) return 0 ;; | ||
| dsr1|minimaxm2.5|minimaxm3|kimik2.5|kimik3|qwen3.5) return 0 ;; | ||
| esac | ||
| # dsv4 multinode runs only under dynamo-vllm on watchtower, which likewise | ||
| # needs the srt-slurm workspace/outputs on a compute-visible shared FS | ||
| # (the runner home is not cross-mounted to compute nodes). | ||
| [[ "$FRAMEWORK" == "dynamo-vllm" && "$MODEL_PREFIX" == "dsv4" ]] && return 0 | ||
| [[ ( "$FRAMEWORK" == "dynamo-vllm" || "$FRAMEWORK" == "vllm-router" ) && "$MODEL_PREFIX" == "dsv4" ]] && return 0 |
There was a problem hiding this comment.
🟡 WARNING: Adding bare dsr1 to the model-prefix case flips every existing dsr1 GB200 multinode lane onto the Watchtower shared-FS path, not just the new vLLM Router validation.
Why it matters: runners.yaml maps the plain gb200 runner group to the same gb200-nv_* machines, so dsr1-fp4-gb200-dynamo-trt, dsr1-fp8-gb200-dynamo-trt, dsr1-fp8-gb200-dynamo-sglang, and dsr1-fp4-gb200-dynamo-sglang all go through this launcher. This change silently relocates their srt-slurm workspace/outputs to /mnt/lustre01/users-public/sa-shared/gha-runs, switches venv seeding to /usr/bin/python3, and re-stages INFMAX_WORKSPACE — none of which is exercised by this PR's two vLLM Router smoke jobs, and it contradicts the PR's stated isolation goal.
Fix: Scope the dsr1 entry to the router framework, mirroring the dsv4 line below:
| case "$MODEL_PREFIX" in | |
| minimaxm2.5|minimaxm3|kimik2.5|kimik3|qwen3.5) return 0 ;; | |
| dsr1|minimaxm2.5|minimaxm3|kimik2.5|kimik3|qwen3.5) return 0 ;; | |
| esac | |
| # dsv4 multinode runs only under dynamo-vllm on watchtower, which likewise | |
| # needs the srt-slurm workspace/outputs on a compute-visible shared FS | |
| # (the runner home is not cross-mounted to compute nodes). | |
| [[ "$FRAMEWORK" == "dynamo-vllm" && "$MODEL_PREFIX" == "dsv4" ]] && return 0 | |
| [[ ( "$FRAMEWORK" == "dynamo-vllm" || "$FRAMEWORK" == "vllm-router" ) && "$MODEL_PREFIX" == "dsv4" ]] && return 0 | |
| case "$MODEL_PREFIX" in | |
| minimaxm2.5|minimaxm3|kimik2.5|kimik3|qwen3.5) return 0 ;; | |
| esac | |
| # dsv4 multinode runs only under dynamo-vllm on watchtower, which likewise | |
| # needs the srt-slurm workspace/outputs on a compute-visible shared FS | |
| # (the runner home is not cross-mounted to compute nodes). | |
| [[ ( "$FRAMEWORK" == "dynamo-vllm" || "$FRAMEWORK" == "vllm-router" ) && "$MODEL_PREFIX" == "dsv4" ]] && return 0 | |
| # dsr1 needs the shared FS only for the exact-head native vLLM Router | |
| # validation; established dynamo-trt/dynamo-sglang dsr1 lanes keep the | |
| # runner-local workspace. | |
| [[ "$FRAMEWORK" == "vllm-router" && "$MODEL_PREFIX" == "dsr1" ]] && return 0 |
| mkdir -p configs | ||
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/configs" configs |
There was a problem hiding this comment.
🟡 WARNING: This exact-ref branch stages only configs/ — it never copies the new benchmarks/multi_node/srt-slurm-recipes/sglang/glm5.2/ recipe into the srt-slurm checkout.
Why it matters: The new glm5.2-fp8-h200-sgl-router-agentic-mtp-2p2d master-config entry points at CONFIG_FILE=recipes/sglang/glm5.2/agentic/disagg-h200-2p2d-pcp8-tp8-dp8-mtp-sgl-router.yaml and routes through this branch. Every other exact-ref/overlay lane (B200 copies recipes/{vllm,sglang}/deepseek-v4, GB200 copies all of recipes/vllm) overlays its in-repo recipe onto the clone; here the recipe YAML added by this PR is never staged, so srtctl will only find it if the pinned srt-slurm commit ab98030f happens to ship an identical copy — which would make the in-repo file dead weight that can silently drift from what actually runs.
Fix: Overlay the glm5.2 recipe like the other launchers do (or confirm the pinned srt-slurm head carries it and drop the in-repo copy):
| mkdir -p configs | |
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/configs" configs | |
| mkdir -p recipes/sglang/glm5.2 configs | |
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/sglang/glm5.2" recipes/sglang/glm5.2 | |
| cp -rT "$GITHUB_WORKSPACE/benchmarks/multi_node/srt-slurm-recipes/configs" configs |
There was a problem hiding this comment.
Beyond the inline nits, this run also checked a few other things and ruled them out: the new SGL Router recipes' frontend.type: sglang (matches the SGLang backend's framework detection, not a router-specific bug), wait_for_slurm_job_success's awk match against heterogeneous/disagg srtctl job IDs (matches correctly), and uses_watchtower_shared_fs unconditionally capturing dsr1 for unrelated dynamo-trt recipes (harmless, since dynamo-trt dsr1 already runs on watchtower shared FS).
Extended reasoning...
This is a large, multi-file infrastructure PR (3 launcher scripts, new router framework wiring, exact-commit srt-slurm pinning, new recipes) touching critical benchmark CI paths, so a human look is warranted regardless of the two non-blocking nits already left inline. During this run, verifier agents also examined and ruled out four other candidate issues raised by finders (listed in the message above) — recording them here so they are not re-explored from scratch in a future pass.
| export MODEL_PATH="/mnt/lustre01/models/MiniMax-M3-MXFP8" | ||
| export SRT_SLURM_MODEL_PREFIX="minimax-m3-mxfp8" | ||
| else | ||
| echo "Unsupported model prefix/precision combination: $MODEL_PREFIX/$PRECISION. Supported combinations for dynamo-vllm: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8" | ||
| echo "Unsupported model prefix/precision combination: $MODEL_PREFIX/$PRECISION. Supported combinations for vLLM backends: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8" | ||
| exit 1 | ||
| fi | ||
| else |
There was a problem hiding this comment.
🟡 runners/launch_gb200-nv.sh:198 — the else-branch error string for the dynamo-vllm||vllm-router dispatch (Supported combinations for vLLM backends: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8) doesn't include dsr1/fp4, even though this PR adds a working elif branch for that exact combo just above (lines 183-187) and rewords this very error line in the same diff. Add dsr1/fp4 to the list so a future genuinely-unsupported combo reports the full supported set.
Extended reasoning...
What the bug is. runners/launch_gb200-nv.sh dispatches on $FRAMEWORK == "dynamo-vllm" || $FRAMEWORK == "vllm-router" and then matches $MODEL_PREFIX/$PRECISION pairs one by one. This PR adds a new elif [[ $MODEL_PREFIX == "dsr1" && $PRECISION == "fp4" ]] branch (lines 183-187) that resolves the official vLLM Blackwell NVFP4 DeepSeek-R1 checkpoint (MODEL_PATH=/mnt/lustre01/models/DeepSeek-R1-0528-NVFP4-v2, SRT_SLURM_MODEL_PREFIX=dsr1-nvfp4), making dsr1/fp4 a genuinely supported combination on this dispatch path for the first time.\n\nThe code path that triggers it. The trailing else (line 198) is the catch-all for any MODEL_PREFIX/PRECISION pair that doesn't match one of the elif branches; it prints a diagnostic listing every combination that IS supported, then exit 1s. That diagnostic string is: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8. dsr1/fp4 is missing from it despite now being handled by its own elif two branches above.\n\nWhy existing code doesn't prevent it. The list is a hand-maintained string literal with no connection to the actual elif chain, so nothing enforces that it stays in sync when a new branch is added. Notably, this PR did touch this exact line — the diff shows the surrounding text changed from for dynamo-vllm to for vLLM backends (widening the wording to also cover the new vllm-router framework) — so the author edited this string in the same change that introduced dsr1/fp4, but didn't add the new combo to the enumeration.\n\nImpact. Purely diagnostic. dsr1/fp4 itself works correctly: it's caught by its own elif and never falls through to the else. The only effect is that if some future, truly-unsupported combo (e.g. dsr1/fp8) hits this else, the printed "supported combinations" list under-reports what's actually supported, which could send someone debugging a launcher failure down the wrong path for a moment. No job fails, no wrong model gets served, and no test would need to change.\n\nStep-by-step proof.\n1. Set FRAMEWORK=vllm-router, MODEL_PREFIX=dsr1, PRECISION=fp8 (an unsupported combo).\n2. The dispatch enters the elif [[ $FRAMEWORK == "dynamo-vllm" || $FRAMEWORK == "vllm-router" ]] block.\n3. None of kimik2.5/fp4, kimik3/fp4, dsv4/fp4, dsr1/fp4 (this PR's new branch requires fp4, not fp8), minimaxm2.5/*, or minimaxm3/fp8 match, so execution falls to the else at line 198.\n4. The script prints: Unsupported model prefix/precision combination: dsr1/fp8. Supported combinations for vLLM backends: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8 and exits 1.\n5. A developer reading that message would reasonably conclude dsr1 isn't supported at all on this path in any precision — when in fact dsr1/fp4 (added by this very PR) works fine. The message should also list dsr1/fp4.\n\nFix. Append dsr1/fp4 to the enumerated list on line 198, e.g.: Supported combinations for vLLM backends: kimik2.5/fp4, kimik3/fp4, dsv4/fp4, dsr1/fp4, minimaxm2.5/fp4, minimaxm2.5/fp8, minimaxm3/fp8.\n\nAll four independent verifiers agreed this is real but non-blocking (diagnostic-only, no functional impact), so I'm filing it as a nit.
| squeue -j "$job_id" --noheader 2>/dev/null | grep -q "$job_id" | ||
| } | ||
|
|
||
| wait_for_slurm_job_success() { | ||
| local job_id="$1" | ||
| local attempts="${SLURM_ACCOUNTING_ATTEMPTS:-24}" | ||
| local interval="${SLURM_ACCOUNTING_INTERVAL_SECONDS:-5}" |
There was a problem hiding this comment.
🟡 This PR copy-pastes the same ~9-line SRT_SLURM_REPOSITORY/SRT_SLURM_REF exact-pin clone/checkout/verify block into all three launchers it touches (runners/launch_b200-dgxc.sh:161-172, runners/launch_gb200-nv.sh:418-429, runners/launch_h200-dgxc-slurm.sh:67-78), and the copies have already drifted: b200/gb200 append || exit 1 after every git command while h200 omits it. Since runners/slurm_utils.sh already centralizes shared launcher behavior (this PR adds wait_for_slurm_job_success there), consider extracting a clone_srt_slurm_pin(repo, ref, target_dir) helper into slurm_utils.sh and calling it from all three launchers.
Extended reasoning...
This PR introduces exact-commit pinning for SRT_SLURM_REPOSITORY/SRT_SLURM_REF in three separate launcher scripts, and the ~9-line block that implements it — validating the two env vars are set together, git clone, cd, git checkout --detach, and verifying git rev-parse HEAD matches the pinned ref with an identical error message — is duplicated verbatim across runners/launch_b200-dgxc.sh (lines 161-172), runners/launch_gb200-nv.sh (lines 418-429), and runners/launch_h200-dgxc-slurm.sh (lines 67-78).
The duplication has already produced drift within this same PR: the b200 and gb200 copies append || exit 1 after each of the git clone, cd, and git checkout --detach commands, while the h200 copy omits it entirely (relying instead on the scripts set -eo pipefail). That is exactly the kind of divergence that copy-pasted logic invites — a future change to error handling, retry behavior, or the checkout flag would need to be applied three times, and nothing enforces that the three copies stay in sync.
runners/slurm_utils.sh is the established home for logic shared across these launchers — this very PR adds wait_for_slurm_job_success there alongside the existing stream_slurm_job_log, and all three launchers already source it. A clone_srt_slurm_pin(repo, ref, target_dir) helper following the same pattern would let each launcher call one function instead of maintaining three copies of the validate+clone+checkout+verify sequence, and would eliminate the || exit 1 inconsistency (and any future ones) at the source.
As a concrete illustration: if a maintainer later wants to add a shallow-clone flag (--depth 1) to speed up CI, they would need to remember to edit all three files identically. Missing one (as already happened with || exit 1) leaves that launcher silently behind — with the shared helper, the change happens once in slurm_utils.sh and all three launchers pick it up automatically.
The per-launcher lines that come after the clone (each launcher copies different recipe subdirectories into the cloned repo with mkdir -p/cp -rT) legitimately differ and should stay at the call sites — only the validate/clone/checkout/verify portion is identical across all three and is the right scope for extraction.
This is a maintainability/reuse cleanup, not a correctness bug: the code works correctly in all three launchers as written, so it does not block merging.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31419549680 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31425550346 |
…00-smoke # Conflicts: # perf-changelog.yaml
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31426276922 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31430860441 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31431838034 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31434053812 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31436843969 |
cquil11
changed the title
|
Exact-head GB200 Router validation is complete: workflow 31436843969 is green for aggregate DEP4, multinode aggregate DEP8, and disaggregated 1P/2D DEP4. The PR description now contains the per-job request counts, coverage, metrics artifacts, consistent-hash affinity audit, P/D two-stage evidence, and uploaded Router-log evidence. |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=31502477920 |
1 participant
Summary
frontend.type: vllm-routerfrom SemiAnalysisAI/srt-slurm commitb4bf6eb0abefa16c5108b339dabb903214a4c806nvidia/DeepSeek-R1-0528-NVFP4-v2, and consistent-hash routing keyed byX-Session-IDgpu-memory-utilization: 0.85) while retaining ample KV capacityInferenceX changes
AIPERF_MAX_CONTEXT_LENGTHpath to the shared AgentX replay builder; recipes that do not set it retain existing behaviorX-Session-IDand configure official Router consistent hashingvllm:metrics and pass one explicit metrics URL per logical vLLM worker leaderEarlier hardware validation
Earlier tested InferenceX head:
395d57f0e1355628e84e12b9826d0ee634d9c631Full workflow: 31436843969 — success
COMPLETED 0:0COMPLETED 0:0COMPLETED 0:0Every job returned
replay_rc=0, passed the requiredvllm:metrics-prefix gate, and uploaded nonempty request and server-metrics artifacts.Router behavior proved from live and uploaded logs
X-Session-IDextraction, explicit prefill and decode selections, HTTP 200 decode responses, and repeatedTwo-stage processing completed successfullyMetrics behavior
/metricsendpoint in this mode, so AIPerf reports 3/4 while the required backend metrics and finalvllm:gate passFailure-driven correction
The preceding exact-head attempt proved topology, Router, dataset, and metrics plumbing but all three workers later OOMed during long-context warmup at
gpu-memory-utilization: 0.90. The failures were vLLM CUDA allocation failures, not Router failures. Reducing the reservation to 0.85 leaves roughly 9 GiB more activation workspace per GPU while still providing about 47 GiB KV per DEP4 rank and 86 GiB per DEP8 rank. The successful workflow above validates that correction under the full fast profile.Static validation
git diff --checkpassThis validation branch is stacked on the existing Router integration work while keeping the three GB200 evidence points isolated and reproducible.
Final Router-readiness validation
Current InferenceX head:
66ea40cc7060dfa84b492db6f24fe9c18f8cde5bPinned srt-slurm head:
b4bf6eb0abefa16c5108b339dabb903214a4c806Readiness correction
The official Router considers a pool ready after any one unique backend host is healthy, while
/workerscan list all DP-expanded URLs before every registered base API is usable. The earlier srt-slurm parser counted registered URLs without requiring every underlying base, which allowed a 2P/2D benchmark to start while one advertised P/D host was still unavailable.The srt-slurm fix keeps this behavior behind the vLLM Router adapter: after the expected expanded
/workerscount passes, it derives every unique direct/healthURL from the logical bases advertised to Router and requires HTTP 200 from all of them. Other frontend readiness semantics are unchanged. Focused tests pass 54/54; the full local suite passes 942 tests, with five unrelated pre-existing platform/path/mock failures; exact-head srt-slurm CI is green.Disaggregated 2P/2D evidence
Job 93681770486 succeeded on Slurm 22971. Router expanded to 8P + 8D; the all-base barrier held until all four P/D APIs were healthy; the realized AIPerf command contained all four explicit metrics URLs and captured 16 backend engine series. The profile completed 518/518 successful with 0 errors, passed the required
vllm:metric gate, and repeated session IDs retained stable affinity independently in prefill and decode pools. Six artifacts, including Router and all server logs plus raw/aggregate AgentX results, are present.The parent workflow 31460081245 is red only because its workflow definition came from an unrelated
mainrevision whose missingEVALSassignment causedfromJson('')at graph construction. Every materialized workload/result job succeeded; this is not a Router or benchmark failure.Aggregate multinode DEP8 evidence
Workflow 31497389903 and job 93798572870 are green on exact tested InferenceX head
08f55e5c7d83a86ee4474c3db29cf5c818924716; Slurm 22973 completed0:0./healthendpoints before AIPerf started.vllm:gate, and successfully collected Router plus both explicit backend metrics endpoints.10.30.1.165:6100and engines 4-7 from10.30.1.39:6100.X-Session-IDmappings across 46 sessions, 34 repeated sessions, zero affinity changes, and routed traffic on all eight ranks.gb200-nv: 1/1.Head
66ea40cc7only synchronizes newer upstream content and resolves the append-only changelog comparison after that exact run; it does not change the validated Router runtime/configuration from08f55e5c7.Current-head official sweep
The official InferenceX sweep on current integration head
66ea40cc7060dfa84b492db6f24fe9c18f8cde5bis fully green: workflow 31502477920 completed withsuccess.COMPLETED 0:0COMPLETED 0:0COMPLETED 0:0COMPLETED 0:0Every performance point passed the required
vllm:metrics gate. Live logs confirm that the Router expanded the expected DP ranks and the new direct barrier required all advertised base APIs before each benchmark. Every eval completed all 1,319 GSM8K requests witheval_exit=0; collect-evals also succeeded. The workflow published 22 nonexpired artifacts (about 115 MB), including raw/aggregate performance results, all evaluation outputs, and Router plus server logs for every topology.