Repository navigation
[PD] Consolidate sender and receiver implementations - #41140
Open
jambow0320 wants to merge 7 commits into
Open
jambow0320 wants to merge 7 commits into
jambow0320 wants to merge 7 commits into
Conversation
This was referenced Sep 30, 2026
jambow0320
force-pushed
the
refactor/pd-receiver-control-send
branch
from
October 3, 2026 17:03
f00c8bf to
e59c5fb
Compare
jambow0320
marked this pull request as ready for review
October 3, 2026 17:06
jambow0320
requested review from
ByronHsu,
Duyi-Wang,
HaiShaw,
Lzy17,
ShangmingCai,
billishyahao,
hnyls2002,
kkHuang-amd and
sogalin
as code owners
October 3, 2026 17:06
jambow0320
force-pushed
the
refactor/pd-receiver-control-send
branch
2 times, most recently
from
October 4, 2026 11:49
d62fa9f to
c4fa949
Compare
4 of 5 tasks
Lzy17
reviewed
Oct 8, 2026
Collaborator
There was a problem hiding this comment.
Thanks for the PR. Items:
- Rebase. Main's Mooncake worker already calls
wait_event.synchronize(). Keep that path in the unifiedsend()and correct the "only Mori useswait_event" statement. - Agree the merge order with #38959 (
poll_pp_consensus), #37358 (four new mori registration fields) and #40861 (NIXLwait_eventaccessor). - Add cross-node RDMA, staging and mori/AMD validation; only single-node H200 is covered.
- Mooncake/mori
transfer_latency_sis now measured from the first enqueue, which shifts nightly baselines. Please flag this to the nightly owners.
Keep backend sender/receiver classes as thin common subclasses. Move control-message encoding behind manager hooks and use ZMQ completion and staging notifications for NIXL with explicit background engine progress. Preserve the upstream destination interface and reject unsupported host destinations. Cover sender lifecycle, receiver publication, completion ordering, staging retries and failure cleanup. Refs: sgl-project#33861 Refs: sgl-project#34510
Keep the receiver poll identical to upstream, which reads the room status through check_status and handles the prefill-complete wait, instead of the latching variant. Inline the missing-bootstrap check in send_metadata and drop the _poll_room_status and _can_send_metadata helpers together with the tests that pinned their behavior.
Drop the stale notification argument from the staged-transfer call: it was bound to staging_buffer and raised a TypeError on the staging path. Leave enable_prog_thread at the NIXL default, which is already True.
…ments Conclude Success when the last chunk completes, as Mooncake does. The sender keeps reporting Transferring while a deferred staging chunk is outstanding, and decode waits for every chunk's scatter before completing. Restore the upstream wording of several comments and drop redundant ones.
Keep the cases that pin the wire format, the deferred staging hold and the NIXL ZMQ completion path, and restore upstream test names and setup where the behavior is unchanged. Drop lifecycle cases that restate the common implementation or duplicate existing coverage.
Mooncake now consumes the early-send wait event in its worker, as Mori does, so the shared send() comment names both. Drive the Mooncake early-send case through a real MooncakeKVSender, since send() delegates to CommonKVSender. Drop the removed chunk_id, notif and NIXL manager fields from the invalidated-peer reload tests.
jambow0320
force-pushed
the
refactor/pd-receiver-control-send
branch
from
October 9, 2026 11:42
c4fa949 to
540545d
Compare
Contributor
Author
|
@Lzy17 Thanks for the review! I have rebased onto the latest main and resolved the conflicts. Once #38959, #37358, and #40861 are merged, I will update this PR accordingly. For performance testing, I currently only have a single H200 node locally, so I will ask around whether someone can help run the tests. Finally, regarding |
15 of 40 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Mooncake, NIXL, and Mori each implement their own KV sender / receiver lifecycle (sending, registration, metadata publication, polling, timeouts, cleanup, failure handling). The code is duplicated, and the backends are inconsistent in details such as failure-reason recording, terminal-state latching, and timeout trigger conditions.
NIXL determines transfer completion differently from the other two backends:
{room}_kv_*,{room}_aux,{room}_state_*,{room}_stg_*, etc.). Decode callsagent.get_new_notifs()inNixlKVReceiver.poll()and usesTransferStatusto count, per PP rank, the arrival of chunks, parts, aux, and state to determine completion. Decode has to maintain a separate set of counting logic, and notifs are only read inside the receiver'spoll().This PR consolidates the sender / receiver lifecycle of the three backends into a common implementation, and switches NIXL completion notification to the same ZMQ mechanism used by Mooncake / Mori, no longer using NIXL notifs.
Refs: RFC #33861, Tracking #34510.
Modifications
1. NIXL switches to the same ZMQ completion mechanism as Mooncake and removes notifs
Prefill side
conclude_transfer(Success), which sends aKV_STATUSmessage to all non-dummy decode endpoints of the room. The failure path is the same as upstream: it sendsKV_STATUS=Failedwith the failure reason.CHUNK_READYis sent (sharingsend_chunk_readywith Mooncake). Before the next peer reuses the prefill-side packing buffer, the worker likewise waits for that handle to complete.send_notif.Decode side
KV_STATUS,CHUNK_READY,STAGING_REQ, andABORT_ACK.KV_STATUSis counted per prefill rank viaapply_prefill_status, and the room is set toSuccessoncerequired_prefill_response_numis reached;CHUNK_READYis passed to the staging handler viahandle_chunk_ready(shared with Mooncake).poll()only readsrequest_statusand no longer callsget_new_notifs().Removed
TransferStatus,update_transfer_status()and its notif parsing functions (_track_kv_arrival,_track_kv_part_arrival,_handle_stg_notification,_handle_aux_notification,_maybe_submit_last_scatter), andcheck_transfer_done().transfer_source_rank,TransferKVChunk.chunk_id(along with the counter in the sender and thechunk_idparameter ofadd_transfer_request), the NIXL manager'sexceptionsdict, and the receiver'sstarted_transfer/expects_state.notifparameter of each NIXL send function.Unchanged
2. Sender / receiver lifecycle consolidated into the common implementation
send,poll,clear,failure_exception,abort(sender) and_register_kv_args,send_metadata,clear,failure_exception,abort(receiver) are implemented inCommonKVSender/CommonKVReceiver. The receiver'spoll()uses the upstream common implementation.NixlKVSender,NixlKVReceiver,MooncakeKVReceiver, andMoriKVReceiverare empty subclasses.MooncakeKVSenderonly keeps the trace decorator (mooncake_trace_func) onsend.MoriKVSenderonly keepsclear, which holds the Mori-specific_abort_ack_lockwhen deferred KV release is enabled.build_receiver_registration_message()/build_receiver_metadata_message(): per-backend message encoding; the wire format is unchanged. Mooncake'sdevice_kv_indices(DSV4 HiSparse) is forwarded throughsend_metadata(**backend_kwargs).prepare_send_state_indices(): returns the input unchanged by default; Mori uses it to skip CP-replicated state and to normalize per component.sender_trace_module: set to"mooncake"by Mooncake, used to create the sender's trace context.add_transfer_request(): unified across the three backends as 4 positional parameters plus keyword parameterswait_event/trace_ctx; each backend ignores the parameters it does not use (wait_eventis used only by Mori,trace_ctxonly by Mooncake).send_chunk_ready()/handle_chunk_ready()are moved toCommonKVManagerand shared by Mooncake and NIXL.prefill_response_trackerchanges fromdefaultdict(set)to a plaindict, with the entry created when the receiver is constructed. The listener thread only reads the entry, so aSuccessarriving afterclear()does not re-create it._staging_ctxis declared onCommonKVManagerwith a default value ofNone.Behavior Changes After Unification
NIXL protocol
Source of the completion signal: decode completion changes from "counting notifs" to "counting
KV_STATUS=Successsent by prefill ranks", the same as Mooncake.When
Successis sent:Successis sent when all handles of the last chunk have completed. If an earlier staging chunk has been deferred due to insufficient staging space and is still queued:Transferringwhile_staging_outstanding > 0, so the source KV is not released;This matches how Mooncake handles it. Before this change, NIXL decode relied on notif counting to wait for all chunks to arrive.
Staging deferral retry: when a chunk is deferred, the worker first waits for the writes already posted in the current round to complete, and then retries (before this change it retried immediately without waiting for the posted handles). The retry still reprocesses all decode peers of the chunk, the same as before this change and as Mooncake.
Failure exception: the sender's
failure_exception()raisesKVTransferError, with the reason text taken fromfailure_records(the worker writesstr(e)on the failure path). Before this change, NIXL re-raised the original exception object caught in the worker, so the exception type and text format in scheduler logs change.Abort received only by prefill: if an abort reaches only the prefill side while the last chunk's writes are in flight, prefill does not send
Success, and decode does not complete the request. When prefill has already cleared the room, it sends no status, and decode fails the request afterSGLANG_DISAGGREGATION_WAITING_TIMEOUT(300 s by default). This matches Mooncake. Before this change, NIXL decode counted all notifs once the writes landed and completed the request.Sender (all three backends)
send()returns immediately once the sender has reached a terminal state (Success/Failed) and no longer enqueues. Before this change, Mooncake / Mori relied on the status check insideadd_transfer_request, and NIXL relied on_send_failed, which was only set infailure_exception().transfer_latency_s, timed from the first enqueue untilpoll()observesSuccess. Before this change these two backends did not set the field, and the metric usedcompletion_time - prefill_transfer_queue_entry_time(covering only the last chunk). With chunked prefill, the new value includes the compute time of subsequent chunks, solatency_msis larger andspeed_gb_sis smaller. NIXL's timing is unchanged.poll()returnsFailedwhen the room is not inrequest_status. Before this change, Mooncake raisedKeyErrorin this case and NIXL returnedWaitingForInput.poll()writesconclude_stateonce it observes a terminal state; before this change NIXL did not latch it.clear()saves the terminal state before removing manager state; cleanup of staging prefetch records (prefetched_rooms/prefetch_requested) is extended from NIXL to Mooncake.aux_indexandstate_indicesare only passed with the last chunk. Before this change NIXL passed them with every chunk, but the worker only read them on the last chunk, so behavior is unchanged.failure_exception()is unified asFailed due to an unknown reason from another rank(before this change it wasNIXL KVSender Exceptionfor NIXL andKV transfer failedfor Mori).Receiver (all three backends)
bootstrap_infosisNone,send_metadata()uniformly records a failure reason and sets the status toFailed. Before this change, NIXL only logged without recording a reason (so the failure was classified as coming from another rank), and Mori returned directly.send_metadata()raisesNotImplementedErrorwhen givenKVTransferDestination.HOST. None of the three backends implements host destinations; before this change, passing this argument raisedTypeErrordue to a signature mismatch.clear()writesconclude_statebefore removing manager state (a terminal state keeps its value; a non-terminal state is recorded asFailed).abort()can be called repeatedly and does not overwrite an already recorded failure reason when alreadyFailed. Mori'sabort()no longer immediately callsclear()and removes the failure reason: cleanup is done byfailure_exception(), which the scheduler calls after the nextpoll()returnsFailed, so reasons such asAborted by AbortReq.are preserved. Before this change, Mori's failure reason was removed early, andfailure_exception()could only reportunknown reason from another rank.failure_exception()is unified (before this change it wasNIXL KVReceiver Exceptionfor NIXL andKV transfer failedfor Mori).poll(): after metadata is published, the waiting timeout is checked in theWaitingForInputstate. Before this change, NIXL conditioned this onstarted_transferand read notifs insidepoll().bootstrap_room is Nonechecks in the Mori receiver are removed. The scheduler already rejects disaggregation requests without abootstrap_roomon intake (except for the FAKE backend, which usesFakeKVReceiver).Mooncake decode
CHUNK_READYhandlinghandle_chunk_ready(): malformed messages are dropped after logging a warning; late messages for cleared rooms are dropped; a warning is logged when the staging handler is not initialized. Before this change these cases raised an exception and triggered anassert, respectively.Wire compatibility
Accuracy Tests
test/registered/unit/disaggregation/(based on upstream0bdd0cc8dc): 637 passed, 11 skipped, 303 subtests passed.test_common_sender_lifecycle.py:add_transfer_requestargument contract of the three backends with two chunks (aux_index/state_indicesare only passed with the last chunk);Successis held while a staging chunk is deferred;test_receiver_connection_pool.py:device_kv_indices;Successfrom multiple prefill ranks;CHUNK_READY;Successconcurrent withclear()does not re-create the rank tracker.test_nixl_backend_basic.py:Successis sent only after the KV, aux, and state handles have all completed;Successis not sent when any handle fails; a deferred staging chunk still completes its transfer after the last chunk has sentSuccess;CHUNK_READYandKV_STATUS;_do_staging_transfertest now checks the returnedchunk_idx;TestNixlTransferStatus,TestNixlNotifications, and the old receiver poll and staging notif tests).test_mooncake_deferred_kv_release.pyandtest_nixl_deferred_kv_release.pydropchunk_id;test_nixl_dsv4_kv_slots.pyadds a default value for thenotifparameter;test_nixl_sender_failure_cleanup.pynow assertsKVTransferErrorand its failure reason.cuda_ipcand TCP, Mooncake over TCP; Qwen3-8B and Qwen3.6-27B): GSM8K accuracy differs from the baseline within run-to-run variation; at concurrency 1 the output tokens are bitwise identical to a non-PD deployment; the servers stay healthy with no KV leak after abort stress and failure injection. Cross-node RDMA end-to-end validation has not been completed.Speed Tests and Profiling
Setup: a single node with 8× H200, Qwen3-8B, one prefill (TP1) and one decode (TP1) instance, NIXL backend (UCX). The baseline is upstream
0bdd0cc8dc; the baseline and this PR were run alternately, restarting the servers for every run, with random-token inputs and fixed output lengths.UCX_TLS=tcp,cuda_copy): across loads with 2048–32768 input tokens and concurrency 1–128 (3 runs per version), no detectable difference in TTFT, ITL, or throughput.cuda_ipc): at concurrency 1 with 2048 input and 4 output tokens (15 runs per version), mean TTFT increases by 0.99 ms (69.59 → 70.58 ms, +1.4%) and p99 TTFT by 1.21 ms. The added time is in transfer-completion detection: decode-sidedecode_transferredincreases by about 1.1 ms and prefill-sideprefill_transfer_kv_cacheby about 0.6 ms, while prefill forward time is unchanged; the cause has not been identified. At concurrency 16–256, no detectable difference was observed.Checklist
isort,ruff format,ruff check).CI States
Latest PR Test (Base): ❌ Run #37925410328
Latest PR Test (Extra): ❌ Run #37925409795
Latest PR Test (AMD ROCm 10): ❌ Run #37925410225