Validate vLLM P/D cache-source metrics on GB300 - #2797
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 关于重新运行失败任务的文档 |
|
Claude finished @cquil11's task in 3m 58s —— View job PR Review
Found 1 blocking issue and 1 warning (posted as inline comments):
Checks that passed: master config change is paired with a |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| - { tp: 8, spec-decoding: mtp, kv-offloading: dram, kv-offload-backend: { name: mooncake, version: "0.3.11.post1" }, conc-list: [6, 8] } | ||
|
|
||
| minimaxm3-fp8-h100-dynamo-vllm-agentic-disagg: | ||
| image: ttl.sh/cquil11-vllm-tier-d67b417bca-pr53087-20260901:24h |
There was a problem hiding this comment.
🔴 BLOCKING: The container image is hosted on ttl.sh, an ephemeral registry, with a :24h tag — the image is automatically deleted 24 hours after push (pushed 2026-09-01 per the tag).
Why it matters: By the time this merges (or on any post-merge sweep rerun), ttl.sh/cquil11-vllm-tier-d67b417bca-pr53087-20260901:24h will no longer be pullable by anyone. The benchmark then depends entirely on the pre-imported .sqsh file on the cluster's Lustre filesystem (the launcher at runners/launch_h100-dgxc-slurm.sh:120-123 hard-fails if it's absent), which is exactly the non-reproducible local-container pattern this repo disallows. This is the only ttl.sh image in either master config — every other entry uses a durable public registry (NGC, Docker Hub, ghcr, quay). The same image reference is also pinned in the recipe's identity.container.image (benchmarks/multi_node/srt-slurm-recipes/vllm/minimax-m3/h100-fp8/agentic/disagg-1p1d-tp8-tp8-c8-agentic.yaml:5,13), so the recorded identity becomes unverifiable once the tag expires.
Fix: Push the patched vLLM build (vLLM PR #53087) to a durable public registry — e.g. ghcr.io/<org>/vllm-tier:<tag> or a Docker Hub repo, ideally pinned by digest — and update the image: here plus the recipe's model.container and identity.container.image to match. Fix this →
| elif [[ $FRAMEWORK == "dynamo-vllm" ]]; then | ||
| CONTAINER_KEY="$IMAGE" | ||
| SQUASH_FILE="/mnt/nfs/lustre/containers/$(echo "$IMAGE" | sed 's/[\/:@#]/_/g').sqsh" | ||
| if ! unsquashfs -l "$SQUASH_FILE" >/dev/null 2>&1; then | ||
| echo "Error: expected pre-imported vLLM image at $SQUASH_FILE" >&2 | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🟡 WARNING: The new dynamo-vllm path requires a pre-imported squash file and exits if it's missing, instead of transforming the public Docker image to an enroot local image.
Why it matters: Nobody else can reproduce this benchmark — the run only works on a cluster where someone has already manually imported the image to /mnt/nfs/lustre/containers/. The single-node path in this same script (lines 340–355) already implements the import-with-lock pattern (enroot import -o "$SQUASH_FILE" docker://$IMAGE) and even uses the identical squash-file naming convention, so the fallback is readily available. I recognize the dynamo-sglang/dynamo-trt branches also assume pre-imported files, but those reference durable public images that anyone can import by hand; combined with the 24h-TTL ttl.sh image here, this check makes the config permanently irreproducible once the tag expires.
Fix: Once the image lives on a durable registry, replace the hard failure with the same lock-guarded enroot import -o "$SQUASH_FILE" docker://$IMAGE fallback used in the single-node branch (this path already runs before the Slurm job is submitted, so the import can run via a short srun/login-node step as appropriate for this cluster). If the pre-import requirement is intentional (e.g., import must not run on the login node), please add a comment explaining that and how the file is provisioned.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33525210094 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33526558499 |
cquil11
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c221f79. Configure here.
| - "CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/agg-gb300-tp4-mtp-agentic.yaml" | ||
| dsv4-fp4-gb300-dynamo-vllm-agentic-mtp-disagg: | ||
| image: vllm/vllm-openai:nightly-dev-arm64-cu13.0.1-426e59f | ||
| image: ttl.sh/cquil11-vllm-tier-7c5dc571-pr53087-d9b6d498-arm64-20260901:24h |
There was a problem hiding this comment.
Recipe container misses patched image
High Severity
The master image is now the patched ttl.sh overlay, but the selected srt-slurm recipe still pins official nightly 426e59f in model.container and identity.container.image. The GB300 launcher maps only the master image as the container-alias key, so the job fails lookup or starts the unpatched nightly without the metric overlay.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c221f79. Configure here.
| - "SYNTHETIC_ACCEPTANCE_LENGTH=2.49" | ||
| - "CONFIG_FILE=recipes/vllm/deepseek-v4/agentic/disagg-gb300-2p1d-dep8-dep12-c1152-mtp-agentic.yaml" | ||
| decode: | ||
| num-worker: 1 |
There was a problem hiding this comment.
Recipe omits cache-source metrics
High Severity
This config is now the sole point for source-labeled prompt-token cache validation, but the recipe it launches never enables kv-cache-metrics, enable-prompt-tokens-details, or AIPERF_REQUIRED_SERVER_METRIC_PREFIX. The removed H100 recipe set all three, so vLLM will not export source=p2p hits and AIPerf will not require them.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c221f79. Configure here.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33528895260 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33529621726 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33530938653 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33532600105 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33536314401 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33651546930 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33651546930 |
Use supporting image 4d3c4a23 with upstream hybrid load-failure recovery and the already validated async queue-progress fix. Preserve the single DEP4/DEP16 c256 point, full eval, and canonical warmup/profile. The previous run crashed on a Mooncake OBJECT_NOT_FOUND response followed by single-group tuple unpack in the scheduler. AI assistance used for diagnosis and validation.
Set the prefill MultiConnector failure policy to recompute so optional Mooncake cache eviction races recover through the tested hybrid path. Preserve fail-closed NIXL decode transfers. AI assistance used for diagnosis and validation.
…disagg-source # Conflicts: # perf-changelog.yaml
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33705137653 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33716740426 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=33769499103 |
1 participant
Summary
source="p2p", separately from Mooncake storeexternalhits.Current validation
Official full-sweep-enabled run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/33769499103
ttl.sh/cquil11-vllm-tier-pd-block-state-0d8739d0-arm64-20260903:24h.sha256:9d2df326911c5edd2cd91e16a05fd803323da228508f9e3a4b05d0047642dcf9.The validation image includes existing upstream runtime fixes, kept outside the metrics PR: hybrid load recovery, async-load progress, independent Mooncake retention, and now #54853 complete connector block tables / #54643 rejected pending-load handling. Support commits: 7c320b4909, 0d8739d029.
Baseline
Exact successful job: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/32809502132/job/97686261004
Prepared with AI assistance; final GPU validation is still in progress.
Note
Medium Risk
Changes benchmark topology scope, Slurm partitions, and vLLM KV/Mooncake failure handling for a validation sweep—not production app code, but misconfiguration could skew GB300 agentic results or job placement.
Overview
Narrows the DeepSeek-V4 GB300 Dynamo-vLLM disaggregated AgentX sweep to the proven c=256 topology (TP4/EP4 prefill, TP16/EP16 decode) and pins a Python-only validation vLLM image (
0d8739d0) that carries connector/block-table and Mooncake-load fixes. The recipe switches prefill MultiConnector load policy torecompute, usesdeep_gemm_mega_moeon both workers, and opts intoAIPERF_REQUIRED_SERVER_METRIC_PREFIX: vllm:prompt_tokens_cached_by_sourceso artifacts must include source-labeled cached prompt tokens (e.g. p2p after NIXL transfer).Custom AgentX jobs on pinned srt-slurm v1.0.36 now apply a scoped patch at launch so worker Prometheus URLs are discovered when that env var is set (not frontend-only). Docs (EN/ZH) describe the opt-in;
test_srt_custom_metrics_patch.pyguards the patch behavior.nvidia-master.yamlalso standardizes several GB300 TensorRT-LLM scenario overrides frombatch_3→batch_1.perf-changelog.yamlrecords the validation image and metric-collection milestones.Reviewed by Cursor Bugbot for commit b41d058. Bugbot is set up for automated code reviews on this repo. Configure here.