[Serve] [SGLang] PD disaggregation - #63741
limarkdcunha wants to merge 41 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces SGLang Prefill-Decode (PD) disaggregated LLM serving by adding SGLangPDPrefillServer and SGLangPDDecodeServer deployments, and updating the SGLang engine to pass bootstrap coordination fields. Feedback on these changes highlights critical issues: first, setting attributes directly on Pydantic v2 models will raise runtime errors, so object.__setattr__ should be used instead; second, the prefill generator must be consumed in a background task to prevent premature garbage collection and engine hangs; third, asyncio needs to be imported to support this background task; and finally, the new servers should be added to the __all__ list in deployment.py to be properly exposed in the public API.
|
can you add a target in |
|
Added the target in release_tests.yaml. Thanks |
jeffreywang88
left a comment
There was a problem hiding this comment.
Left some design questions for you to consider while revising the RFC.
| to the right place. The decode server generates the bootstrap_room upfront and | ||
| dispatches both sides simultaneously — it does not wait for a prefill response | ||
| before starting decode. |
There was a problem hiding this comment.
Looks similar to the parallel handoff pattern (https://github.com/ray-project/ray/pull/63950/changes#diff-ccc2365e3ea6c5f4fd309223ecbaaa15d44deb1d510f0acf985604060a7f1747R56). Could you assess the feasibility of leveraging the established concurrent handoff mechanism?
| # LLMServer.get_deployment_options calls get_engine_config(), which | ||
| # unconditionally imports vLLM. The SGLang byod image uninstalls vLLM, |
There was a problem hiding this comment.
We will need to somehow decouple vLLM from the common paths. It might require a refactor. Could you explore some options?
| carrying the same prefill bootstrap host/port/room — the decode | ||
| KVReceiver connects to that prefill bootstrap server and blocks | ||
| internally waiting for the KV cache to arrive. | ||
| 5. Streams the decode response back to the client. |
There was a problem hiding this comment.
This is exactly what we need, nice!
| For each chat/completions request it: | ||
| 1. Reads the PREFILL node's bootstrap_host and bootstrap_port, fetched | ||
| from the prefill deployment at init (the bootstrap server lives there). | ||
| 2. Generates a unique bootstrap_room integer. |
There was a problem hiding this comment.
In the design doc, it'd be great to clarify what bootstrap_room does and where it lives.
| # TODO: Users currently need to set disaggregation_mode manually in engine_kwargs. | ||
| # The builder should set this automatically since it already knows which | ||
| # config is prefill and which is decode. |
There was a problem hiding this comment.
We should emulate ray serve LLM's PD API for vLLM here. The user API should be similar.
|
|
||
| Unlike the vLLM flow, we do not wait for a prefill response before | ||
| starting decode — the bootstrap_room is established upfront and both | ||
| sides coordinate directly via SGLang's bootstrap server. |
There was a problem hiding this comment.
Is this bootstrap server strictly required?
| pass | ||
|
|
||
|
|
||
| @PublicAPI(stability="beta") |
There was a problem hiding this comment.
We should start from alpha.
| - UCX_TLS=all | ||
| - UCX_NET_DEVICES=all |
There was a problem hiding this comment.
Could you help me understand why we need these?
There was a problem hiding this comment.
When we use SGLang with NIXL, it uses a networking library called UCX to move data directly from one GPU to another.
By default, our testing environment (the CI box) has strict or limited network paths. Without these two settings, UCX gets confused, picks a network path that doesn't actually have access to the GPUs, and the data transfer crashes.
Here is what these two lines specifically tell UCX to do:
-
UCX_TLS=all: Tells it, 'You are allowed to use any available transport method (like shared memory, TCP, or CUDA-IPC) to move this data.'
-
UCX_NET_DEVICES=all: Tells it, 'You are allowed to look at all network devices on this machine to find a working path.'
Basically, it forces UCX to stop being picky and use whatever path works on our test machines so the test doesn't fail. Once we know the absolute bare minimum network settings needed for this specific CI box, we can narrow this down."
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
|
|
||
| # Does this handle when every worker is full ? | ||
| # Ans - it actually does it the correct way by parking the request and going to sleep, | ||
| # rather than spinning and wasting CPU. |
There was a problem hiding this comment.
Accidental personal notes in router
Low Severity
A Q&A-style personal note (Does this handle... / Ans - ...) and a # background dispatcher aside landed in the core Serve request router. These read as scratch notes rather than intentional docs and do not belong in production code.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e467352. Configure here.
|
Hi @limarkdcunha — I saw that this POC is still in progress, including the hardware-availability note. I'd like to help with a bounded validation slice on your existing connector instead of starting another P/D implementation. I inspected head This would validate the concrete bootstrap/request-shaping contract, not duplicate your builder, orchestrator, room-uniqueness tests, or claim that CPU tests replace NIXL transfer or the required two-node release test. I can also help with a separately reported single-host GPU smoke once the environment is ready; I cannot claim two-node coverage from one shared L20 host. Is this a useful subtask to take on, and is this head still the right integration target? I will keep any preparation in a local branch until we agree on how to integrate it; I am not assuming permission to push to your branch. AI assistance is being used, and Ray's human review/local-test submission requirements will still apply before opening any code PR. Preparation update (2026-09-06): the bounded local extension now passes 38 CPU cases (10 existing + 28 using the real request models), with the five GPU release cases explicitly deselected. All applicable pre-commit hooks pass. Production code is unchanged. This result does not establish GPU/NIXL/KV-transfer or two-node correctness; the initial new-test failures were test-side nested-message assumptions, not upstream defects. The patch remains local and unpushed pending integration agreement and the required human review. Single-host GPU follow-up (2026-09-06 UTC): the unchanged PR head Environment: Python 3.12, Ray nightly core The successful container exited normally and released both GPUs. Command: |
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
|
FYI @xyuzh |
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
| ) | ||
|
|
||
|
|
||
| # this is where routing happens from |
There was a problem hiding this comment.
Scratch note committed in proxy
Low Severity
A personal navigation note was left on ProxyActor in core Serve. It is unrelated to PD disaggregation and does not document behavior.
Reviewed by Cursor Bugbot for commit b201b25. Configure here.
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Reviewed by Cursor Bugbot for commit f4207a4. Configure here.
Signed-off-by: Limark Dcunha <limarkdcunha@gmail.com>


Description
A POC PR for the my proposal (#63257) related to Ray Serve SGLang PD disaggregation support.
Related issues - #62792 #63257