feat(rollout): vLLM prefill/decode (PD) disaggregation via static vllm-router - #166
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for Prefill/Decode (PD) disaggregation in the vLLM engine by launching prefill and decode engines with the NIXL KV connector and configuring unique side-channel ports. It also adds a static PD router startup flow in the rollout module that collects engine URLs before launching the router. The review feedback suggests replacing a process-alive assertion with an explicit conditional check to avoid issues when assertions are disabled in production, and optimizing the sequential ray.get calls inside a loop by batching them into a single parallelized call.
| process.daemon = True | ||
| process.start() | ||
| time.sleep(3) | ||
| assert process.is_alive() |
There was a problem hiding this comment.
Using assert for production control flow or process health checks is a bad practice because assertions can be disabled globally in Python when run with the -O (optimize) flag. If assertions are disabled, assert process.is_alive() will be completely ignored, and if the process failed to start or crashed immediately, the code will proceed silently. Use an explicit conditional check and raise a RuntimeError instead.
| assert process.is_alive() | |
| if not process.is_alive(): | |
| raise RuntimeError("Failed to start vLLM-router process.") |
| if use_static_pd_router: | ||
| prefill_urls: list[tuple] = [] | ||
| decode_urls: list[str] = [] | ||
| for g in server_groups: | ||
| for e in g.engines: | ||
| if e is None: | ||
| continue | ||
| if g.worker_type == "prefill": | ||
| url, bport = ray.get([e.get_url.remote(), e.get_pd_bootstrap_port.remote()]) | ||
| if url: | ||
| prefill_urls.append((url, bport)) | ||
| elif g.worker_type == "decode": | ||
| url = ray.get(e.get_url.remote()) | ||
| if url: | ||
| decode_urls.append(url) | ||
| _launch_static_pd_router(args, router_ip, router_port, prom_port, prefill_urls, decode_urls) |
There was a problem hiding this comment.
Gathering Ray ObjectRefs inside a nested loop and calling ray.get sequentially on each iteration is a performance bottleneck. It forces sequential round-trips to the Ray actors, blocking the main thread. Instead, you should collect all remote calls into a list and resolve them in a single parallelized ray.get call.
if use_static_pd_router:
prefill_engines = []
decode_engines = []
for g in server_groups:
for e in g.engines:
if e is None:
continue
if g.worker_type == "prefill":
prefill_engines.append(e)
elif g.worker_type == "decode":
decode_engines.append(e)
prefill_refs = []
for e in prefill_engines:
prefill_refs.extend([e.get_url.remote(), e.get_pd_bootstrap_port.remote()])
decode_refs = [e.get_url.remote() for e in decode_engines]
all_results = ray.get(prefill_refs + decode_refs)
prefill_results = all_results[:len(prefill_refs)]
decode_results = all_results[len(prefill_refs):]
prefill_urls: list[tuple] = []
for i in range(0, len(prefill_results), 2):
url = prefill_results[i]
bport = prefill_results[i+1]
if url:
prefill_urls.append((url, bport))
decode_urls: list[str] = [url for url in decode_results if url]
_launch_static_pd_router(args, router_ip, router_port, prom_port, prefill_urls, decode_urls)…m-router Add the PD (prefill/decode) split on top of the colocate DP+EP rollout: - vllm_engine.py: for prefill/decode rollout groups (node_rank 0), pin the NIXL side-channel host/port to the orchestrator-allocated bootstrap port (VLLM_NIXL_SIDE_CHANNEL_HOST/PORT); plumb disaggregation_bootstrap_port through _compute_server_args and persist it on the actor. - rollout.py: _start_router gains a static-PD path (bind + static prefill_urls/decode_urls); start_rollout_servers reserves the router endpoint, starts engines, collects per-worker_type URLs, then launches the vllm-router in PD mode. Engines do not self-register. NIXL pull mode carries the side-channel coords in the prefill response (kv_transfer_params), so prefill_urls use (url, None) -- the per-URL bootstrap port is Mooncake-only and not advertised to the router. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ef58226 to
21026bf
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
…rgs) Router.disable_health_check was added speculatively but does not exist in the installed vllm_router version. Router already has disable_circuit_breaker which handles the transient-RDMA concern. Remove the invalid kwarg. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
We’d better test it e2e. |
Cross-Node PD Validation Results (H200 ×2)Setup
Runs
Eval Before Train (iteration 0)
Steady-State Performance (iteration 1, no eval overhead)
TP=2 is the sweet spot — faster weight sync offsets the slightly slower per-engine throughput. Issues Encountered (all environment-level, no vime code bugs)
ConclusionCross-node PD disaggregation via NIXL works correctly on H200 ×2. No new code bugs found. The |
…m-router (#166) * feat(rollout): vLLM prefill/decode (PD) disaggregation via static vllm-router Add the PD (prefill/decode) split on top of the colocate DP+EP rollout: - vllm_engine.py: for prefill/decode rollout groups (node_rank 0), pin the NIXL side-channel host/port to the orchestrator-allocated bootstrap port (VLLM_NIXL_SIDE_CHANNEL_HOST/PORT); plumb disaggregation_bootstrap_port through _compute_server_args and persist it on the actor. - rollout.py: _start_router gains a static-PD path (bind + static prefill_urls/decode_urls); start_rollout_servers reserves the router endpoint, starts engines, collects per-worker_type URLs, then launches the vllm-router in PD mode. Engines do not self-register. NIXL pull mode carries the side-channel coords in the prefill response (kv_transfer_params), so prefill_urls use (url, None) -- the per-URL bootstrap port is Mooncake-only and not advertised to the router. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Update rollout.py Signed-off-by: aoshen02 <aoshen@inferact.ai> * Update rollout.py Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(rollout): remove disable_health_check (not in vllm_router RouterArgs) Router.disable_health_check was added speculatively but does not exist in the installed vllm_router version. Router already has disable_circuit_breaker which handles the transient-RDMA concern. Remove the invalid kwarg. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Carves the PD (prefill/decode) disaggregation half out of #108 so that PR can stay DP+EP-only. On top of the colocate DP+EP rollout, this adds the prefill/decode split via the production vllm-router in static mode:
vllm_engine.py— for a rollout group whoseworker_typeisprefill/decode(node_rank 0), pin the NIXL side-channelVLLM_NIXL_SIDE_CHANNEL_HOST/PORTto the orchestrator-allocateddisaggregation_bootstrap_port, plumbed through_compute_server_argsand persisted on the actor. The KV connector (--kv-transfer-config) itself is supplied by the operator via normal vLLM arg passthrough.rollout.py—_start_routergains a static-PD path (bind+ staticprefill_urls/decode_urls, health check disabled).start_rollout_serversreserves the router endpoint, starts engines (which do not self-register), collects per-worker_typeURLs, then launches the router in PD mode.NIXL pull mode → no per-URL bootstrap port
vime uses vLLM's NIXL pull connector: the prefill engine returns the side-channel coords (
remote_host/remote_port/remote_block_ids) to the decode side at request time via the response'skv_transfer_params. The router never consumes a per-prefill-URL bootstrap port (that path is Mooncake-only). Soprefill_urlsare(url, None)tuples — matching SkyRL's static-PD reference andvllm_router._parse_prefill_urls(which explicitly acceptsnone).Relationship to #108
build_vllm_cmd_and_env, so they compose without conflict.Test
py_compileclean on both files.(url, None)tuple shape. One pre-existing latent crash surfaced in the refactored_start_router(the router-reuse early-return returned a 2-tuple while callers unpack 3) and is fixed here (returns(ip, port, None)).🤖 Generated with Claude Code