Repository navigation
[Frontend / Core] Support Running Frontend Only & Validate Stage IDs - #8626
Conversation
|
This PR appears to belong to: docs/design/module/observability.md. Module owners: @lishunyang12 @vraiti @hsliuustc0106 Routing: @lishunyang12 via module of the changed files; @vraiti via module of the changed files; @hsliuustc0106 via CODEOWNERS @alex-jw-brooks, 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 routing recordAssigned Strict on cursor (cursor-grok-4.6-high) under experiment |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
Omni ReviewBot: supersededThe CI failure noted on |
|
|
||
| def _parse_stage_id(value: str) -> int: | ||
| """Parse `--stage-id` as a non-negative int, or `none` (no local stage) as `FRONTEND_ONLY_ID_FILTER`.""" | ||
| if value == "none": |
There was a problem hiding this comment.
Add a .lower() since users would likely type None.
There was a problem hiding this comment.
Sure, sounds reasonable to me. We may want to take a pass for other flags at some point though and ensure they normalize "none" consistently. I didn't add it initially because other flags don't normalize it. For example
This is ok
$ vllm serve black-forest-labs/FLUX.2-klein-4B --omni --port 8313 --cache-backend noneBut this fails:
$ vllm serve black-forest-labs/FLUX.2-klein-4B --omni --port 8313 --cache-backend None
[rank0]: RuntimeError: Orchestrator initialization failed: Unsupported cache backend: None. Supported: 'cache_dit', 'tea_cache', 'mag_cache', 'sea_cache', 'step_cache'| if not (value.isascii() and value.isdigit()): | ||
| raise argparse.ArgumentTypeError(f"--stage-id must be a non-negative integer or 'none', got {value!r}") | ||
| return int(value) |
There was a problem hiding this comment.
Can't think of an edge case here, but a try catch for ValueError would be more lenient.
There was a problem hiding this comment.
I think it should be safe since we guard isascii(), because the main edge case would be sub/superscripts 🤔 imo there isn't any intuitive behavior if int coercion fails though, unless you mean still failing loudly here, but reraising with a more direct error string?
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 120s (try 2 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; retrying strict/cursor/cursor-grok-4.6-high in 600s (try 3 of 3)). |
Omni ReviewBot attempt recordReview attempt ended as failed (failed; falling back to direct/cursor/auto). |
695224e to
8fcb22b
Compare
Omni ReviewBot: supersededThe CI failure noted on |
NickCao
left a comment
There was a problem hiding this comment.
Tested running Qwen/Qwen3-Omni-30B-A3B-Instruct using this.
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Signed-off-by: Alex Brooks <albrooks@redhat.com>
Purpose
Currently, stage ID can take any numeric value, which isn't properly validated. E.g., users can pass
--stage-id -1as a hack to try run against all stages running as--headless. Similarly, passing an out of range stage id, e .g., 999, isn't validated and behaves the same.Running just the frontend (i.e., API server & orchestrator) is a useful capability, and will be very important for effectively running all stages independently with multiple replicas & external orchestration, which is being worked on in #6389. As such, we should both support this as a formal feature, and tighten the validation for correctness. This is done as follows:
--stage-id nonestarts just API server and orchestrator and is mutually exclusive with--headless, because--headlessdoesn't make sense if all stages are remoteExamples:
vllm serve --omni \ Qwen/Qwen3-Omni-30B-A3B-Instruct \ --stage-id none \ --omni-master-address 127.0.0.1 \ --omni-master-port 20000In the logs, you'll see:
will raise:
ValueError: No stage config found for stage_id=1000. Available stage ids: [0, 1, 2]Test Plan
Added tests at various levels, for parsing and ensuring that passing the
FRONTEND_ONLY_ID_FILTERthrough engine creation will create a plan for all remote replicas.vLLM Version::
0.31.0vLLM-Omni Commit:: 32ead8e
Test Result
Tests pass.