Repository navigation
[Bugfix][CI/Build] Place stage-CLI test replicas on separate GPUs - #9041
alex-jw-brooks merged 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
OmniServerStageCli launched one headless process per replica with
--replica-id, which serve ignores. Each process ran with
--omni-dp-size-local 1, so split_devices_for_replicas handed every
replica the whole devices pool ("1,2,3") and all of them landed on
the same card. The BAGEL multi-replica weekly test OOMed as a result.
Launch one headless process per stage and pass
--omni-dp-size-local <num_replicas>, so the supported headless path
splits the pool one replica per card. Stage processes and logs are
now keyed by stage_id, since there is one process per stage.
Fixes vllm-project#6843
Signed-off-by: cipheraxat <cipheraxat@users.noreply.github.com>
5dd5f2b to
2b09e15
Compare
|
This PR matches CODEOWNERS paths: /tests/. Code owners: @NickCao @yenuo26 Routing: @NickCao via CODEOWNERS; @yenuo26 via CODEOWNERS @cipheraxat, 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. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
…lm-project#9041) Signed-off-by: cipheraxat <cipheraxat@users.noreply.github.com> Co-authored-by: cipheraxat <cipheraxat@users.noreply.github.com> (cherry picked from commit c704a28) Signed-off-by: tzhouam <tzhouam@connect.ust.hk>
Contributor intent (required human answers)
1. What outcome do you want to achieve?
The weekly BAGEL multi-replica e2e test (
tests/e2e/online_serving/test_bagel_multi_replicas.py) passes again instead of OOMing.2. What is the highest-leverage variable for that outcome?
Which physical GPU each headless DiT replica lands on.
3. How will this work move that variable?
The stage-CLI test harness launches one headless process per stage with
--omni-dp-size-local N, sodevices: "1,2,3"is split one replica per card.4. Whose customer or user needs does this address, and what are those needs?
Maintainers / CI: a trustworthy weekly multi-replica diffusion signal without false OOM failures.
Purpose
Fixes #6843.
Before:
OmniServerStageCli._start_serverstarted one headless process per replica, eachwith
--replica-id i.serveignores--replica-id(deprecated in #3569), so each processran with
omni_dp_size_local=1.get_headless_replica_devices→split_devices_for_replicas("1,2,3", 1, 1, 1)returned["1,2,3"], so all three DiT replicasused local device 0 (physical GPU 1). With
gpu_memory_utilization: 0.7the third replicaOOMs. The OOM line in the issue shows three 22–28 GiB processes on one GPU.
After: the harness starts one headless process per stage and adds
--omni-dp-size-local <num_replicas>when the stage has more than one replica. The supportedheadless path then splits the pool to
["1", "2", "3"]and starts one replica per card.--replica-idis no longer passed.There is now one harness process per stage, so the per-replica bookkeeping is gone too:
stage_procsand the stage log maps are keyed bystage_id, and stage logs are namedomni_stage_<id>_<port>.log. The only other reader,test_gepard_tts.py, iterates_stage_log_paths.values()and is unaffected.Scope:
bagel_multi_replicas_4gpuis the only stage-CLI overlay withnum_replicas > 1.test_qwen3_omni_multi_replicas.pyuses the single-runtime server and is unaffected.Docs: no change.
docs/design/module/stage_runtime.mdownsstage_engine_startup.py(headless device splitting), which this PR does not change. It onlyaligns the test harness with it. No production code changes.
Test Plan
vLLM Version: 0.31.0 (repo pin). See the note on the local environment below.
vLLM-Omni Commit: base
b164fea79Regression tests (CPU,
core_model+cpu):test_stage_cli_builds_headless_replica_cmd: stage 1 command carries--omni-dp-size-local 3and no--replica-id.test_stage_cli_omits_dp_size_local_for_single_replica_stagetest_stage_cli_launches_one_process_per_stage:_start_serverlaunches stage 1 once,not once per replica.
Test Result
--replica-idpresent).After the fix: 22 passed (
test_runtime_stage_cli.py+test_runtime_startup_timeout.py).split_devices_for_replicas(real source):("1,2,3", 1, 1, 1)→['1,2,3']before;("1,2,3", 3, 1, 1)→['1', '2', '3']after.pre-commit run --fileson the 3 changed files: all hooks pass (ruff, mypy-3.10, testmarks, SPDX, forbidden imports, typos).
Not run:
vllm/vllm_omniimportswere stubbed for these pure-Python harness tests. The tests have not been run in the real
Linux CPU-test environment.
test_bagel_multi_replicas.pywas not run (no hardware). GPU placement isverified at source level only. A maintainer rerun of that weekly test would confirm the fix.
AI assistance: Used Claude Code (Claude Opus 5.5) to trace the root cause, write the harness fix and regression tests, and draft this description and the issue comment.
Validation: see Test Result above. CPU harness tests and pre-commit ran locally on macOS with
vllmimports stubbed; the 4×H100 e2e was not run.