[PowerX] verify Slurm completion and normal llm-d exits / 验证 Slurm 完成与 llm-d 正常退出 - #3052
[PowerX] verify Slurm completion and normal llm-d exits / 验证 Slurm 完成与 llm-d 正常退出#3052edwingao28 wants to merge 15 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 关于重新运行失败任务的文档 |
验证 Slurm 完成状态并让 llm-d 工作进程正常退出，先保存诊断证据。
ec4a58f to
85986f4
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
按完整字段解析 Slurm 分配退出码，避免误用 DerivedExitCode。
在现有 CPU 工作流中执行 llm-d 生命周期回归，并跟踪相关脚本变更。
合并 Kimi-K3 B200 前置，并保留所有路径在失败返回前的诊断采集。
将已验证的信号就绪屏障移至共享原生采集前置，避免重放 exec 前的竞态。
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it reworks how success/failure is determined for Slurm jobs and llm-d multi-node workers across every Slurm launcher, and the author's own checklist flags launcher qualification and CODEOWNER sign-off as still pending, a human look would still be worthwhile before merge.
What was reviewed: the new verify_slurm_job_completion sacct-with-scontrol-fallback logic and outcome-file recording in runners/slurm_utils.sh; the llm-d job.slurm/server.sh rework that replaces the scancel-on-marker watcher with a real exit-code marker (finish_llmd_node) and a worker polling loop; and exit-code propagation (SRT_JOB_RC/main_rc) through all 8 Slurm launcher scripts plus the new unit tests covering these paths.
Extended reasoning...
Overview
This PR changes how the InferenceX runner stack decides whether a Slurm job (and, for llm-d, its multi-node workers) truly succeeded, rather than inferring success from "left the queue" or "marker file appeared." Core additions are verify_slurm_job_completion() in runners/slurm_utils.sh (reads sacct, falls back to scontrol show job, writes a receipt file, requires exact COMPLETED/0:0), a rewrite of the llm-d job.slurm/server.sh shutdown protocol (real exit-code marker instead of scancel-on-touch, worker polling instead of wait), and propagation of the verified return code through all 8 Slurm launcher scripts. Tests (test_slurm_utils.py, new test_llmd_lifecycle.py) exercise the sacct/scontrol fallback matrix and the llm-d marker publication/consumption boundary with subprocess-level fixtures.
Security risks
No injection, auth, or data-exposure surface is introduced; this is internal CI/benchmark orchestration logic operating on trusted Slurm/CI infrastructure. The main risk class is correctness/availability (a benchmark job silently reported as failed/succeeded incorrectly), not security.
Level of scrutiny
This warrants more than a quick skim: it touches a shared bash library (slurm_utils.sh) used by every Slurm-based launcher, changes exit-code semantics that downstream CI/reporting depends on, and reworks a fragile shell IPC pattern (file-based markers, EXIT/TERM traps, SIGTERM forwarding) across coordinator/worker roles in server.sh. The PR description itself marks "Actual launcher qualification" and "CODEOWNER sign-off" as pending, which is a direct signal from the author that this hasn't been validated end-to-end on real hardware yet.
Other factors
Test coverage for the new logic is genuinely present and reasonably thorough (parametrized sacct/scontrol race scenarios, marker publish/consume ordering via a printf shim). No outstanding third-party review comments are visible in the timeline, and CODEOWNERS only lists the generic @ InferenceX/core for these paths (no specific blocking owner). Given the breadth (8 launchers + shared lib + multi-node protocol rewrite) and the author's own pending-qualification caveat, a human check-in before merge is the safer default even though the automated hunt turned up nothing actionable.
This review covers commit ec4a58f, which is no longer the latest commit on this pull request; later commits are not covered by it.
中文：保留主分支请求诊断与上传门禁，同时保留 Slurm 完成状态文档及独立变更记录。
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a620774. Configure here.
中文：工作节点检测到引擎退出时再次读取完成标记，避免协调节点正常结束被误判为失败；增加成功、非零状态及缺失标记的回归测试。
中文：同步已合入的原生功耗基础设施，完整保留主分支适配器与采集器，并合并 Slurm 状态文档和工件上传规则。
中文：合入主分支 GLM MI355X 更新，保留原有 Slurm 完成状态变更及独立追加的性能日志。
中文：合入主分支 TileRT 更新，保留原有 Slurm 状态处理和独立性能日志。
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
中文：合入 GLM AgentX 功耗更新，分别保留原生作业与功耗审计状态，收集失败证据后优先返回原生退出码。
中文：合并主分支的 GB300 AgentX 功耗支持，保留原生 Slurm 失败状态和诊断产物。
中文：合并 H200 功耗支持，分别保留原生任务及功耗校验状态，在归档证据后优先返回原生失败。
合入最新主线，保留 B300 镜像与草稿缓存修复及 Slurm 终态检查。
同步主线 H100 强制功耗配置，保留 Slurm 终态检查和追加式变更记录。
1 participant
Description
Check terminal allocation status using the allocation ExitCode and preserve diagnostics before returning failure. Publish llm-d completion atomically so successful workers exit normally.
Testing: Linux CPU CI: 360 passed; 51 local Slurm/lifecycle/Kimi tests and nine saved-receipt replays passed.
Pending: Actual launcher qualification and CODEOWNER sign-off.
中文
使用分配的 ExitCode 检查最终状态，并在返回失败前保留诊断。原子发布 llm-d 完成状态，使成功工作进程正常退出。
测试： Linux CPU CI 360 项通过；本地 51 项 Slurm/生命周期/Kimi 测试及 9 项既有回执回放通过。
待完成： 实际启动器资格验证和 CODEOWNER 签核。
Related Issue
Split from #3026; independent of native power collection.
拆分自 #3026；独立于原生功耗采集。
Type of Change
Checklist
perf-changelog.yamland have not edited historical entriesOWNER/MEMBER/COLLABORATOR) has commented/reuse-sweep-runon 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.Note
Medium Risk
Touches shared Slurm wait/exit paths and llm-d coordinator shutdown across many cluster launchers; misclassification of terminal state could fail green jobs or accept cancelled allocations.
Overview
Slurm-backed benchmark launchers now treat job disappearance from
squeueas insufficient proof of success. Sharedverify_slurm_job_completioninslurm_utils.shchecks terminal allocation state andExitCodeviasacct, falls back toscontrolwhen accounting is stale, writesslurm_job_*_outcome.txt, and is wired intostream_slurm_job_logand multiplelaunch_*.shscripts so logs and artifacts are collected before the launcher exits with the verified rc. Power-audit workflow uploads now include those outcome files.llm-d multinode stops using an external
scancelwatcher on normal completion. The decode coordinator (NODE_RANK == PREFILL_NODES) atomically publishes the final benchmark exit code in.bench_done.<jobid>; workers poll that marker (with a race-safe recheck if vLLM dies) and exit with the same code, whilejob.slurmpropagatessrunfailure without cancelling successful allocations. Failed concurrency points set a retained nonzeroBENCH_RCon the coordinator.Docs,
perf-changelog.yaml, changelog-gate paths, and new tests intest_slurm_utils.pyandtest_llmd_lifecycle.pycover the receipt contract and lifecycle behavior.Reviewed by Cursor Bugbot for commit 8742b8b. Bugbot is set up for automated code reviews on this repo. Configure here.