Skip to content

[Bugfix][ROCm] Make Qwen OOM recovery pressure deterministic - #9425

Merged
andyluo7 merged 2 commits into
vllm-project:mainfrom
hacZ17:ci/rocm-qwen-oom-recovery-eager
Oct 27, 2026
Merged

andyluo7 merged 2 commits into
vllm-project:mainfrom
hacZ17:ci/rocm-qwen-oom-recovery-eager

Conversation

@hacZ17

@hacZ17 hacZ17 commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Run Qwen3-Omni GPU OOM recovery on the oom server set so HIP stages 0 and 1 use enforce_eager=True.
  • Cap residual free memory at 32 MiB for HIP recovery. A proportional 0.95 target alone left enough memory for both MI300 recovery requests to complete successfully.
  • Keep CUDA recovery ratio-only by passing no effective free-memory ceiling, and leave models without eager_oom_stage_ids (including Wan) unchanged.
  • Add regression coverage for the HIP-only ceiling, its injection plumbing, both Qwen deploy scenarios, unchanged CUDA deployment, and unchanged Wan behavior.

Confirmed failure and root cause

AMD weekly build #14733 ran the previous exact PR head 8614ce54fe4f04aa062b552a77802c567eb6477e with a clean checkout.

  • Result: 2 failed, 25 passed, 10 skipped; no retries.
  • Both test_reliability_fault_gpu_oom_recovers_after_fault_removed variants returned HTTP 200 while pressure was active.
  • Eager execution was enabled, but the ratio-only sidecar left approximately 1.05 GiB free on GPU 0 and 4.03 GiB free on GPU 1.
  • The existing HIP hard-OOM cases already use and pass with a 32 MiB ceiling, so recovery now uses the same HIP ceiling while retaining CUDA's existing ratio-only behavior.

Why this is not a duplicate

No open PR changes GpuOomRecovery.server_set or adds a HIP-only residual-memory ceiling to Qwen recovery. This PR does not change the HunyuanImage process-kill matrix.

Contributor intent

  1. What outcome do you want to achieve? Make the Qwen3-Omni OOM recovery cases reliably observe a request failure while GPU memory pressure is active, then verify the server after pressure is removed.
  2. What is the highest-leverage variable for that outcome? The recovery server must avoid captured graph pools, and HIP pressure must bound residual free memory rather than relying only on a percentage of a high-memory MI300 device.
  3. How will this work move that variable? Select the eager oom server set and apply the proven 32 MiB HIP free-memory ceiling to recovery injection. Regression tests check both Qwen scenarios, the ceiling plumbing, unchanged CUDA deployment behavior, and unchanged Wan behavior.
  4. Whose customer or user needs does this address, and what are those needs? AMD weekly CI owners need Qwen reliability coverage that deterministically exercises OOM recovery instead of intermittently completing the fault-phase request successfully.

Design and documentation

This changes only the reliability-test harness. It does not change public runtime behavior, user configuration, or an active design contract, so no user or design documentation update is required.

AI assistance disclosure

Cursor using Grok 4.7 drafted the original server-set change and regression test. Codex analyzed the MI300 failure, implemented the HIP-only residual-memory ceiling, updated regression coverage, rebased the branch, and updated this description. The changes were reviewed and the reported commands/results were verified before submission.

Test plan and current results

  • Changed-file pre-commit hooks passed, including Ruff, Ruff format, mypy 3.10, test marks, SPDX, forbidden imports, and torch.cuda checks.

  • Static regression subset runnable without Torch: 5 passed, 1 deselected.

  • Python syntax compilation passed for both changed test files.

  • MI300 weekly qualification run 1: AMD #14742, Qwen job 01a17a37-58ae-4281-9730-b08fff3cb4e9, 27 passed, 10 skipped, 14 warnings in 1:10:05.

  • MI300 weekly qualification run 2: AMD #14747, Qwen job 01a17a8e-e037-42c8-ac10-92268f4360a1, 27 passed, 10 skipped, 14 warnings in 1:08:54.

  • Both Qwen jobs ran the exact final head with clean weekly checkouts, exited 0, had no retries or soft failures, and passed all eight OOM cases while pressure reached the 32 MiB HIP ceiling on both GPUs.

  • The known unrelated nonblocking HunyuanImage result was excluded from the named Qwen qualification gate.

  • READY AMD #14749: 33/33 records terminal and passing.

  • READY CUDA #18204: 18/18 records terminal and passing.

  • Intel #10921: 2/2 records terminal and passing.

  • NPU #9947: 21/21 executed records terminal and passing.

  • All four platform builds ran the exact final head with no nonzero exits, retries, soft failures, or unfinished records.

Current base: c84638f3208f0fffd872200adfc5860103a6244b

Current head: d8ec07595844ab8aee6080bcdf49c1bc2ce7898c

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-26T16:45:23.048639Z
We couldn’t complete this review. Comment @codex review to try again.
ef22e8d PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR matches CODEOWNERS paths: /tests/.

Code owners: @NickCao @yenuo26

Routing: @NickCao via CODEOWNERS; @yenuo26 via CODEOWNERS

@hacZ17, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

@vllm-omni-review-bot

vllm-omni-review-bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit d8ec07595844 produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

@hacZ17
hacZ17 force-pushed the ci/rocm-qwen-oom-recovery-eager branch from ef22e8d to 8614ce5 Compare October 26, 2026 16:59
@andyluo7 andyluo7 added weekly-test label to trigger buildkite weekly test CI amd-test Used to trigger AMD CI separately. labels Oct 26, 2026
@vllm-omni-review-bot

vllm-omni-review-bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Omni ReviewBot: superseded

The CI failure noted on 8614ce54fe4f refers to an earlier head; the pull request now points at d8ec07595844.

haic0 and others added 2 commits October 26, 2026 13:29
The recovery scenario used the default graph server, so HIP requests could
succeed from captured pools while the sidecar held device memory. Select the
OOM server set, which forces eager execution on the declared HIP stages and
leaves CUDA and models without those stages unchanged.

Signed-off-by: haic0 <149741444+haic0@users.noreply.github.com>
Co-authored-by: Grok 4.7 <noreply@x.ai>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: lm10-arg <347490427+lm10-arg@users.noreply.github.com>
@andyluo7
andyluo7 force-pushed the ci/rocm-qwen-oom-recovery-eager branch from 8614ce5 to d8ec075 Compare October 26, 2026 20:33
@andyluo7 andyluo7 changed the title [Bugfix][ROCm] Run Qwen OOM recovery on the eager server [Bugfix][ROCm] Make Qwen OOM recovery pressure deterministic Oct 26, 2026
@andyluo7 andyluo7 added amd-test Used to trigger AMD CI separately. ready label to trigger buildkite CI and removed amd-test Used to trigger AMD CI separately. weekly-test label to trigger buildkite weekly test CI ready label to trigger buildkite CI labels Oct 26, 2026

@andyluo7 andyluo7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at exact head d8ec075 after two clean AMD weekly Qwen qualification runs and clean READY AMD, CUDA, Intel, and NPU builds with no retries or soft failures.

@andyluo7
andyluo7 merged commit 1ddd2c7 into vllm-project:main Oct 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready label to trigger buildkite CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants