Fix GB300 DeepSeek-V4 AgentX worker metrics / 修复 GB300 DeepSeek-V4 AgentX worker 指标采集#2291
Fix GB300 DeepSeek-V4 AgentX worker metrics / 修复 GB300 DeepSeek-V4 AgentX worker 指标采集#2291cquil11 wants to merge 2 commits into
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 关于重新运行失败任务的文档 |
7cd3b30 to
b4dbfa7
Compare
|
Claude finished @cquil11's task in 6m 5s —— View job PR Review
Summary1 warning, no blocking issues. The one finding (posted inline on Everything else checks out:
Note: I could not re-run the pytest/validator commands in this sandbox (execution not permitted) or fetch the fork commit contents directly — I relied on the author-reported validation, the fork SHA's existing use on main, and CI, which runs the matrix/changelog checks anyway. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| benchmark: | ||
| type: custom | ||
| aiperf_server_metrics: true |
There was a problem hiding this comment.
🟡 WARNING: The inter-concurrency drain gate can't see SGLang worker activity — it only sums vLLM metric names.
Why it matters: This flag now populates AIPERF_SERVER_METRICS_URLS with SGLang worker endpoints, and wait_for_agentic_servers_idle in benchmarks/multi_node/agentic_srt.sh:78-79 sums only vllm:num_requests_running / vllm:num_requests_waiting. SGLang exports sglang:num_running_reqs / sglang:num_queue_reqs (verified against the SGLang metrics collector — there are no vllm:* names), so worker_active is always 0 and the worker half of the drain check is a silent no-op. This recipe is the one affected: with conc-list: [1, 4, 8, 16] the drain gate runs between points, and it will only wait on dynamo_frontend_active_requests — i.e., the pre-PR behavior, minus the worker-side guarantee this PR sets out to add. (Second-order effect: if any injected worker URL is unreachable from the node running agentic_srt.sh, every poll raises, idle_polls never accumulates, and the run hard-fails after AIPERF_DRAIN_TIMEOUT_SECONDS=1800s even when servers are idle.)
Fix: In agentic_srt.sh, also sum the SGLang gauge names:
worker_active += metric_sum(worker_metrics, "vllm:num_requests_running")
worker_active += metric_sum(worker_metrics, "vllm:num_requests_waiting")
worker_active += metric_sum(worker_metrics, "sglang:num_running_reqs")
worker_active += metric_sum(worker_metrics, "sglang:num_queue_reqs")metric_sum returns 0 for absent metrics, so this stays backend-agnostic for the existing vLLM GB200 agentic recipes.
There was a problem hiding this comment.
LGTM. The AIPerf metrics wiring (aiperf_server_metrics: true across all six recipes), the new cluster:gb300-nv hardware entry, and the perf-changelog addition are mechanical, consistent with existing patterns in this repo, and don't touch production-critical or security-sensitive code. The flagged nit (agentic_srt.sh / generate_aiperf_plots.py hardcoding vllm:-prefixed metric names, which won't match SGLang's sglang: prefix) has bounded, diagnostic-only impact — the frontend-side drain check still functions and the raw sglang:* series are still captured in server_metrics_export.json — so it doesn't need to block this PR.
Extended reasoning...
Overview
This PR wires up AIPerf Prometheus server-metrics collection (aiperf_server_metrics: true) for six GB300 DeepSeek-V4 AgentX SGLang recipes, layers a pinned cherry-pick commit from a contributor fork (cquil11/srt-slurm-nv) onto the NVIDIA/srt-slurm v1.0.10 checkout in runners/launch_gb300-nv.sh so the host-side orchestrator injects per-worker metrics URLs, adds a cluster:gb300-nv hardware entry to configs/runners.yaml, and appends a perf-changelog entry. All changes are confined to benchmark recipe YAML, a CI launcher script, and static config/changelog data.
Security risks
None of production consequence. The only notable supply-chain-adjacent element is the git fetch .../cquil11/srt-slurm-nv.git <full-commit-sha> && git cherry-pick --no-commit FETCH_HEAD addition in launch_gb300-nv.sh. It's pinned to a full commit SHA (content-addressed, can't be silently swapped) and mirrors an existing pattern already used a few branches down in the same script (the plain agentic path also clones cquil11/srt-slurm-nv and checks out a pinned commit), so this isn't a new risk pattern for this codebase.
Level of scrutiny
This is CI/benchmark-infra tooling, not a production code path — the blast radius of a mistake is a failed or misleading benchmark run, not a live-traffic incident. Low-to-moderate scrutiny is appropriate. The change is largely repetitive (same one-line flag added identically across six recipe files) plus a well-scoped, narrated launcher script edit.
Other factors
The one substantive finding (vllm:-only metric names not matching SGLang's sglang: prefix in two downstream consumers) is real but nit-level per its own bounded-impact analysis: the frontend-side drain-wait check still gates concurrency transitions, and the plots consumer is best-effort/diagnostic-only with the raw metrics still captured in the exported JSON. It's already surfaced as an inline comment, so it doesn't need to gate this approval. Validation described in the PR (matrix_logic tests, srt-slurm test suites, changelog validator, bash -n, local matrix generation) is reasonably thorough for this kind of change.
|
|
||
| benchmark: | ||
| type: custom | ||
| aiperf_server_metrics: true |
There was a problem hiding this comment.
🟡 Setting aiperf_server_metrics: true on these six SGLang recipes makes AIPERF_SERVER_METRICS_URLS non-empty for the SGLang path for the first time, but two downstream consumers still hardcode vLLM-only metric names instead of SGLang's sglang:-prefixed ones: wait_for_agentic_servers_idle() in benchmarks/multi_node/agentic_srt.sh (worker-level drain check silently reports 0 activity), and panel_kv_cache_usage()/panel_queue_depth() in utils/generate_aiperf_plots.py (KV-cache/queue-depth panels render empty). Fix by matching sglang:-prefixed names or selecting the metric family by backend in both files; this same recipe line is added identically across all six new recipes so the fix applies uniformly.
Extended reasoning...
Turning on benchmark.aiperf_server_metrics: true (added identically to all six GB300 DeepSeek-V4 SGLang AgentX recipes in this PR) causes the cherry-picked srt-slurm patch to populate AIPERF_SERVER_METRICS_URLS with one real Prometheus endpoint per logical SGLang worker leader. Per the PR description, that variable was never set on this SGLang path before (PR #2157 left it empty), so this is the first time these worker URLs are actually fetched rather than being an inert, empty list.
Two existing consumers of that variable hardcode vLLM-only Prometheus metric names and never branch on backend/framework to select the SGLang equivalents:
-
wait_for_agentic_servers_idle()inbenchmarks/multi_node/agentic_srt.sh(~lines 74-79) computes worker activity viametric_sum(worker_metrics, "vllm:num_requests_running")and"vllm:num_requests_waiting".metric_sum()matches on an exact metric-name prefix, so it can never match SGLang'ssglang:num_requests_running/sglang:num_requests_waitinglines.worker_activeis therefore always computed as0.0, regardless of how many requests SGLang workers are actually still running or queuing. This function is invoked between every pair of concurrency points in the sweep loop to make sure the previous point has fully drained before the next one starts; withworker_activepinned to zero, the worker-level half of that guard is dead code for SGLang and the drain-wait falls back to relying solely ondynamo_frontend_active_requests. -
panel_kv_cache_usage()andpanel_queue_depth()inutils/generate_aiperf_plots.py(~lines 208-212, 246-249) look upvllm:kv_cache_usage_perc,vllm:cpu_kv_cache_usage_perc,vllm:num_requests_running, andvllm:num_requests_waitingin the scraped server metrics. None of these keys exist in an SGLang/metricspayload, so both panels in the generatedmetrics_plots.pngrender empty for all six of this PR's new recipes — silently defeating part of the stated point of the PR (capturing per-worker metrics for visualization).
SGLang's distinct metric family is already established elsewhere in the repo: benchmarks/single_node/agentic/dsv4_fp4_b300_sglang.sh:207 greps ^(sglang:(...|num_requests_running|num_requests_waiting)), and utils/agentic/aggregation/backends/sglang.py keys every SGLang server metric off the sglang: prefix. Neither agentic_srt.sh nor generate_aiperf_plots.py has any equivalent branching logic.
Step-by-step proof (drain-wait): (1) Recipe sets aiperf_server_metrics: true. (2) srt-slurm's cherry-picked patch resolves the SGLang worker leaders' hostnames/ports and exports AIPERF_SERVER_METRICS_URLS="http://worker1:PORT/metrics,http://worker2:PORT/metrics,...". (3) agentic_srt.sh splits this into worker_urls and, between concurrency points, curls each URL's /metrics, which (from an SGLang engine) contains lines like sglang:num_requests_running{...} 3 and sglang:num_requests_waiting{...} 1. (4) metric_sum(worker_metrics, "vllm:num_requests_running") scans those same lines for an exact match on vllm:num_requests_running and finds none, returning 0; same for vllm:num_requests_waiting. (5) worker_active = 0 + 0 = 0 even though 3 requests are running and 1 is queued. (6) The idle-wait loop only needs dynamo_frontend_active_requests == 0 for 3 consecutive polls to declare "drained," so it can do so while those SGLang requests are still in flight from the previous concurrency point.
Impact is real but bounded, which is why this should land as a nit rather than a blocker: the drain-wait's frontend-side check (dynamo_frontend_active_requests) still functions and is a reasonably strong proxy since a request stays "active" at the frontend until its streamed response completes; additionally, agentic_srt.sh's per-concurrency-point cache-bust marker gives each point a disjoint KV keyspace independently of the drain-wait, further limiting cross-contamination risk. For the plots consumer, generate_aiperf_plots.py is invoked best-effort (wrapped in || true from benchmark_lib.sh) purely for diagnostic visualization — it doesn't gate run success or affect the benchmark result JSON, and the raw sglang:* series are still captured intact in server_metrics_export.json; only these two specific plot panels come out empty.
Fix direction: have agentic_srt.sh and generate_aiperf_plots.py also match sglang:-prefixed metric names, or branch on backend/framework to select the right prefix family, since both files are now shared by vLLM and SGLang multi-node recipes that set aiperf_server_metrics.
Derive logical worker Prometheus endpoints in the InferenceX custom benchmark from the Slurm allocation and recipe worker widths while keeping NVIDIA/srt-slurm v1.0.10 unmodified. Teach the drain gate SGLang queue metrics, add topology tests, and retain the GB300 hardware metadata needed by matrix generation. 中文：在 InferenceX 自定义基准测试中，根据 Slurm 资源分配和配方中的 worker 节点宽度推导各逻辑 worker 的 Prometheus 端点，保持 NVIDIA/srt-slurm v1.0.10 完全不变。为排空检查补充 SGLang 队列指标，增加拓扑测试，并保留矩阵生成所需的 GB300 硬件元数据。
b4dbfa7 to
a42cc4b
Compare
|
Updated the implementation to use untouched NVIDIA/srt-slurm v1.0.10 with no custom fork or patch. Endpoint discovery now runs entirely in the InferenceX custom benchmark, and the SGLang drain metrics noted in review are included. / 已将实现改为原样使用 NVIDIA/srt-slurm v1.0.10，不再使用自定义 fork 或补丁。端点发现现完全由 InferenceX 自定义基准测试完成，审阅中指出的 SGLang 排空指标也已补充。 |
Pin the GB300 DeepSeek-V4 AgentX path to NVIDIA/srt-slurm PR #276 through the upstream pull ref, verify its immutable head, and log whether srt-slurm supplied AIPerf worker metrics endpoints. 中文：将 GB300 DeepSeek-V4 AgentX 路径通过上游 Pull Request 引用固定到 NVIDIA/srt-slurm PR #276，校验不可变的 Head Commit，并在日志中明确标识 AIPerf Worker 指标端点是否由 srt-slurm 提供。
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29787068414 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=29791311093 |
1 participant
Summary
3fe5508, fetched through the upstream pull ref with no downstream patch or custom-fork clonescontrolin the model containercluster:gb300-nvhardware metadata required by current AgentX DRAM-offload matrix generationRoot cause and consequences
AIPerf supports multiple values after one
--server-metricsflag, and InferenceX already forwardsAIPERF_SERVER_METRICS_URLS. Upstream srt-slurm also passesbenchmark.envtobenchmark.type: customcommands unchanged. Released upstream srt-slurm versions only auto-discover worker endpoints for built-in AIPerf runners, not custom commands, and PR #2157 did not otherwise populate the variable. This PR keeps an InferenceX fallback for released tags and directly validates NVIDIA/srt-slurm #276, which exports the authoritative ordered endpoint list to custom benchmarks.Its successful sweep therefore scraped only
http://localhost:8000/metrics. The benchmark results contained Dynamo router/frontend metrics but no directsglang:*worker series, which removed observability for SGLang token/cache/HiCache and queue metrics. The InferenceX aggregation layer already normalizes captured SGLang metrics; the data was simply never collected. The drain gate also previously checked only vLLM queue names, so its worker-side check was ineffective for SGLang.Validation
python -m pytest utils/agentic/test_srt_metrics_endpoints.py utils/matrix_logic/ -q— 232 passedpython -m pytest utils/agentic/test_srt_metrics_endpoints.py utils/agentic/aggregation/test_process_agentic_result.py -q— 39 passedv1.0.10with all six recipes overlaid:pytest tests/test_configs.py tests/test_benchmarks.py -q— 168 passedpull/276/head, with all six recipes overlaid: focused custom-benchmark, node-placement, and recipe-loading tests — 16 passedpython utils/validate_perf_changelog.py --base-ref origin/main --head-ref HEADbash -n benchmarks/multi_node/agentic_srt.sh runners/launch_gb300-nv.shfull-sweep-enabled中文说明
概述
3fe5508；通过上游 Pull Request 引用拉取，不应用下游补丁，也不克隆自定义 Forkscontrolcluster:gb300-nv硬件元数据根因与影响
AIPerf 支持在单个
--server-metrics参数后传入多个端点，InferenceX 也已支持转发AIPERF_SERVER_METRICS_URLS。上游 srt-slurm 同样会把benchmark.env原样传给benchmark.type: custom命令。但已发布的上游 srt-slurm 版本只会为内置 AIPerf Runner 自动发现 Worker 端点，不会为自定义命令执行该操作，而 PR #2157 也未通过其他方式设置此变量。本 PR 为已发布版本保留 InferenceX 侧的回退逻辑，并直接验证 NVIDIA/srt-slurm #276；该上游 PR 会向自定义基准测试导出权威且有序的端点列表。因此，原扫描虽然成功，却只抓取了
http://localhost:8000/metrics。结果中包含 Dynamo 路由器/frontend 指标，但没有任何直接的sglang:*worker 时序数据，导致 SGLang token、缓存、HiCache 和队列指标不可观测。InferenceX 聚合层已经支持归一化采集到的 SGLang 指标，问题仅在于这些数据此前从未被抓取。另外，原排空检查只识别 vLLM 的队列指标名，因此其 worker 侧检查对 SGLang 实际无效。验证
python -m pytest utils/agentic/test_srt_metrics_endpoints.py utils/matrix_logic/ -q— 232 项通过python -m pytest utils/agentic/test_srt_metrics_endpoints.py utils/agentic/aggregation/test_process_agentic_result.py -q— 39 项通过v1.0.10工作树中覆盖全部 6 个配方后运行pytest tests/test_configs.py tests/test_benchmarks.py -q— 168 项通过pull/276/head拉取 NVIDIA/srt-slurm Update NVIDIA DeepSeek sglang Docker image from v0.5.5 to v0.5.6 #276 的精确 Head Commit，并覆盖全部 6 个配方；自定义基准测试、节点放置和配方加载相关测试 — 16 项通过python utils/validate_perf_changelog.py --base-ref origin/main --head-ref HEADbash -n benchmarks/multi_node/agentic_srt.sh runners/launch_gb300-nv.shfull-sweep-enabled执行完整扫描