Repository navigation
[Refactor] pd: Extract NixlTransport behind the common Transport interface - #46614
Open
jambow0320 wants to merge 24 commits into
Open
jambow0320 wants to merge 24 commits into
jambow0320 wants to merge 24 commits into
Conversation
Contributor
|
CC |
Contributor
Author
|
@livingshade Thanks for the heads-up. #46747 calls the NIXL agent directly in |
jambow0320
force-pushed
the
pd/nixl-transport
branch
from
October 27, 2026 14:41
1c1b94e to
1a72483
Compare
transport/base.py defines Transport (register, deregister, write, status, close, plus write_all and status_all). MooncakeTransport owns the engine calls: registration with per-region key ref-counting, uint64 address math, one blocking batch_transfer_sync per write_all, and the failed-session probe; the process-wide engine is borrowed. The Mooncake send paths pair buffer entries by index and hand the transport Copy(src key, dst key, offsets); shared PP helpers are reused unmodified by passing entry indices or zero pointers. Page groups are computed once per transfer and scaled per entry. The wire format is unchanged: prefill imports peer buffers from today's registration fields. Teardown also deregisters staging and DCP pack buffers, and Ascend registers through the transport. Co-Authored-By: GPT-6 Astra <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BufferKey declares no __slots__, so instances keep a __dict__ anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Send paths named every copied buffer with a fresh MooncakeBufferKey on every transfer, converting the kind enum each time. kv_key, aux_key and the state key builders are now cached per process, keys precompute their hash for the transport's dict lookups, and registration uses the same builders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… paths Keys are now built once per side: BufferKeys lays out a side's KV, aux and state keys as its registration message lists them. The manager builds its own at init, and each imported peer keeps its keys next to its RemoteBuffers in PeerBuffers. Send paths take src_keys/dst_keys lists parallel to their pointer lists instead of an index-to-key function, so no key is constructed on the transfer path and the process-wide key cache goes away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every key the send paths name was registered or imported together with its address, so write_all no longer re-checks each copy on every transfer; a key missing from the tables is a bug and raises KeyError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep only main's skip of an empty batch, without rebuilding the copy list each transfer. The missing-peer branch could not be reached: send paths index _peers before they build copies, and _peers is written with decode_kv_args_table under one lock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop two asserts for states the code cannot reach (a staging buffer registered twice, a PENDING Mooncake result), the uint64 cast in write_all (every producer of Copy.ranges already builds uint64), the BufferKind conversion of keys built from BufferKind values, the strict zip that main's registration never applied, the teardown resets of tables the discarded manager no longer uses, an unused logger, and the transport package re-exports nothing imports. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With indexShare and kpool compression, shared-topk layers keep a zero-row DSA tail entry so layer offsets stay aligned across peers. Such an entry is not registered and yields no blocks, but it still became an empty Copy; submitted with valid layers, write_all looked up its unregistered key and the request failed with KeyError. Build a Copy only for entries that have blocks, as the generic path already drops elided entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: GPT-6 Astra <noreply@openai.com>
The state send paths, _send_kvcache_generic and _state_stride_mismatch used their pointer lists only for counts and positions; take the key lists (or entry counts) instead and drop the parallel pointer parameters. send_kvcache_dcp builds _DcpEntryCopy and _DcpDraftSlice instead of positional six-field tuples, and moves target packing, draft slice planning and draft packing into helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Register a mix of live and zero-row DSA index-K layers through register_buffer_to_engine, then send the tail: only the live layers are written, and the unregistered layer is never looked up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Build fixtures with MooncakeKVManager.__new__ so extracted DCP planning helpers remain available, and remove redundant method forwarding. Co-Authored-By: GPT-6 Astra <noreply@openai.com>
The test from sgl-project#42973 replaced _transfer_data, which the Mooncake transport extraction removed. Attach a real MooncakeTransport over a memory-copy engine instead; the byte-level checks are unchanged.
jambow0320
force-pushed
the
pd/nixl-transport
branch
from
October 27, 2026 15:18
1a72483 to
693d7d0
Compare
…rface Route every NIXL agent call in nixl/conn.py through a NixlTransport built on the interface of RFC sgl-project#46455. NixlKVManager.init_engine() creates the agent with create_nixl_agent(). The wire format and transfer behavior are unchanged. transport/base.py and the Mooncake files are untouched. nixl/buffers.py matches mooncake/buffers.py apart from the key class name, and nixl/conn.py follows the Mooncake manager: BufferKeys built once per side, PeerBuffers, staging keys by base address, _transfer_copies, and src_keys/dst_keys on the send paths. NixlTransport adds what the NIXL engine needs beyond MooncakeTransport: memory kinds and device ids, an owned agent, loading and reloading peer metadata, asynchronous status, and prepared descriptor lists. The quiescent flag of NixlResult and the error of NixlHandle carry the failure cases nixl/conn.py already distinguishes, so a rejected post still raises _NixlPostError and failures are reported as before.
jambow0320
force-pushed
the
pd/nixl-transport
branch
from
October 28, 2026 15:56
693d7d0 to
95146ce
Compare
jambow0320
marked this pull request as ready for review
October 28, 2026 15:59
jambow0320
requested review from
ByronHsu,
Duyi-Wang,
HaiShaw,
ShangmingCai,
hnyls2002,
iforgetmyname,
ping1jing2 and
sogalin
as code owners
October 28, 2026 15:59
27 of 39 tasks
Comments that moved from nixl/conn.py into nixl/transport.py keep their original wording; the sync_mode note names NixlKVManager instead of self. Restore the _register_staging_memory docstring and the callers line of _send_kvcache_generic, and drop new docstrings that restate their code. The elided DSA entry note no longer names initialize_xfer(): a copy from an unregistered source key now fails in the transport. The SWA head-slice notes and the send_kvcache_staged docstring match the Mooncake side.
This comment has been minimized.
This comment has been minimized.
sgl-project#43299 binds the NIXL transfer worker's device through torch before the early-send wait synchronizes; the NixlTransport extraction had removed the import when the agent creation moved to nixl/transport.py.
The agent is dedicated to one KVManager, so NixlTransport is its only holder: create() makes the agent and returns the Transport over it, and init_engine() no longer keeps a reference on the manager. The creation code moves unchanged from create_nixl_agent(), which is removed.
NixlTransport now owns every prepared dlist, kept by side and layout name together with the runs it was built from. prepare() takes a layout name and write_prepared() selects both dlists by layout name and peer. reload() rebuilds the dlists prepared over the peer from their kept runs and unloads the peer again if a rebuild fails, so _reload_invalidated_peers() no longer drops and re-prepares them. This side's dlists stay valid across a reload and are not rebuilt; a state plan that could not be prepared at registration stays generic across a reload. NixlKVManager still builds the runs and computes the indices: the slice geometry and the state components stay on the manager, which no longer holds any dlist. NixlPreparedList is removed.
reload() takes the peer's dlists out of reach before NIXL loads the peer again and puts the rebuilt ones back, as the manager did before the dlists moved into NixlTransport: a dlist prepared before the reload is invalid once the peer is loaded again. A failed load or rebuild leaves the peer dropped and keeps its dlists, whose runs a later reload rebuilds from. Unlike before the move, a state dlist that cannot be rebuilt leaves the whole peer dropped until a later reload succeeds, instead of falling back to generic state transfers.
The second, successful reload repeated what test_failed_rebuild_leaves_the_peer_to_a_later_reload checks. The test now covers only what no other case does: the peer's dlists are unreachable while NIXL loads it, and a failed load keeps them.
NixlKVManager kept _prepared_src_layouts and _kv_layout_peers, two sets that copied which dlists NixlTransport held. It now asks the transport through is_prepared() and keeps only the geometry it computes indices from; send_kvcache() takes the prepared path when both sides' KV dlists are prepared, as before the dlists moved into NixlTransport. When the peer's state dlist fails to prepare, _build_prepared_state_plan() discards this side's one through discard_prepared(), as the manager dropped the unused source handle before the move. The plan is still None and state transfers stay generic.
They narrowed src_mem_kind for the NIXL calls that took it as a memory kind. Since the transport derives memory kinds from registrations, neither _prepare_payload_xfer() nor send_kvcache() reads it, and both run only with KV buffers registered, where src_mem_kind is always set.
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
Step 2 of #34510 for NIXL: move the NIXL engine calls behind a
NixlTransporton the interface proposed in #46455.Stacked on #46460 (MooncakeTransport);
Merge order: #46460 should be merged first. This PR is then rebased onto main.
Modifications
nixl/transport.py:NixlTransportowns the NIXL agent and makes every engine call: memory registration, peer import and reload, writes, prepared descriptor lists and transfer status. It also keeps every prepared descriptor list;nixl/conn.pykeeps only the geometry it computes indices from, and no longer calls the agent directly or holds a list._NixlPostError, and the failure messages are kept. Two failure paths of the peer reload differ; see Notes.nixl/buffers.pymatchesmooncake/buffers.pyapart from the key class name, andnixl/conn.pyfollowsMooncakeKVManager(init_engine(),BufferKeysbuilt once per side,PeerBuffers, staging keys by base address,_transfer_copies(),src_keys/dst_keyson the send paths).transport/base.pyand the Mooncake files are untouched. NIXL-only types stay undernixl/; shared types are extracted once every backend has a Transport.What
NixlTransportadds overMooncakeTransportRegistration, the per-region registration bookkeeping,write(),write_all(),status()andclose()have the same shape as inMooncakeTransport. The rest follows from how the NIXL engine differs from Mooncake's. Apart from the interface methods, each item is engine handling thatnixl/conn.pyalready had before this PR and that moved into the Transport.NixlTransportNixlTransport.create()create()makes the agent and returns the Transport that holds it; the KVManager keeps no reference. Moved fromNixlKVManager.__init__;init_engine()calls it.agent_name,agent_metadatasession_idis for Mooncake.import_registration(agent_name, metadata, bases, devices)add_remote_agent) before any transfer, and peer descriptors need each buffer's memory kind and device id. Mooncake only needs the session id and base addresses.is_loaded(),reload(),unload()reload()rebuilds those lists from their kept runs and keeps them out of reach until rebuilt. Mooncake uses a session blacklist andsend_probe()instead.register_all(),deregister()andwrite_all()VRAMorDRAM), and each descriptor carries a device id. Registrations are therefore grouped by kind, and onewrite_all()becomes one NIXL transfer per (source kind, destination kind). Mooncake registers and transfers raw addresses in a single batch.prepare(),write_prepared(),StridedRun,supports_strided_runswrite_prepared()selects both lists by name;nixl/conn.pystill builds the runs and computes the indices. NIXL without strided-run support needs one descriptor per block, soprepare()keeps that fallback; state lists, which can hold millions of slots, are never expanded (allow_expansion=False).status()polls and caches the final resultcheck_xfer_state. Mooncake'swrite_all()blocks, so its handle already holds the final result.NixlResult.quiescent,NixlHandle.errornixl/conn.pyhandles a transfer that NIXL never created or reported asERRdifferently from a rejected post or an unreadable state, and builds its failure messages from the NIXL exception. These two fields carry that information across the Transport boundary. Mooncake cannot make this distinction: a timed-outbatch_transfer_syncmay still be accessing memory.close()also drops the prepared lists and unloads peersNotes
MooncakeTransportdoes andderegister()intransport/base.pyrequires.nixl/conn.pystill leaves zero-pointer state tensors unregistered, as before.check_xfer_state()does not return"ERR", and a failed transfer takes the unreadable-state path, as it does on main.nixl/conn.pyused to drop a reloaded peer's lists and rerun the registration-time preparation;reload()now rebuilds the lists prepared over the peer from their kept runs, with the same descriptors. This side's lists stay valid across a peer reload and are kept. Two failure paths differ from main:deregister()andclose()implement the abstract interface;nixl/conn.pyhas no teardown and does not call them yet.Tests
test/registered/unit/disaggregation/passes on CPU (407 passed, 17 skipped). Not yet run with a real NIXL backend.CI States
Latest PR Test (Base): ❌ Run #41037302856
Latest PR Test (Extra): ❌ Run #41037302734
Latest PR Test (AMD ROCm 10): ❌ Run #41037302824