[PowerX] disable DCGM profiling in power and Tachometer exporters / [PowerX] 关闭功耗与 Tachometer exporter 的 DCGM profiling - #3590
Conversation
|
Thanks for the contribution!
中文感谢你的贡献！
|
edwingao28
changed the title
|
Claude finished @edwingao28's task in 1m 45s —— View job Review of PR #3590
LGTM - no blocking issues found What I checked:
I didn't run the tests; this was a read-only review. GPU qualification is still pending, as the PR description says. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| for runner in runners: | ||
| entry = { | ||
| Fields.IMAGE.value: config[Fields.IMAGE.value], | ||
| Fields.DCGM_COUNTER_PROFILE.value: config.get( |
There was a problem hiding this comment.
📈 Line Count Report
- Total Lines: 1691
- Base Lines: 1685
- Change: +6 lines
edwingao28
changed the title
There was a problem hiding this comment.
Beyond the inline finding, I also checked whether setting dcgm-counter-profile to stock/noprof on a non-ENGINES framework (e.g. tilert) causes an error — it doesn't: plan_commands in synthetic_acceptance.py returns early for those frameworks without calling build_counter_overrides, so the override is silently skipped rather than rejected. Worth confirming this silent no-op is intended before merge.
Extended reasoning...
The PR adds an inherit/stock/noprof DCGM counter-profile selection mechanism at the shared srt-slurm submission boundary (counter_profiles.py, synthetic_acceptance.py, matrix generate/validation, slurm_utils.sh) plus config/doc/changelog updates; it touches CI job entrypoints and recipe override plumbing but no auth/crypto surface. A confirmed inline finding already shows apply_srt_recipe now requires DCGM_COUNTER_PROFILE, which will break existing CI workflows (profile.yml, speedbench-al.yml) that don't export it — this alone is enough to defer for human review. I additionally verified the ruled-out candidate (non-ENGINES frameworks silently skipping profile overrides) against counter_profiles.py and synthetic_acceptance.py and confirmed the described behavior, but judged it a lower-severity design question rather than a hard bug worth a separate inline comment.
| return 1 | ||
| fi | ||
| check_env_vars MODEL_PREFIX IS_AGENTIC EVAL_ONLY SPEC_DECODING | ||
| check_env_vars MODEL_PREFIX IS_AGENTIC EVAL_ONLY SPEC_DECODING DCGM_COUNTER_PROFILE |
There was a problem hiding this comment.
🔴 Adding DCGM_COUNTER_PROFILE to apply_srt_recipe's required env vars breaks CI workflows that were not updated to export it, so their profiling/speedbench jobs now hard-fail instead of running. .github/workflows/profile.yml and .github/workflows/speedbench-al.yml call launch_*.sh scripts (e.g. launch_gb200-nv.sh, launch_b300-dsxe.sh) which source slurm_utils.sh and call apply_srt_recipe, but neither workflow's env block sets DCGM_COUNTER_PROFILE (only benchmark-tmpl.yml and benchmark-multinode-tmpl.yml got the new env line). check_env_vars exits 1 with 'DCGM_COUNTER_PROFILE' missing before any job submits. …
Why this was flagged
…Fix: default DCGM_COUNTER_PROFILE (e.g. ': "${DCGM_COUNTER_PROFILE:=inherit}"') in slurm_utils.sh or add the env var to every workflow that calls apply_srt_recipe, covering profile.yml and speedbench-al.yml.
profile.yml's job env (lines ~33-54) sets EVAL_ONLY, IS_AGENTIC, IS_MULTINODE, RUN_EVAL, SALLOC_TIME_LIMIT defaults but not DCGM_COUNTER_PROFILE; it calls bash ./runners/launch_${RUNNER_NAME%%_*}.sh which sources slurm_utils.sh and calls apply_srt_recipe unconditionally (e.g. launch_gb200-nv.sh:503). apply_srt_recipe now runs check_env_vars MODEL_PREFIX IS_AGENTIC EVAL_ONLY SPEC_DECODING DCGM_COUNTER_PROFILE at slurm_utils.sh:141; check_env_vars (benchmark_lib.sh) calls exit 1 when a var is unset. Before this change apply_srt_recipe did not require this var, so profile.yml worked; after merge every Profile workflow run fails immediately with 'Error: The following required environment variables are not set: - DCGM_COUNTER_PROFILE'. Same gap in speedbench-al.yml (env block ~line 76-91), which calls launch_b300-dsxe.sh via the same apply_srt_recipe path.
Verification: Severity: normal (regression). The diff at inferencex-e2e/runners/slurm_utils.sh:141 adds DCGM_COUNTER_PROFILE to check_env_vars inside apply_srt_recipe. check_env_vars (inferencex-e2e/benchmarks/benchmark_lib.sh:4) exits 1 when any listed var is unset/empty. Two existing workflows call apply_srt_recipe but never set DCGM_COUNTER_PROFILE, and neither is among the PR's 17 changed files (grep…
functionstackx
left a comment
functionstackx
left a comment
There was a problem hiding this comment.
added review in slack
cquil11
changed the title
edwingao28
changed the title
Add the profiling-free DCGM counter file and pass it with -f in all 66 srt-slurm recipes that run a DCGM power exporter, so power sampling no longer stalls on synchronous profiling-watch repair. Independent of PR 3550; the CSV and the H200 Kimi-K3 latency recipe line are identical there, so the two merge cleanly in either order.
14a8d07 to
9e93ecd
Compare
2ff047f to
dc92b73
Compare
| DCGM_FI_DEV_FB_USED, gauge, Framebuffer memory used (in MiB). | ||
| DCGM_FI_DEV_FB_RESERVED, gauge, Framebuffer memory reserved (in MiB). | ||
| DCGM_FI_DEV_NVLINK_BANDWIDTH_TOTAL, gauge, Total number of NVLink bandwidth counters for all lanes. | ||
| DCGM_FI_DEV_VGPU_LICENSE_STATUS, gauge, vGPU License status |
通过 NVIDIA 集群默认配置补齐 Tachometer 的 DCGM 路径；按 exporter 版本保留原字段类型并删除 vGPU license counter。
edwingao28
changed the title
删除 DCGM 配置中的重复说明注释，保留解析后的 YAML 配置和 CSV 指标记录不变。
精简四处中英文说明，保留配置和场景覆盖。
合并同场景条目并去重配置项，保持完整覆盖范围。
移除本 PR 的 changelog 新增条目，保留原有历史。
Point the ten NVIDIA srt-slurm runner profiles' default_gpu_exporter at nvcr.io#nvidia/k8s/dcgm-exporter:4.6.0-4.8.3-distroless, the image the power path already uses, and at the shared /configs/dcgm-counters-noprof.csv. Interval (1000 ms) and port (9401) are unchanged. Drop the 3.3.9-specific CSV and update the docs. AMD profiles and recipe power blocks untouched.
合并 main，将十个 NVIDIA exporter 配置迁入当前集群清单。
移除 B300 recipe 与集群 cpus-per-gpu 冲突的 CPU 参数；GB300 默认 exporter 改用 9402，避开已有的主机服务。
为 Qwen GB200/GB300 聚合式 recipe 补齐功耗拓扑参数，并在请求回放前验证必需输入。
3 participants
Description
Use one noprof CSV/exporter 4.6.0-4.8.3 for NVIDIA telemetry. Remove profiling/license fields; validate GB power topology before replay.
Testing: 42 tests/lint; B200/H200 evidence retained; GB200, GB300, B300 compatibility CI passed. Diagnostic retries add deadlines/watchdog to 979832e.
Limits: B300 CPU-exporter TaskProlog127 persists; GB300 stall unreproduced. Full qualification and three existing H200 recipe errors remain.
中文
统一 NVIDIA 采集的 noprof CSV 与 4.6.0-4.8.3 exporter；移除 profiling/license 字段，回放前校验 GB 功耗拓扑。
**测试：**42 项测试及 lint 通过；保留 B200/H200 证据，GB200、GB300、B300 兼容性 CI 通过。诊断重试在 979832e 上增加作业时限/watchdog。
**限制：**B300 CPU exporter 的 TaskProlog127 仍存在；GB300 卡住未复现。完整验收及已有 3 个 H200 配方错误仍待解决。
AI：GPT-6 负责实现及独立审查，Claude 负责此前简化；具体版本无法确认。关联 #2681。
AI model disclosure
Related Issue
Related to #2681.
Type of Change
Checklist
inferencex-e2e/perf-changelog.yamland have not edited historical entriesOWNER/MEMBER/COLLABORATOR) has commented/use <run_id>(or the legacy/reuse-sweep-run) on this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the sweep label will no longer automatically kick off new sweeps. Remove and re-add the label to force one.