Repository navigation
[Bugfix] Recognize mixed-attention warmup requests in MRv2 - #9264
Conversation
Signed-off-by: Weiming Liao <liaowm5@gmail.com>
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. |
|
This PR appears to belong to: docs/design/module/ar_runtime.md, docs/design/module/model_integration.md. Module owners: @tzhouam @fake0fan @Gaohan123 Routing: @tzhouam via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Gaohan123 via module of the changed files @FayeSpica, 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. |
amy-why-3459
left a comment
There was a problem hiding this comment.
Centralizing the warmup check is a good cleanup, and the parametrized tests cover the new IDs well. Two points on the helper's matching.
|
|
||
| def is_warmup_request(req_id: str) -> bool: | ||
| """Recognize vLLM kernel and mixed-attention warmup requests.""" | ||
| return req_id.startswith(("_warmup_", "_v2_mixed_warmup_")) |
There was a problem hiding this comment.
run_mixed_prefill_decode_warmup takes req_id_prefix as a parameter, and on vLLM main other callers already pass different prefixes: flashinfer_sparse_mla_warmup.py uses "_sparse_mla_v2_warmup" and step5_dsa_warmup.py uses "_step5_dsa_warmup". Those requests would miss this tuple and go through the normal TTS / model-state path again, so the same startup failure would come back on the next vLLM bump for any model whose worker enables those warmups. Could the check key off the shared shape instead, e.g. req_id.startswith("_") and "_warmup" in req_id, or at least add the two known prefixes plus a test?
Also, every call site you replaced used to coerce with str(...) first, but the helper now calls .startswith on the raw value. Doing str(req_id).startswith(...) here would keep the old behavior for non-str IDs.
There was a problem hiding this comment.
Since NewRequestData.req_id is typed as str and upstream warmup IDs are constructed as strings, I think we can keep the direct check here.
| return | ||
| for request_data in getattr(scheduler_output, "scheduled_new_reqs", []): | ||
| if str(getattr(request_data, "req_id", "")).startswith("_warmup_"): | ||
| if is_warmup_request(request_data.req_id): |
There was a problem hiding this comment.
This one also drops the getattr(request_data, "req_id", "") fallback that was here before. If anything still puts objects without req_id into scheduled_new_reqs (test doubles, or the native data-plane shims), this now raises AttributeError instead of falling through to register_request. Was that intended?
There was a problem hiding this comment.
This method is specific to MRv2. Entries in scheduled_new_reqs are required to have req_id
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
linyueqian
left a comment
There was a problem hiding this comment.
Reviewed a5bc5ab. vLLM 0.32 builds its mixed prefill/decode warmup requests as _v2_mixed_warmup_decode_ and _v2_mixed_warmup_prefill_ (vllm/v1/worker/gpu/warmup.py, run_mixed_prefill_decode_warmup), so matching both prefixes in one helper covers every warmup site that used the old _warmup_ check, and no raw startswith("_warmup_") is left under vllm_omni/. The tests cover both prefixes and a normal request id.
Static review only; I did not run the tests locally. Main CUDA passes at a5bc5ab (build 17883).
#8483 #9272 #8802 #9248 #9282 #9264 (#9295) Signed-off-by: psv666 <2693925048@qq.com> Signed-off-by: haic0 <149741444+haic0@users.noreply.github.com> Signed-off-by: andyluo7 <andy.luo@amd.com> Signed-off-by: Joshna Medisetty <joshna.medisetty@intel.com> Signed-off-by: Chendi Xue <chendi.xue@intel.com> Signed-off-by: Wenbo Ji <36562829+fusheng-ji@users.noreply.github.com> Signed-off-by: NumberWan <wantszkin2003@gmail.com> Signed-off-by: Sylvan Liu <Sylvan.Liu@amd.com> Signed-off-by: wangqiankun13 <wangqiankun13@huawei.com> Signed-off-by: amy-why-3459 <wuhaiyan17@huawei.com> Signed-off-by: Weiming Liao <liaowm5@gmail.com> Co-authored-by: psv666 <150513104+psv666@users.noreply.github.com> Co-authored-by: haic0 <149741444+haic0@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: andyluo7 <andy.luo@amd.com> Co-authored-by: Joshna-Medisetty <joshna.medisetty@intel.com> Co-authored-by: Chendi Xue <chendi.xue@intel.com> Co-authored-by: Wenbo Ji 嵇文博 <36562829+fusheng-ji@users.noreply.github.com> Co-authored-by: dengyunyang <584797741@qq.com> Co-authored-by: NumberWan <wantszkin2003@gmail.com> Co-authored-by: Sylvan Liu <syliu@amd.com> Co-authored-by: wangqiankun13 <wangqiankun13@huawei.com> Co-authored-by: amy-why-3459 <wuhaiyan17@huawei.com> Co-authored-by: Weiming Liao <liaowm5@gmail.com>
Contributor intent (required human answers)
1. What outcome do you want to achieve?
Fix the Qwen3-TTS startup failure during FlashInfer warmup in vLLM 0.32.
2. What is the highest-leverage variable for that outcome?
Correct identification of synthetic MRv2 warmup requests.
3. How will this work move that variable?
Identify the new warmup requests added in vLLM 0.32 and skip text validation for them, preventing the startup error.
4. Whose customer or user needs does this address, and what are those needs?
Qwen3-TTS users need the server to start successfully. Release CI needs the TTS performance tests to run.
Purpose
Relates to #9233.
vLLM 0.32 adds warmup requests with a new ID prefix. Omni does not recognize them as warmup requests and tries to process them as normal TTS input. Because they contain no text, Qwen3-TTS fails to start.
This change adds a shared function to recognize both warmup ID prefixes and uses it across MRv2 runners and model states. Warmup requests then follow the existing warmup handling. No user configuration or API changes are needed, so user guides and examples remain unchanged.
Test Plan
Environment: A100 80GB, vLLM 0.32.0, PyTorch 2.13.0+cu130, Omni
798fef99a+ this patch.MRv2 and model-state regression tests:
Qwen3-TTS two-stage startup with FlashInfer:
vllm serve Qwen/Qwen3-TTS-12Hz-0.6B-Base --omni --trust-remote-code \ --host 127.0.0.1 --port 19233 \ --stage-overrides '{"0":{"attention_config":{"backend":"FLASHINFER"}},"1":{"devices":"1"}}' curl --fail http://127.0.0.1:19233/healthRuff and diff hygiene on changed files.
Test Result
/healthreturned HTTP 200.git diff --checkpassed.Not tested: B200 / 1.7B Base or speech generation.
AI assistance: Codex assisted with implementation, tests, validation, and the PR description.