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 |
1 participant
Summary
source="p2p"No new launcher, allocation, or recipe infrastructure is introduced.
Validation
utils/matrix_logic/test_generate_sweep_configs.py: 127 passeddsv4_p1x4ep4dpa_d1x16ep16dpa_conc256), five GB300 nodessha256:2c8fefc55defd3f157b78a33b0637bdcb5f26f2a7253f960aedbecba65da8510Baseline
Exact successful job: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/32809502132/job/97686261004
Note
Medium Risk
Benchmark and cluster config only, but changes KV-load failure handling, container image, and Slurm partition routing for multiple GB300 sweeps.
Overview
Narrows the DeepSeek-V4 GB300 Dynamo-vLLM disaggregated AgentX matrix to a single proven c=256 point (DEP4 prefill / DEP16 decode) and pins a Python-only validation image (
ttl.sh/...tier-pd-recovery...) instead of the nightly vLLM image.The disagg recipe switches prefill/decode MoE to
deep_gemm_mega_moe, sets prefill MultiConnectorkv_load_failure_policytorecompute(decode NIXL stays fail-closed), and opts intoAIPERF_REQUIRED_SERVER_METRIC_PREFIX=vllm:prompt_tokens_cached_by_sourceso runs must export source-labeled cached prompt-token metrics.GB300 launcher applies a scoped srt-slurm v1.0.36 patch so custom AgentX commands discover prefill/decode worker Prometheus URLs when that env var is set; behavior is covered by
test_srt_custom_metrics_patch.py. Docs (EN/ZH) document the opt-in.nvidia-master.yamlalso moves remainingSLURM_PARTITION=batch_3overrides on this branch tobatch_1for DeepSeek-V4 / GLM TensorRT-LLM agentic configs.perf-changelog.yamlrecords the validation image, metrics collection, Mooncake/hybrid recovery, and partition standardization.Reviewed by Cursor Bugbot for commit 7187c07. Bugbot is set up for automated code reviews on this repo. Configure here.