Repository navigation
[PD] Add the missing Prefill bootstrap timeout for NIXL - #34692
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
ShangmingCai
left a comment
There was a problem hiding this comment.
If it appears on all backends now, should we just move it into common? Also, I remember NIXL has his own timeout notification mechanism in the C++ player, should we respect that?
Thanks! I agree that this behavior should ultimately be unified in the common protocol layer. However, Mori still differs in this path because its bootstrap timeout must go through _finalize_failure() to preserve its backend-specific terminalization, terminal-once guard, and Decode notification behavior. Given that difference, I would prefer not to introduce a larger Common extraction in this PR. At the current stage, I am focusing on low-risk defensive parity fixes: aligning clearly missing behavior without changing the existing backend success paths, wire format, or terminal semantics. I plan to submit a dedicated follow-up PR for the remaining safe defensive alignments. Once those straightforward gaps are closed, I will move to the next stage and start extracting the shared protocol logic with the backend-specific terminal behavior explicitly accounted for. Regarding the NIXL timeout mechanism, this timeout occurs before a NIXL transfer is created. While the Sender is in Bootstrapping, Prefill is still waiting for the per-room Decode metadata through SGLang's bootstrap channel; there is no NIXL transfer handle or completion notification yet. I also did a preliminary scan of the timeout mechanisms in the NIXL v1.3.0 and latest C++ sources. The mechanisms I found apply to progress-thread wakeups, metadata transport, connection/handshake handling, active transfers, or NIXL-EP operations. They do not appear to cover SGLang's pre-transfer Bootstrapping phase, so I do not think they need to be considered for this patch. |
|
/rerun-group disaggregation |
|
Results for 🚀 🚀 🚀 🚀 🚀 🚀 🚀 🚀 |
|
Failed tests are irrelevant. |
Background
RFC #33861 proposes gradually consolidating the duplicated PD request/room protocol logic in Mooncake, NIXL, and Mori into a single common protocol layer, while keeping third-party engine-specific behavior in each backend Transport.
Before extracting the common protocol layer, Step 1 of the implementation plan in #34510 aligns clear, non-controversial semantic gaps through small, independent, backend-local PRs. This PR addresses the first gap: the missing bootstrap timeout in the NIXL Prefill Sender.
The bootstrap timeout covers the following case:
Current Problem
CommonKVSenderalready provides_check_bootstrap_timeout():This helper:
init_time;Nonewhile the request remains within the deadline;KVPoll.Failed;KVPoll.Failed.However, the current NIXL Sender has two missing pieces:
NixlKVSender.__init__()does not record the start of the Prefill bootstrap deadline;NixlKVSender.poll()does not call the existing helper while the room is inKVPoll.Bootstrapping.NIXL currently records
_transfer_start_timeonly for actual KV/state transfer latency:That timer starts when the first meaningful KV/state chunk is submitted. It does not include the bootstrap phase spent waiting for Decode metadata, so it cannot replace
init_time.Similarly, the
init_timeset byNixlKVReceiver.send_metadata()belongs to the Decode Receiver waiting timeout. It is not the Prefill Sender bootstrap deadline.As a result, if Decode destination metadata never arrives, a NIXL Prefill room can remain in
KVPoll.Bootstrappingindefinitely.Existing Behavior in the Other Backends
Mooncake
Mooncake records the bootstrap start time when creating the Sender:
Its
poll()calls the common helper while the room remains inKVPoll.Bootstrapping:Mooncake therefore cannot wait indefinitely for missing Decode metadata.
Mori
Mori also records the bootstrap start time when creating the Sender:
Mori does not call
_check_bootstrap_timeout()directly. Instead, it performs the equivalent check inline in its ownpoll():Mori uses an inline implementation because its Sender currently owns backend-specific terminalization. In addition to updating the local room state,
_finalize_failure():conclude_state;_notify_lock/status_notifiedto emit the terminal status at most once;The common
_check_bootstrap_timeout()helper only records a local failure and updates the Manager status. It does not understand Mori's remote notification or terminal-once state. Mori therefore implements the same deadline semantics while retaining its backend-local failure finalization.This PR only aligns NIXL with the bootstrap deadline already implemented by Mooncake and Mori. It does not change Mori's terminalization behavior.
Changes
This PR only changes
NixlKVSender.1. Record the bootstrap start time when creating the Sender
2. Call the existing timeout helper while Bootstrapping
The timeout check runs only when
status == KVPoll.Bootstrapping. Once enough Decode metadata has arrived and the room transitions toWaitingForInput, this deadline no longer applies.Behavior After This Change
Before:
After:
The deadline continues to use the existing environment variable:
Users can continue to relax the deadline through the existing environment variable. This PR adds no new configuration.
Testing
To keep the implementation PR diff minimal, the CPU regression test is currently stored on a dedicated branch in the fork:
Test scenario:
Assertions:
sender.init_time == 10.0;sender.poll() == KVPoll.Failed;request_status[room] == KVPoll.Failed;timed out.Test results:
CI States
Latest PR Test (Base): ❌ Run #31676012627
Latest PR Test (Extra): ❌ Run #31676012346