Skip to content

feat: [AMD Support via SemiAnalysis][2/4] vLLM MoRI-IO integration - #432

Open
cquil11 wants to merge 13 commits into
NVIDIA:mainfrom
SemiAnalysisAI:upstream/vllm-mori
Open

cquil11 wants to merge 13 commits into
NVIDIA:mainfrom
SemiAnalysisAI:upstream/vllm-mori

Conversation

@cquil11

@cquil11 cquil11 commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Integrate native vLLM Router with MoRI-IO prefill/decode workers. Static router registration and NIXL port handling do not describe MoRI's ZMQ discovery path; this PR supplies the connector-specific launch, port, and readiness contract for it on the seams main now has: a connector table row on the vLLM backend, discovery hooks on the vLLM Router frontend, and per-process listeners from the port allocator.

Stacked on #431, which combines AMD hardware enablement and required cluster runtime. #433 follows this PR; ATOM uses Mooncake and has no engine dependency on MoRI.

Review map

  • backends/vllm.py: moriio is a KVConnector row with discovery=True and a mode-dependent role (the row type, the connector_for_mode resolver, and the discovery seam landed on main in refactor(frontends,vllm): readiness probes see the recipe; one connector resolver; discovery seam on static routers #491, so no connector name is compared in a branch here). kv_transfer_config(mode, process, runtime) builds the worker JSON; the topology arguments are optional and only a discovery row reads them. For a discovery connector the extra config carries proxy_ip (head node), proxy_ping_port, http_port, host_ip (the worker's own routable IP), handshake_port, notify_port, read_mode: the keys moriio_common.py reads (vllm-project/vllm 9679173788). Upstream's fixed defaults of 6301 and 61005 collide as soon as two workers share a node, so the allocator hands each worker a handshake port and a notify block of one port per local rank (upstream adds the DP and TP rank to the base); a worker built without them is refused. Such a worker gets no VLLM_PORT scan range because MoRI-IO opens listeners inside TP children that inherit it.
  • frontends/vllm_router.py, ports.py: in discovery mode the vLLM Router adds --kv-connector <name> --vllm-discovery-address 0.0.0.0:36367 (RouterArgs in vllm-project/router) and validates the discovery rules (both roles on the connector, a P/D topology, one router on the head node). Readiness is the Router's own /health, which answers 503 until a prefill and a decode have registered (vllm-project/router 43140bc8e2, VllmPDRouter::health); the probe reads the recipe refactor(frontends,vllm): readiness probes see the recipe; one connector resolver; discovery seam on static routers #491 hands it, so core/health.py has no per-frontend branch and no completions POST. The probe_ready docstring records why /health and not /workers, and what the upstream fix would be.
  • core/schema.py: refuses a discovery connector with any frontend other than vllm-router; the Router's own rules live in its validate.
  • core/topology.py: Process.moriio_handshake_port and moriio_notify_port.
  • Docs: docs/vllm-router.md gains a MoRI-IO discovery section with the full contract table; config-reference.md points at it; the port tables in CLAUDE.md and docs/architecture.md list the two kinds. examples/vllm/vllm-router-moriio-disagg.yaml (colocated P/D) is in the examples matrix and in every example-driven test.
  • Tests: worker JSON against the realized topology, refusal without listeners, colocated listener allocation, the role override, the discovery command, the bootstrap port, the /health probe, and the recipe rules.

All four fork PRs target upstream main and are manually stacked: #431#432#433#476. Each later branch includes the preceding layers; merge in order and rebase the remaining PRs.

Review only this layer.

@cquil11
cquil11 force-pushed the upstream/vllm-mori branch 2 times, most recently from 3a13d27 to 56bd982 Compare September 17, 2026 22:26
@cquil11 cquil11 changed the title [AMD Support][2/3] vLLM MoRI-IO integration [AMD Support][2/4] vLLM MoRI-IO integration Sep 17, 2026
@cquil11 cquil11 changed the title [AMD Support][2/4] vLLM MoRI-IO integration feat: [AMD Support][2/4] vLLM MoRI-IO integration Sep 17, 2026
@cquil11 cquil11 changed the title feat: [AMD Support][2/4] vLLM MoRI-IO integration feat: [AMD Support via SemiAnalysis][2/4] vLLM MoRI-IO integration Sep 17, 2026
@cquil11
cquil11 force-pushed the upstream/vllm-mori branch 2 times, most recently from 802578c to 401ddc2 Compare September 18, 2026 18:36
cquil11 and others added 6 commits September 18, 2026 16:26
The GPU-mask test never branched on the variable name, so parametrize over
the behavioral cases only. Drop the duplicate dry-run mask value and an
assertion already implied by exact equality, and check both resolved
control-plane IPs on the runtime.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The one-server-per-endpoint, multi-node DEP8 hybrid pool, and NIXL P/D
worker tests restated assertions that test_vllm_router_frontend.py and
the existing DP tests already make. Keep the MoRI-IO and device-binding
cases, which exercise logic this branch adds, and update the latter to
the set_visible_devices field name introduced by the runtime branch.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Brings in NVIDIA#486 (RuntimeContext.container_log_dir), so the branch's own
_container_log_path helper and its Path import are dropped in favor of
the shared accessor; the config dump test is now identical on both sides.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
main now routes every backend question through BackendProtocol and every frontend decision through the frontend class, so the AMD runtime changes land on those seams instead of beside them: the visible-devices hook is the protocol member should_set_visible_devices() on every backend (vLLM answers from set_visible_devices, the others True), the worker stage builds the mask from runtime.visible_devices_env in one place, and the static router's pre-start worker probe reads config.health_check and runtime.srun_options directly instead of probing with getattr.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
…M Router

Merge of upstream/amd-runtime (main + NVIDIA#431) into this branch, with the
MoRI-IO integration rebuilt on the seams main now has instead of beside
them. What the feature does is unchanged: engine.connector: moriio runs a
ROCm prefill/decode pair over AMD's MoRI-IO connector with the vLLM Router
in ZMQ discovery mode.

Backend (backends/vllm.py)
- The connector table rows are KVConnector(kv_connector, kv_role,
  module_path, discovery). "moriio" is a row with discovery=True and a
  mode-dependent kv_role; no connector name is compared in a branch.
- connector_for_mode(mode) is the one resolver for engine.connector and the
  roles.<role>.args.connector override; kv_connector_for_mode and
  discovers_workers are built on it. All three command paths use it.
- kv_transfer_config(mode, process, runtime) builds --kv-transfer-config.
  For a discovery connector the extra config carries proxy_ip (head node),
  proxy_ping_port, http_port, host_ip (the worker's own routable IP),
  handshake_port and notify_port (the keys moriio_common.py reads,
  vllm-project/vllm 9679173788), read_mode. Upstream's defaults of 6301 and
  61005 for every worker collide on colocated P/D; a worker built without
  its listeners is refused rather than given them.
- The allocator hands a discovery worker MORIIO_HANDSHAKE_PORTS and a
  MORIIO_NOTIFY_PORTS block of one port per local rank (upstream adds the
  DP and TP rank to the notify base) and no VLLM_PORT scan range, since
  MoRI-IO opens its other listeners inside TP children that inherit it.
  Process gains moriio_handshake_port and moriio_notify_port.

Frontend (frontends/vllm_router.py, static_router.py)
- StaticRouterFrontend.discovers_workers(backend) decides whether the P/D
  URLs go on the command line; build_router_command takes the backend.
- VLLMRouterFrontend adds --kv-connector <name> --vllm-discovery-address
  0.0.0.0:36367 (RouterArgs in vllm-project/router), advertises no NIXL
  bootstrap port, and validates the discovery rules: both roles on the
  connector, a P/D topology, one router on the head node.
- Readiness is the Router's own /health, 503 until a prefill and a decode
  registered (vllm-project/router 43140bc8e2, VllmPDRouter::health), through
  probe_ready. probe_ready now receives the recipe on every frontend, and
  wait_for_model passes it; the completions POST in core/health.py is gone.
- The schema refuses a discovery connector with any other frontend.

Docs, example, tests
- docs/vllm-router.md gains a MoRI-IO discovery section with the contract
  table; config-reference.md points at it (the duplicate section is gone);
  ports tables in CLAUDE.md and docs/architecture.md list the two kinds.
- examples/vllm/vllm-router-moriio-disagg.yaml, colocated P/D, is in the
  examples matrix and in every example-driven test.
- Tests cover the worker JSON against the realized topology, the refusal
  without listeners, colocated listener allocation, the role override, the
  discovery command, the bootstrap port, the /health probe, and the recipe
  rules.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
@ishandhanani

Copy link
Copy Markdown
Collaborator

Merged the updated #431 head into this branch and rebuilt the MoRI-IO layer on what main now has (#487 frontend protocol, #489 ports and backend protocol). Same feature, same recipe surface (engine.connector: moriio); the implementation moved onto the existing seams. I updated the review map in the description to match. What changed and why, in review-finding order:

  1. Readiness is the Router's own /health, not a completions POST. Since vllm-project/router 43140bc8e2 VllmPDRouter::health answers 503 Waiting for discovered workers until a prefill and a decode have registered, then 200. VLLMRouterFrontend.probe_ready polls it in discovery mode; probe_ready now takes the recipe on every frontend so the frontend decides, and core/health.py has no frontend_type == "vllm-router" branch and no model name to thread through.
  2. Handshake and notify ports come from the allocator. Upstream defaults every worker to 6301 and 61005, which collide the moment two workers share a node (the repo's own colocated P/D layout). Each discovery worker now gets moriio_handshake_port and a moriio_notify_port block of one port per local rank (upstream adds the DP and TP rank to the base, moriio_common.py 9679173788), plus host_ip set to its own routable IP instead of upstream's interface guess. A worker built without these is refused rather than defaulted.
  3. One resolver for the connector. connector_for_mode(mode) covers engine.connector and the roles.<role>.args.connector override; uses_moriio() read only the engine-level field. discovers_workers() is built on it, and validate requires both roles to agree.
  4. The name lives in the table. _CONNECTOR_MAP rows are KVConnector with a discovery attribute; moriio is a row, and nothing compares the string "moriio" in a branch. kv_transfer_config builds the JSON from the row and the topology.
  5. Validation moved to the frontend. The P/D-only, single-router, head-placement rules are in VLLMRouterFrontend.validate; the schema only refuses a discovery connector with any other frontend.

Also: examples/vllm/vllm-router-moriio-disagg.yaml, a MoRI-IO section in docs/vllm-router.md with the contract table, and the duplicate vllm-router heading in config-reference.md folded into the existing one. Full suite on Python 3.13 and 3.10, and srtctl dry-run on all 26 baseline example recipes is unchanged from #431.

One thing I could not verify here: the exact vllm-router version your container ships. Discovery-aware /health needs 43140bc8e2 (2026-08-10) or later; with an older router the readiness gate would time out instead of passing early, which is the safe failure.

ishandhanani added a commit that referenced this pull request Sep 22, 2026
…tor resolver; discovery seam on static routers (#491)

Three seams a router mode or a KV connector needs and main did not have.
Extracted from the vLLM MoRI-IO integration (#432), which otherwise had to
ship them inside a feature PR. No behavior changes; the dry-run output of
all 26 example recipes is unchanged.

- FrontendProtocol.probe_ready takes the recipe as its last argument and
  wait_for_model passes it (keyword-only config). A frontend whose
  readiness contract depends on the recipe (the vLLM Router when its
  workers register through a discovery connector) can decide inside its
  own probe instead of a frontend-name branch in core/health.py. Every
  implementation gained the parameter; none reads it yet.
- VLLMProtocol.connector_for_mode(mode) is the one resolver for
  engine.connector and the roles.<role>.args.connector override, in the
  get_config_for_mode style the design rules call for; kv_connector_for_mode
  returns the table row and kv_transfer_config(mode) the flag. All three
  worker command paths use it instead of resolving the fields inline.
- _CONNECTOR_MAP rows are KVConnector(kv_connector, kv_role, module_path,
  discovery) instead of bare dicts, so a connector can carry behavior
  without a branch on its name. kv_role None follows the worker mode. The
  JSON handed to --kv-transfer-config is byte-identical to before.
- StaticRouterFrontend.discovers_workers(backend) decides whether the P/D
  URLs go on the router command line; build_router_command receives the
  backend. The vLLM Router answers from VLLMProtocol.discovers_workers,
  which is False for every connector in the table today, and advertises no
  NIXL bootstrap port for discovered workers.
- tests/test_vllm_connectors.py pins the resolver, the presets' exact JSON,
  raw passthrough, and the mode-dependent role; the router tests cover a
  discovering router's command and the probe receiving the recipe.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
… the upstream fix is

In discovery mode the Router serves /workers and /get_server_info from its empty static list and exposes the discovery registry only through /health, which proves one worker per role rather than all of them. The override in VLLMRouterFrontend.probe_ready exists for that gap; when vllm-project/router merges the discovery registry into /workers (src/routers/router_manager.rs), the override goes and the count-based probe covers both modes.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>

# Conflicts:
#	docs/schema-reference.md
Brings main with NVIDIA#491 (readiness probes see the recipe, one connector resolver, KVConnector rows, discovery seam on static routers) underneath this layer. The hunks that NVIDIA#491 carried are dropped here; what remains is the moriio row, the two MoRI-IO port kinds, the Router's discovery command, validation and /health probe, the schema guard, docs, example, and tests. kv_transfer_config keeps NVIDIA#491's signature and takes the worker topology as optional arguments that only a discovery row needs.

Signed-off-by: Ishan Dhanani <idhanani@nvidia.com>
@ishandhanani

Copy link
Copy Markdown
Collaborator

Restacked on #491 (merged) through the updated #431 head. The seams this PR used to carry are now on main, so the layer over #431 dropped from 26 files (+762/-74) to 19 files (+614/-28), and its source diff is only the feature: the moriio row and its topology JSON in backends/vllm.py, the two port kinds and Process fields, the Router's discovery command, validation and /health probe in frontends/vllm_router.py, and the schema guard. Everything else is docs, the example recipe, and tests.

One API note from the restack: kv_transfer_config(mode, process=None, runtime=None) keeps #491's one-argument form; only a discovery row reads the topology arguments and refuses to run without them.

Full suite on Python 3.13 and 3.10; srtctl dry-run on all 26 baseline example recipes is unchanged from #431. Description updated to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants