[TRTLLM-12499][feat] Pipelined KVCache transfer for disaggregated serving in Python Cache Transceiver - #15727
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds sender-side pipelined KV transfer for disaggregated serving. The change introduces block projection, separate sender and receiver identifiers, session retirement, configuration validation, executor integration, and unit and integration coverage. ChangesPipelined KV transfer
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PyExecutor
participant KvCacheTransceiverV2
participant TxSession
participant RxSession
PyExecutor->>KvCacheTransceiverV2: send prefill chunk
KvCacheTransceiverV2->>TxSession: send projected KVSlice
TxSession->>RxSession: deliver KV result with sender and receiver IDs
RxSession-->>PyExecutor: resolve receiver task and report completion
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tensorrt_llm/_torch/disaggregation/native/transfer.py (1)
488-507: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMark the task as in-flight before waiting on the CUDA event.
Line 492 waits while the task is still
INIT, socancel_request()can see noTRANSFERRINGtasks and free KV pages before the event completes. Move the INIT→TRANSFERRING transition before the event wait, and keep the cancelled/error abort path before synchronization.Suggested fix
- # For pipelined prefill-transfer: wait for the GPU forward - # to finish writing KV data before starting RDMA. This - # blocks only this worker thread, not the GPU or main thread. - if task._slice.cuda_event is not None: # TODO: should I sync after the task status is set to TRANSFERRING? - task._slice.cuda_event.synchronize() - - if timer: - timer.record_push_end(write_meta.peer_rank) # Hold session.lock to serialize the INIT→TRANSFERRING transition with # cancel(): prevents cancel_request() from freeing KV pages while a # worker is about to write into them. with session.lock: status = session.status if status in (SessionStatus.ERROR, SessionStatus.CANCELLED): should_abort = True else: task.status = TaskStatus.TRANSFERRING should_abort = False + + if should_abort: + ... + return + + # For pipelined prefill-transfer: wait for the GPU forward + # to finish writing KV data before starting RDMA. + if task._slice.cuda_event is not None: + task._slice.cuda_event.synchronize() + + if timer: + timer.record_push_end(write_meta.peer_rank)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/disaggregation/native/transfer.py` around lines 488 - 507, The task transition in transfer.py is happening too late in the prefill-transfer flow: `task._slice.cuda_event.synchronize()` runs while the task is still `INIT`, so `cancel_request()` can miss it and free KV pages too early. In the transfer path around `task`, `session.lock`, and `TaskStatus.TRANSFERRING`, move the INIT→TRANSFERRING state update (with the session ERROR/CANCELLED abort check) before waiting on the CUDA event, and keep the abort branch ahead of synchronization so in-flight work is visible before any blocking wait.
🧹 Nitpick comments (2)
tensorrt_llm/_torch/disaggregation/transceiver.py (2)
582-585: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant session assignment.
_get_or_create_send_sessionalready inserts the session intoself._send_sessions, so re-assigning the return value is redundant (and could mask a future divergence between the two code paths). Mirror the simpler form used inrespond_and_send_async.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/disaggregation/transceiver.py` around lines 582 - 585, The send-session initialization in transceiver logic has a redundant assignment because _get_or_create_send_session already stores the session in self._send_sessions. Update the rid-not-in-self._send_sessions branch in transceiver.py to follow the same pattern as respond_and_send_async by simply invoking _get_or_create_send_session(req) for its side effects, then keep setting _ever_had_send_session and _pipelined_chunk_offsets[rid] as before.
602-604: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNumerous open TODOs in the pipelined-send path before merge.
send_prefill_chunkandrespond_and_send_asynccarry several unresolvedTODO(athenac)questions on correctness-critical fields (token_range,mamba_state_index,layer_range, thereq.statetransition, the offset accumulation "might be a faulty calculation", and the redundancy between the two methods). Since the PR is marked WIP, these need resolution before this is production-ready. I can help draft the offset/metadata handling and consolidate the shared logic into a single helper.Also applies to: 608-611, 628-631, 657-666, 675-675
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/_torch/disaggregation/transceiver.py` around lines 602 - 604, The pipelined-send path in transceiver.py still contains unresolved correctness TODOs in send_prefill_chunk and respond_and_send_async, especially around token_range, mamba_state_index, layer_range, req.state transitions, and the offset accumulation logic. Resolve these TODO(athenac) questions by verifying the metadata semantics, fixing the offset calculation, and making the state update explicit and correct before merge. Also remove the duplicated logic between send_prefill_chunk and respond_and_send_async by consolidating the shared send/metadata assembly into a single helper so the two paths stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/disaggregation/native/transfer.py`:
- Around line 728-745: The chunked destination slicing in transfer logic is now
using chunk offsets, but the token alignment still assumes each chunk maps to a
suffix ending at token_range.end. Update the code around the chunked path in
transfer.py and the downstream token-start calculation to derive starts from
chunk_block_offset, or require callers to provide per-chunk KVSlice.token_range
for each chunk. Make sure the block selection and token-range alignment stay
consistent for prefix-cache and SWA cases so the written blocks match the
intended chunk.
- Around line 523-524: The abort/result notification path in transfer.py still
uses write_meta.slice_id, which can conflict with the receiver’s single-task
slice handling. Update the abort send logic in the relevant transfer routine to
mirror the success path by reporting receiver_slice_id as 0 for aborts too, so
the receiver does not see a later-chunk slice ID and hit its slice assertion.
Keep the existing task/event unblocking behavior intact while ensuring the
aborted/failure result is always sent to receiver slice 0.
- Around line 736-743: The chunk-to-destination mapping in transfer.py is too
strict for exhausted layer groups: when len(src_block_ids) is 0, the current
bounds check in the chunk slicing logic still raises on advanced chunk_offset
values. Update the chunk handling around the dst_block_ids slice so empty source
chunks become a no-op and do not trigger the out-of-bounds error; keep the
existing bounds validation for non-empty chunks in the same chunk
offset/full_dst_block_ids path.
In `@tensorrt_llm/_torch/disaggregation/transceiver.py`:
- Line 648: The `respond_and_send_async` skip guard in `transceiver.py` needs
both a lint fix and a logic check: move the `return` onto its own line to
satisfy E701, and verify the condition around `rid in self._send_sessions and
rid not in self._pipelined_chunk_offsets` correctly prevents duplicate sends
while pipelined chunks are still outstanding. If needed, adjust the guard so the
full `_create_kv_slices` resend path only runs when it is safe, using the
existing `_send_sessions` and `_pipelined_chunk_offsets` state to avoid
duplicate transfer.
- Around line 591-611: The chunking logic in send_prefill_chunk() and the
_pipelined_chunk_offsets update can split KV slices on token boundaries that are
not aligned to tokens_per_block, which causes the boundary block to be resent
and offsets to drift. Adjust the prefill chunk selection so every chunk boundary
lands on a KV block boundary (or clamp the sliding-window fallback so it only
overlaps when it evenly divides tokens_per_block), and then recompute
_pipelined_chunk_offsets from the actual block count in the chunk.
In `@tests/unittest/disaggregated/test_kv_transfer.py`:
- Around line 1789-1825: The send/receive flow is using the wrong API shape:
TxSession.send() and RxSession.receive() should be called with a fully populated
KVSlice rather than extra kwargs, and they do not return futures. Update the
test setup around KVSlice, sender_session.send(), and
receiver_sessions/RxSession.receive() to set chunk_block_offset and cuda_event
on the slice object before calling send/receive, then replace the .result()
waits with wait_complete()/wait_complete(blocking=True) on the session or slice
as appropriate.
---
Outside diff comments:
In `@tensorrt_llm/_torch/disaggregation/native/transfer.py`:
- Around line 488-507: The task transition in transfer.py is happening too late
in the prefill-transfer flow: `task._slice.cuda_event.synchronize()` runs while
the task is still `INIT`, so `cancel_request()` can miss it and free KV pages
too early. In the transfer path around `task`, `session.lock`, and
`TaskStatus.TRANSFERRING`, move the INIT→TRANSFERRING state update (with the
session ERROR/CANCELLED abort check) before waiting on the CUDA event, and keep
the abort branch ahead of synchronization so in-flight work is visible before
any blocking wait.
---
Nitpick comments:
In `@tensorrt_llm/_torch/disaggregation/transceiver.py`:
- Around line 582-585: The send-session initialization in transceiver logic has
a redundant assignment because _get_or_create_send_session already stores the
session in self._send_sessions. Update the rid-not-in-self._send_sessions branch
in transceiver.py to follow the same pattern as respond_and_send_async by simply
invoking _get_or_create_send_session(req) for its side effects, then keep
setting _ever_had_send_session and _pipelined_chunk_offsets[rid] as before.
- Around line 602-604: The pipelined-send path in transceiver.py still contains
unresolved correctness TODOs in send_prefill_chunk and respond_and_send_async,
especially around token_range, mamba_state_index, layer_range, req.state
transitions, and the offset accumulation logic. Resolve these TODO(athenac)
questions by verifying the metadata semantics, fixing the offset calculation,
and making the state update explicit and correct before merge. Also remove the
duplicated logic between send_prefill_chunk and respond_and_send_async by
consolidating the shared send/metadata assembly into a single helper so the two
paths stay consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 784a62c4-609a-4545-bc83-7d70311b9078
📒 Files selected for processing (11)
tensorrt_llm/_torch/disaggregation/base/transfer.pytensorrt_llm/_torch/disaggregation/native/transfer.pytensorrt_llm/_torch/disaggregation/transceiver.pytensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.pytensorrt_llm/_torch/pyexecutor/py_executor.pytensorrt_llm/llmapi/llm_args.pytests/integration/defs/accuracy/test_disaggregated_serving.pytests/integration/test_lists/test-db/l0_dgx_b200.ymltests/unittest/disaggregated/test_chunked_transfer.pytests/unittest/disaggregated/test_kv_transfer.pytests/unittest/llmapi/test_llm_args.py
|
@coderabbitai check the test coverage, including function test, perf test, stress test and multi nodes test. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Changed/mentioned test files =="
for f in \
tests/unittest/disaggregated/test_chunked_transfer.py \
tests/unittest/disaggregated/test_kv_transfer.py \
tests/unittest/llmapi/test_llm_args.py \
tests/integration/defs/accuracy/test_disaggregated_serving.py \
tests/integration/test_lists/test-db/l0_dgx_b200.yml \
tensorrt_llm/_torch/disaggregation/transceiver.py \
tensorrt_llm/_torch/pyexecutor/py_executor.py \
tensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.py
do
if [ -f "$f" ]; then
echo "-- outline: $f"
ast-grep outline "$f" --view expanded || true
echo
else
echo "Missing: $f"
fi
done
echo "== Search for perf/stress/multi-node/disaggregated related tests =="
rg -n -i -C2 \
'(perf|performance|benchmark|stress|soak|multi[-_ ]node|multinode|disaggregated serving|disaggregated|chunked|pipelined)' \
tests tensorrt_llm/_torch | sed -n '1,260p'
echo
echo "== Inspect relevant slices from changed tests =="
sed -n '1,260p' tests/unittest/disaggregated/test_chunked_transfer.py
echo "-----"
sed -n '1,260p' tests/unittest/disaggregated/test_kv_transfer.py
echo "-----"
sed -n '1,220p' tests/integration/defs/accuracy/test_disaggregated_serving.py
echo "-----"
sed -n '1,220p' tests/unittest/llmapi/test_llm_args.py
echo "-----"
sed -n '1,220p' tests/integration/test_lists/test-db/l0_dgx_b200.yml
echo
echo "== Inspect relevant implementation slices for testability points =="
sed -n '1,260p' tensorrt_llm/_torch/disaggregation/transceiver.py
echo "-----"
sed -n '1,220p' tensorrt_llm/_torch/pyexecutor/py_executor.py
echo "-----"
sed -n '1,220p' tensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.pyLength of output: 144072 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Direct test references for new pipelined/chunked entry points =="
rg -n -C2 \
'_maybe_send_prefill_chunk|send_prefill_chunk|enable_pipelined_transfer|respond_and_send_async|request_and_receive_async|_create_kv_slices|create_kv_cache_transceiver' \
tests tensorrt_llm/_torch | sed -n '1,260p'
echo
echo "== Disaggregated multi-node / stress / soak style tests in test tree =="
rg -n -i -C2 \
'(multi[-_ ]node|multinode|soak|stress|long[-_ ]run|longevity|burn[-_ ]in)' \
tests | sed -n '1,220p'
echo
echo "== Show pipelined accuracy test and nearby YAML entries =="
sed -n '720,820p' tests/integration/defs/accuracy/test_disaggregated_serving.py
echo "-----"
rg -n -C2 'chunked_kv_transfer|pipelined_kv_transfer' tests/integration/test_lists/test-db/l0_dgx_b200.ymlLength of output: 49120
What is covered wellFunction / unit tests
Integration / accuracy
Gaps I found1. Some new entry points are not directly tested
So the core mechanics are exercised indirectly, but several newly added integration points are still missing targeted tests. 2. Pipelined accuracy test does not appear to be scheduled in the shown pre-merge list
So pipelined accuracy exists in code, but it does not look wired into this pre-merge list yet. 3. No PR-specific perf validation
Given the PR objective is explicitly performance-oriented, this is a notable gap. 4. No PR-specific stress coverage
But I did not find stress coverage specifically targeting:
5. No PR-specific multi-node coverage
But I did not find multi-node tests specifically exercising this PR’s new chunked/pipelined KV transfer path. Overall assessment
Recommended follow-ups
If you want, I can turn this into a concrete review comment with prioritized test additions. |
7705b98 to
37f1bdd
Compare
37f1bdd to
7173ea6
Compare
d3de936 to
aa98b71
Compare
|
/bot run |
661dee6 to
7c87a01
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tests/unittest/disaggregated/test_chunked_transfer.py (2)
657-666: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
_chunk_size_blocksstub attribute.
KvCacheTransceiverV2.pipeline_transfer_enabledreads only_enable_pipelined_transfer. The production class defines no_chunk_size_blocksattribute. Setting it here suggests a dependency that does not exist.♻️ Proposed cleanup
transceiver = MagicMock() transceiver._enable_pipelined_transfer = False - transceiver._chunk_size_blocks = 64 result = KvCacheTransceiverV2.pipeline_transfer_enabled.fget(transceiver)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/disaggregated/test_chunked_transfer.py` around lines 657 - 666, Remove the unused transceiver._chunk_size_blocks = 64 stub assignment from test_pipelined_transfer_disabled_by_default; keep the test focused on the _enable_pipelined_transfer value read by KvCacheTransceiverV2.pipeline_transfer_enabled.
15-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the remaining pipelined-transfer coverage.
Coverage: 55 test functions cover projection, slice IDs, session lifecycle, cancellation, retirement, and configuration. The accuracy tests are registered in
tests/integration/test_lists/test-db/l0_dgx_b200.yml, and unit tests are collected by directory.Add tests for:
_close_failed_sessions(..., mark_retired=True)whenridis absent from_send_reqs.- Non-block-aligned chunk ends, asserting successive destination writes do not overlap.
- Multiple chunks through a real
respond_and_send_asyncandTxSession; current tests mock the session and chunk builder.Run
pytest tests/unittest/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/disaggregated/test_chunked_transfer.py` around lines 15 - 51, Add unit tests in the existing chunked-transfer test module covering three gaps: verify _close_failed_sessions with mark_retired=True retires sessions whose rid is absent from _send_reqs; exercise non-block-aligned chunk ends and assert successive destination writes do not overlap; and drive multiple chunks through the real respond_and_send_async and TxSession paths without mocking the session or chunk builder. Follow the existing fixtures and assertions, then run pytest tests/unittest/.Sources: Coding guidelines, Path instructions
tests/unittest/disaggregated/test_kv_transfer.py (2)
1996-2001: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet
token_rangeon the receiver slice for parity with the chunked helper.
add_and_verify_chunked_requestposts the receive slice withtoken_range=TokenRange(start=0, end=request_len)(Line 1774). The pipelined helper omitstoken_range. The docstring states the receiver posts one slice covering the whole prompt, so make the range explicit. This keeps the two helpers comparable and removes reliance on the defaultNone.♻️ Proposed change
for recv_session, block_ids_per_groups in zip(receiver_sessions, gen_block_ids): full_slice = KVSlice( is_last_slice=True, block_ids_per_layer_groups=block_ids_per_groups, + token_range=TokenRange(start=0, end=request_len), ) recv_session.receive(full_slice)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/disaggregated/test_kv_transfer.py` around lines 1996 - 2001, Update the receiver-side KVSlice construction in the pipelined helper to set token_range explicitly to TokenRange(start=0, end=request_len), matching add_and_verify_chunked_request and covering the full prompt while leaving the existing final-slice and block-group fields unchanged.
1962-1995: 📐 Maintainability & Code Quality | 🔵 TrivialTest coverage summary
Added test functions in
tests/unittest/disaggregated/test_kv_transfer.py:
test_build_prefill_chunk_projects_incremental_source_against_full_prompttest_build_prefill_chunk_normalizes_swa_source_to_computed_prefix(2 cases)test_send_prefill_chunks_basic(5 cases)test_send_prefill_chunks_integrity_check(3 cases)test_send_prefill_chunks_multiple_layer_groupstest_send_prefill_chunks_preserves_mamba_state_indextest_send_prefill_chunks_none_mamba_state_indextest_transfer_worker_chunked(V1 and V2)test_transfer_worker_pipelined(V1 and V2)test_transfer_worker_pipelined_ctx_prefix_reuse(V1 and V2)Modified test functions:
test_transfer_with_gen_prefix_offset: newchunk_size_blocksparameter withsingle_sliceandsender_chunkedcases.test_session_cancel_after_send: now assertswait_complete() == WaitResult.FAILEDinstead of an exception.New helpers:
_send_prefill_chunks,_setup_chunked_request,_verify_and_cleanup_chunked,add_and_verify_chunked_request,add_and_verify_pipelined_request.Test list status: the new tests live in an existing module in
tests/unittest/disaggregated/, so they run with the existing unit-test job for that directory. No entry was added undertests/integration/test_lists/test-db/ortests/integration/test_lists/qa/for these unit tests, which matches the existing pattern for this module.Covered paths: chunk projection against a growing source, SWA normalization, chunk counts including empty and oversized chunk sizes, block integrity with a context-side reuse prefix, layer-group suffix handling,
mamba_state_indexpropagation, cancellation state, and end-to-end chunked and pipelined transfer for V1 and V2.Gaps: no test drives
send_prefill_chunkor_maybe_send_prefill_chunkon the executor path; no test covers a mid-transfer failure or timeout during pipelined sending; no multi-rank (TP or PP > 1) pipelined configuration;PIPELINED_TEST_CONFIGSandCHUNKED_TEST_CONFIGSboth cover only tp1/pp1.Verdict: needs follow-up. The unit-level chunk projection coverage is strong, but the executor entry points and multi-rank pipelined paths are untested. Do you want me to open an issue to track the executor-path and multi-rank pipelined tests?
As per path instructions: "Always produce a test coverage summary, even if no issues are found."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unittest/disaggregated/test_kv_transfer.py` around lines 1962 - 1995, Extend test coverage for the missing executor paths and multi-rank pipelined transfers. Add tests that invoke send_prefill_chunk and _maybe_send_prefill_chunk through the executor, including failure or timeout behavior during pipelined sending, and add TP/PP > 1 cases to PIPELINED_TEST_CONFIGS or the corresponding pipelined test setup while preserving existing V1/V2 coverage.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tensorrt_llm/_torch/disaggregation/transceiver.py`:
- Around line 732-741: The chunk construction in _build_prefill_chunk must
ensure every non-final chunk ends on a tokens-per-block boundary; assert that
non-final chunk_end_pos is divisible by tpb, or clamp chunk_end_block down for
non-final chunks to prevent overlapping destination blocks. In
tensorrt_llm/_torch/disaggregation/base/transfer.py lines 58-74, verify
derive_chunk_block_coords is only called with block-aligned pipelined chunks and
document the producer alignment precondition; no other behavior should change.
- Around line 601-610: Update _close_failed_sessions to tolerate failed session
IDs that have no corresponding entry in reqs, matching the asymmetry handling in
_retire_send_session. Only update request state, retirement flags, and delete
the request when reqs contains rid; always close and remove the associated
session so reconciliation continues without KeyError or leaked sessions.
---
Nitpick comments:
In `@tests/unittest/disaggregated/test_chunked_transfer.py`:
- Around line 657-666: Remove the unused transceiver._chunk_size_blocks = 64
stub assignment from test_pipelined_transfer_disabled_by_default; keep the test
focused on the _enable_pipelined_transfer value read by
KvCacheTransceiverV2.pipeline_transfer_enabled.
- Around line 15-51: Add unit tests in the existing chunked-transfer test module
covering three gaps: verify _close_failed_sessions with mark_retired=True
retires sessions whose rid is absent from _send_reqs; exercise non-block-aligned
chunk ends and assert successive destination writes do not overlap; and drive
multiple chunks through the real respond_and_send_async and TxSession paths
without mocking the session or chunk builder. Follow the existing fixtures and
assertions, then run pytest tests/unittest/.
In `@tests/unittest/disaggregated/test_kv_transfer.py`:
- Around line 1996-2001: Update the receiver-side KVSlice construction in the
pipelined helper to set token_range explicitly to TokenRange(start=0,
end=request_len), matching add_and_verify_chunked_request and covering the full
prompt while leaving the existing final-slice and block-group fields unchanged.
- Around line 1962-1995: Extend test coverage for the missing executor paths and
multi-rank pipelined transfers. Add tests that invoke send_prefill_chunk and
_maybe_send_prefill_chunk through the executor, including failure or timeout
behavior during pipelined sending, and add TP/PP > 1 cases to
PIPELINED_TEST_CONFIGS or the corresponding pipelined test setup while
preserving existing V1/V2 coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dc203880-13e7-4d84-bb84-3e48776a9ec6
📒 Files selected for processing (22)
tensorrt_llm/_torch/auto_deploy/shim/ad_executor.pytensorrt_llm/_torch/disaggregation/base/transfer.pytensorrt_llm/_torch/disaggregation/native/transfer.pytensorrt_llm/_torch/disaggregation/transceiver.pytensorrt_llm/_torch/pyexecutor/_util.pytensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.pytensorrt_llm/_torch/pyexecutor/llm_request.pytensorrt_llm/_torch/pyexecutor/py_executor.pytensorrt_llm/_torch/pyexecutor/py_executor_creator.pytensorrt_llm/commands/serve.pytensorrt_llm/llmapi/disagg_utils.pytensorrt_llm/llmapi/llm_args.pytensorrt_llm/usage/llm_args_golden_manifest.jsontests/integration/defs/accuracy/test_disaggregated_serving.pytests/integration/test_lists/test-db/l0_dgx_b200.ymltests/unittest/_torch/executor/test_disagg_index_mapper_early_release.pytests/unittest/_torch/executor/test_disagg_inflight_cancel_gate.pytests/unittest/disaggregated/test_bounce.pytests/unittest/disaggregated/test_chunked_transfer.pytests/unittest/disaggregated/test_disagg_utils.pytests/unittest/disaggregated/test_kv_transfer.pytests/unittest/disaggregated/test_transceiver_bounded_polling.py
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/unittest/_torch/executor/test_disagg_inflight_cancel_gate.py
- tests/unittest/disaggregated/test_transceiver_bounded_polling.py
- tensorrt_llm/usage/llm_args_golden_manifest.json
- tensorrt_llm/_torch/pyexecutor/_util.py
- tensorrt_llm/_torch/auto_deploy/shim/ad_executor.py
- tensorrt_llm/_torch/pyexecutor/llm_request.py
- tests/unittest/_torch/executor/test_disagg_index_mapper_early_release.py
- tensorrt_llm/_torch/pyexecutor/py_executor_creator.py
- tests/unittest/disaggregated/test_disagg_utils.py
- tests/integration/defs/accuracy/test_disaggregated_serving.py
- tests/unittest/disaggregated/test_bounce.py
- tensorrt_llm/commands/serve.py
- tensorrt_llm/_torch/pyexecutor/kv_cache_transceiver.py
- tensorrt_llm/_torch/pyexecutor/py_executor.py
- tensorrt_llm/llmapi/llm_args.py
- tensorrt_llm/_torch/disaggregation/native/transfer.py
- tests/integration/test_lists/test-db/l0_dgx_b200.yml
|
/bot run --disable-fail-fast |
|
PR_Github #65352 [ run ] triggered by Bot. Commit: |
|
PR_Github #65352 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
2dd90a0 to
4450d48
Compare
…ving in Python Cache Transceiver Instead of waiting for all prefill chunks to complete before starting KV cache transfer, each chunk's KV data is transferred to the generation server immediately after its prefill completes. This overlaps GPU compute with RDMA transfer, hiding transfer latency behind prefill computation. Only the last chunk's transfer remains on the critical path. The feature is gated behind `enable_pipelined_transfer` on `CacheTransceiverConfig` and is implemented in `KvCacheTransceiverV2` only. It requires `schedule_style: generation_first`, `enable_chunked_prefill: true`, `beam_width == 1`, the NIXL backend, `kv_cache_bounce_size_mb == 0`, `pipeline_parallel_size == 1` on the sender, and a non-Mamba/hybrid cache manager. Each requirement is enforced at startup or per request. Squashed from 15 commits: - Chunking is sender-side only; the generation server posts a single receive covering the whole prompt and completes on `is_last_slice`. - `KVSlice` now describes one chunk rather than one whole request, gaining `total_blocks` and a meaningful `is_last_slice`. `prompt_len` became required on the session args so SWA can compute the stale-block boundary. - `project_blocks_to_global_chunk` intersects ranges instead of indexing, so resident-suffix block lists (sliding window groups, prefix reuse, incremental allocation) project correctly onto a global chunk. - The first slice always extends back to block 0, so a context-side prefix-reuse hit does not leave `[0, prepopulated_prompt_len)` unsent. - Source blocks are capped at the computed chunk boundary before SWA trimming, normalizing V1's full-prompt reservation against V2's incremental allocation. - `KV_AGENT_RESULT` carries `sender_slice_id` and `receiver_slice_id` separately, making per-chunk RDMA failures attributable. Behavior-neutral for the monolithic receiver. - KV transfer activity is modeled by transceiver session membership rather than `LlmRequestState`, so mid-prefill cancellation and transfer-timeout monitoring work during the pipelined phase. - A retired send session cannot be silently re-created, since closing it drops the peer's `RecvReqInfo` and the receiver never re-registers. - `TxSession.dispatch_lock` serializes chunk dispatch across the executor thread and the late-peer replay path, so a newer slice cannot reach a peer's queue ahead of an older one. - Transceiver configuration resolution happens early and idempotently, and backend/runtime compatibility validation is centralized. Signed-off-by: Athena Cai <athenac@nvidia.com> Simpilify _build_kv_write_meta logic Signed-off-by: Athena Cai <athenac@nvidia.com>
4450d48 to
1f88ba1
Compare
Shixiaowei02
left a comment
There was a problem hiding this comment.
Thanks for the effort.
A scope note, not a correctness one. About 350 of the ~1000 changed source lines are the feature. Most of the rest is refactoring that would be easier to review, and to revert, on its own. Three things worth doing before the next round.
The chunked-prefill check reads the wrong value. It reads the declared setting. The executor factory later turns chunked context off for star attention and for MLA on unsupported SMs, and that later value is what the scheduler uses. So on those models the check passes, chunking is off, and the pipelined path runs over one degenerate chunk. Moving the check to where the effective value is decided fixes it, and drops the new parameter, the call-site reflow, and the AutoDeploy hunk.
The result frame does not need a new field. The real problem is just that the sender puts its own task index on the wire, where the receiver expects its own. The receiver posts exactly one task, so send the peer's index in the existing field and keep the sender's index local for logging. That avoids widening a binary contract with no version negotiation, and stops an unrelated KV-bounce test from being rewritten.
The new any-inflight helper looks unnecessary. The timeout sweep already runs once per iteration from the scheduling path, so two of the four sites are redundant. The other two are on the pipeline-parallel loop, which this feature rejects on the sending side. It does change flag-off behaviour, and that looks like a latent fix for pipeline parallelism — worth landing with its own test. Note the per-request variant is a different method, and the only thing keeping pages from being freed under an active write. That one should stay.
Three smaller ones: the YAML validation is not the enforcement point — the per-request check covers every deployment, and the YAML one never sees the config that takes effect; keeping the slice's existing extent field would also stop an untouched pre-merge test from failing to build a session; and the lock change is ~90 lines of pure re-indentation, easier to read as its own commit.
| @@ -1 +1 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
This file carries more than the feature needs. Could we only make these changes?
@dataclass
class ChunkCoords:
"""Position of one pipelined chunk in a request's global block space."""
block_offset: int
block_count: int
def __post_init__(self):
if self.block_offset < 0 or self.block_count < 0:
raise ValueError(f"Invalid chunk: offset={self.block_offset}, count={self.block_count}")
...
class KVSlice:
...
chunk: Optional[ChunkCoords] = None
There was a problem hiding this comment.
What do you mean by this? Are you saying I should move the project_blocks_to_global_chunk helper to another file? Or do you mean I should reduce the comments?
There was a problem hiding this comment.
The _has_any_inflight_kv_transfer helper is meant to track transfer state of the request independently of it's prefill compute state, since DISAGG_CONTEXT_TRANS_IN_PROGRESS is not set until the last chunk transfer is scheduled.
I can reduce the usage of this helper though.
| f"kv_transfer_timeout_ms={timeout_ms}ms") | ||
| req.py_kv_transfer_timed_out = True | ||
|
|
||
| # Context requests start their clock on the last chunk, which is also when |
There was a problem hiding this comment.
The configured timeout is not monitored until the final chunk is submitted, leaving the earlier chunks transfer uncovered, e.g., taking longer or hanging indefinitely.
We should monitor the request for every chunk transfer and define how a stalled NIXL operation is drained or terminated before its KV memory can be reused. For example, add a test that blocks the first chunk and verifies that the request times out safely before the final chunk.
There was a problem hiding this comment.
UIUC, the last chunk is blocked behind the earlier chunks so the transfer timeout of the last chunk would catch any previous chunks which are hanging.
I will file a ticket for this for now.
There was a problem hiding this comment.
I think it is fine to only monitor timeout for the final chunk transfer, which transitively detects earlier chunk transfer hang, under FIFO sequence.
The deeper concern, however, is once a timeout is detected for the last chunk transfer, how do we correctly mark the corresponding transfer (not just the final one but potentially some of the earlier ones) terminal. Today, cancel_request() marks the session CANCELLED but returns false while the earlier task is still TRANSFERRING. On the next status sweep, the cancelled session is retired without rechecking whether that task has physically drained; a subsequent retry can then release the request’s KV ownership.
PS: I am flagging this as a potential transfer lifecycle/ownership risk; addressing this completely may go beyond the scope of this PR.
| if schedule_style: | ||
| disagg_cfg.schedule_style = schedule_style | ||
| disagg_cfg = parse_disagg_config_file( | ||
| config_file, schedule_style_override=schedule_style) |
There was a problem hiding this comment.
The --schedule_style override works in the launcher, but fleet and MPI workers reload the original YAML and validate it before applying the override. For example, a YAML file containing context_first and pipelining can be launched with --schedule_style generation_first; the launcher accepts it, but its workers read context_first again and fail during startup. Please pass the resolved schedule style to every worker before parsing and validation, and add a test for this configuration.
Documentation (WIP) https://docs.google.com/document/d/1Z9ARCc48QNCbKfoTEhZT1W4W440x6QsbKfVh3ksKIW0/edit?tab=t.0
Status: In review
Follow up PR to remove redundant KVCM work: #17526
Description
Summary
Implements pipelined prefill-transfer for disaggregated serving. Instead of waiting for all prefill chunks to complete before starting KV cache transfer, each chunk's KV data is transferred to the generation server immediately after its prefill completes. This overlaps GPU compute with RDMA transfer, hiding transfer latency behind prefill computation. When transfer time per chunk is less than prefill time per chunk (typical for 100+ Gbps NIC with long-context workloads), transfer latency is nearly fully hidden.
Only the last chunk's transfer remains on the critical path. Everything before it is paid for out of prefill compute time.
Related work by @chienchunhung for chunked KV cache transfer: Reducing KV Block Residency and Peak Memory Pressure in Disaggregated Serving
Configuration
The feature is gated behind
enable_pipelined_transferonCacheTransceiverConfig(tensorrt_llm/llmapi/llm_args.py). The flag is consumed by the Python transceiver only and has no C++ counterpart (_to_pybinddoes not forward it).Requirements and where they are enforced
schedule_style: generation_first_validate_disagg_configintensorrt_llm/llmapi/disagg_utils.py(startup, from the disagg YAML) andPyExecutor._validate_request(per request)enable_chunked_prefill: truereq.py_last_context_chunk); there is no separate chunk-size knobcreate_kv_cache_transceiver(ValueError)beam_width == 1PyExecutor._validate_request(ValueError), asserted again in_build_prefill_chunkKvCacheTransceiverV2resolve_cache_transceiver_configkv_cache_bounce_size_mb == 0resolve_cache_transceiver_config(ValueError)pipeline_parallel_size == 1on the sendercreate_kv_cache_transceiver(ValueError); the generation server is unaffected and may still use PPcreate_kv_cache_transceiverrejectsMambaHybridCacheManagerand any explicit Mamba cache manager (ValueError)Transceiver auto-selection: when
enable_pipelined_transferis set and no explicittransceiver_runtimeis given,resolve_cache_transceiver_configselectsPYTHONand logs a warning. Settingtransceiver_runtime='CPP'explicitly, or using a backend that resolves to something other than NIXL, raisesValueError.The Mamba/hybrid gate applies to both
MixedMambaHybridCacheManagerandCppMambaHybridCacheManager. A normal transfer copies recurrent state once after prefill has produced its final value. Pipelined transfer would attach the samemamba_state_indexto every chunk while prefill continues mutating that slot; because the writes are asynchronous, a slice has no stable per-chunk recurrent-state snapshot. The implementation therefore fails during transceiver creation instead of allowing a configuration that could transfer an intermediate or inconsistently observed state.Two notes on the validation added in this PR:
extract_disagg_cfgcalls_validate_disagg_config), so a mismatchedschedule_stylefails before any worker starts rather than on the first request.parse_disagg_config_filealso takesschedule_style_overrideso the--schedule_styleCLI flag participates in the same validation instead of being applied afterward._validate_requestrejects only a request that carriespy_disaggregated_paramswith a non-generation-first style. A request with no disaggregated params (an aggregate request hitting the same executor, e.g. during warmup) is allowed through.Architecture
Sender-side chunking, monolithic receiver
Chunking is entirely a sender-side concept. The context server splits its source blocks into N chunks and issues N writes; the generation server posts a single receive covering the whole prompt and never learns how many chunks arrived.
flowchart LR subgraph ctx [Context server] exec[PyExecutor._send_kv_async] --> tcv[respond_and_send_async] tcv --> bld[_build_prefill_chunk] bld --> tx["TxSession, one per request"] tx --> t0["KVSendTask slice_id 0"] tx --> t1["KVSendTask slice_id 1"] tx --> tn["KVSendTask slice_id N-1, is_last_slice"] end subgraph gen [Generation server] rx["RxSession, one per request"] --> r0["KVRecvTask slice_id 0, whole prompt"] end t0 -->|RDMA write| r0 t1 -->|RDMA write| r0 tn -->|"RDMA write, is_last"| r0This asymmetry is what keeps the change contained:
request_and_receive_asyncon the generation side is unchanged, and the receiver completes when it has seenexpected_transfersresults carryingis_last_slice.KVSlice
KVSlice(tensorrt_llm/_torch/disaggregation/base/transfer.py) previously described one whole request. It now describes one chunk of one request:is_last_slicewas a latent field that was alwaysTrue. It is nowFalsefor intermediate chunks, and it is the signal that drives finalization on both sides.total_blocks(new) is the full logical block span[0, total_blocks)that chunk offsets and the resident-suffix projection are expressed in. Without it, a receiver-side projection has no way to tell where a resident suffix begins.chunk(new,ChunkCoords) is where this slice sits in that span, as a block offset and count. Its presence is what makes a slice a chunk, so a monolithic transfer keeps its whole-request addressing untouched — including the packed beam layout, which the chunked path is not written for and which_build_prefill_chunkrejects viabeam_width == 1.token_range(removed). It had exactly one consumer: the sender readtoken_range.endto recovertotal_blocks = ceil(end / tpb), the anchor for all the suffix arithmetic..startwas never read, since_create_kv_slicealways produced[0, prompt_len). Chunking took both jobs away — a non-final chunk's end is neither the request's block span, which the destination projection needs, nor itsprompt_len, which SWA needs — so they moved to the explicittotal_blocksfield and to a required sessionprompt_lenrespectively, leaving nothing that reads the range.Reusing the vacated field to carry the chunk cursor would have given it a third, unrelated meaning, encoded as
block_index * tokens_per_blockand divided back out on arrival — and it cannot carry that meaning honestly anyway, because the chunk window is decided rather than observed (see "Deriving the chunk"). So a slice now describes its extent exactly one way: a chunk by itschunk, a monolithic transfer by the session'sprompt_len, which is where the request's extent already lives.Deleting the field moves one invariant:
_create_kv_slicemust produceceil(prompt_len / tpb)blocks and must not include thenum_extra_kv_tokensslots speculative decoding reserves, since an extra block would shift every per-layer token start. That was previously expressed astoken_range.end == prompt_len;TestCreateKvSliceBlockSpannow asserts it on the block list directly, which is the thing the sender actually consumes.SessionArgsBase.prompt_len(and theTxSession/RxSession/KVSendTaskconstructors) changed fromOptional[int]to required. SWA needs the request'sprompt_lento compute the stale-block boundary:Previously the slice's own extent was an acceptable stand-in for
prompt_len, because a slice was the whole request. A chunk stops at its own boundary, so using it would place the sliding window in the wrong place for every chunk but the last.TxSession and KVSendTask
TxSessionalready held a list of KV tasks; the list simply now has more than one entry.TxSession.sendassignsslice_id = len(self.kv_tasks), so a chunk's id is its arrival order.Session status aggregates over all tasks:
So the session is
KV_TRANSFERREDonly once every chunk has landed,ERRORif any chunk failed, andTRANSFERRINGwhile any chunk is mid-write.wait_completewaits on every task, andhas_transferring_tasks()(used bycancel_request) reports whether any chunk is mid-write.Slice ordering. With N slices per session there are now two producers that can enqueue writes for the same peer:
send()on the executor thread, andSender._respond_with_kvreplaying already-created slices when a peer registers late.TxSession.dispatch_lockspans both the snapshot and the enqueue loop in each path, because the receiver completes on theis_last_sliceresult without checking that earlier slices landed — a newer slice reaching a peer's queue ahead of an older one would let the generation server start decoding on incomplete KV.Executor loop integration
sequenceDiagram participant Exec as PyExecutor participant Sampler as TorchSampler participant Tcv as KvCacheTransceiverV2 participant Tx as TxSession participant Worker as Sender worker thread participant Rx as RxSession on gen loop intermediate prefill chunk Exec->>Exec: _forward_step(scheduled_batch) Exec->>Sampler: _update_requests(sample_state) Sampler-->>Exec: sampler_event.synchronize() Exec->>Tcv: respond_and_send_async(req) Tcv->>Tcv: _build_prefill_chunk(req) Tcv->>Tx: send(slice) with is_last_slice False Tx->>Worker: dispatch KVSendTask slice_id i Worker->>Rx: RDMA write then KV_AGENT_RESULT end Note over Exec: final chunk: is_context_finished or is_finished_due_to_length Exec->>Exec: release_index_slot(req) Exec->>Exec: async_transfer_manager.start_transfer(req) Exec->>Tcv: respond_and_send_async(req) Tcv->>Tx: send(slice) with is_last_slice True Tcv->>Tcv: _finalize_send: pack and send aux, set ContextPhaseParams Tcv->>Exec: req.state = DISAGG_CONTEXT_TRANS_IN_PROGRESSBoth branches live in
PyExecutor._send_kv_async, which runs after each forward step:Points worth calling out:
TorchSampler._update_requestsalready callsstate.sampler_event.synchronize()before_send_kv_asyncruns, so the chunk's KV writes are complete on the device by the time the slice is dispatched. An earlier revision carried acuda_eventonKVSlicefor this; it was removed as redundant.respond_and_send_asynchandles both cases. It creates or reuses theTxSessionvia_get_or_create_send_session, builds a chunk with_build_prefill_chunkwhen pipelining is on, and only calls_finalize_sendand setsDISAGG_CONTEXT_TRANS_IN_PROGRESSwhenslice.is_last_sliceis true.start_transfercommits the request's blocks to the reuse tree and pins them; it must run before the last slice is sent, because sending the last slice is what starts the request's transition toward completion.Chunk projection
The hard part of sending a chunk is that "chunk" is defined in one coordinate space (global block position within the prompt) while every block list involved is a resident suffix of some other range. Sliding-window layer groups, prefix reuse, and incremental allocation during prefill all shorten a list from the front. Indexing such a list with a raw global offset yields the wrong blocks.
project_blocks_to_global_chunk(base/transfer.py) resolves this by intersecting ranges rather than indexing:A list that does not reach the chunk at all returns empty rather than raising or silently sending the wrong blocks.
Deriving the chunk
_build_prefill_chunk(transceiver.py) turns the scheduler's token chunk into block coordinates:total_blocksis alwaysceil(prompt_len / tpb)— the full prompt span, which is what the destination side is allocated for and whatKVSlice.total_blockscarries to the sender worker. The chunk bounds are clamped to it. Passingresident_block_end=chunk_endalso normalizes the two cache-manager allocation models: V1's full-prompt reservation is capped at the computed boundary, while V2's incrementally allocated list is already bounded there.This is the only place chunk geometry is decided, and the resulting
ChunkCoordstravels to the sender worker as-is. That matters because the window is not a function of the scheduler's token bounds alone: extending the first chunk back to block 0 depends onprepopulated_prompt_len, the clamp depends onprompt_len, and the rounding rule below depends ontokens_per_block. A sender re-deriving the window from a token range would have to be handed the decided answer encoded asblock_index * tpband divide it back out — the same numbers, one lossy detour, plus an alignment precondition to validate on arrival.Unaligned chunk boundaries round up
Nothing requires the scheduler to cut chunks on block boundaries, and the two schedulers disagree about whether it does. V1 makes it true by construction:
setPrepopulatedPromptLenshrinks the first chunk soprepopulatedPromptLen + chunkSizefloors to a block boundary, andTLLM_CHECKs the result to prevent cache fragmentation. The V2 Python scheduler rounds the chunk size tochunk_unit_sizebut not the absolute end, so a partial-block reuse hit offsets every boundary that follows — a gemma3 V2 run withtokens_per_block=32produced a non-final chunk ending at token 259.An earlier revision asserted alignment here and failed on exactly that. The transfer layer should not dictate scheduler chunk policy, so the rule is instead:
An unaligned boundary therefore lands one block in both chunks. The earlier chunk sends it with a stale tail past the boundary; the next chunk, whose start floors into the same block, rewrites it whole once the tokens are computed.
flowchart LR subgraph chunkA [Chunk A, ends at token 259] a1["blocks 0..7<br/>fully computed"] a2["block 8<br/>rounded up, tail is stale"] end subgraph chunkB [Chunk B, starts at token 259] b1["block 8 again<br/>start floors to 8"] b2["blocks 9 onward"] end a2 -->|"same worker thread, second write waits"| b1Four properties make this safe:
ceil(chunk_end_pos / tpb)is the index just past the block holding the last computed token, and that block is allocated — the forward pass just wrote into it. Soresident_block_end = chunk_endstays within the source list under both allocation models.Sender._enqueueroutes by(unique_rid, peer_rank)to a single worker thread, and_deliver_kv_to_agentblocks onstatus.wait()before taking the next item. The stale copy is fully landed before the computed copy is even submitted.is_last_slicefrom every peer, so no decode reads the destination while a partial block is sitting there.The cost is at most one extra block per chunk boundary, negligible at production chunk sizes. Rounding down instead was rejected: it leaves the partial tail unsent until the next chunk (which must start at
floorregardless, so the block is re-sent either way), and it produces empty slices whenever a chunk falls entirely inside one block.The first slice always starts at block 0
The scheduler's chunk sequence does not necessarily cover the whole prompt. On a context-side prefix-reuse hit,
setPrepopulatedPromptLenadvancescontext_current_positiontoprepopulated_prompt_lenbefore the first chunk is cut, sopy_last_context_chunkstarts atP = prepopulated_prompt_lenand no chunk ever spans[0, P). Those blocks are resident and valid —_create_kv_sliceforcescached_per_lg = [0] * len(layer_groups)on the context side, so the base slice holds them — but the projection would drop them and the generation server would decode over whatever those pages held:So the first slice extends its start back to block 0, carrying the reused prefix along with the first computed chunk. The monolithic path was never affected: it sends the whole base slice.
prepopulated_prompt_lenis written exactly once per request and chunk starts increase monotonically, so only the first chunk satisfies the equality; with no reuseP == 0and the rule is a no-op.req.is_first_context_chunkcannot substitute for it, because it comparescontext_current_positionagainstprepopulated_prompt_lenand the cursor has already advanced by the time_send_kv_asyncruns — the recorded chunk start is the pre-advance value.Both projections still hold with
chunk_start = 0. On the source,_create_kv_slice(..., resident_block_end=chunk_end)first removes uncomputed V1 pages and trims VSWA groups to the overlap between the computed prefix and the final prompt window. On the destination (resident_block_end = total_blocks) the overlap is[max(0, G_b), chunk_end), which no-ops whatever the generation server already has. And when the whole prompt fits in one chunk alongside a reuse hit, the slice degenerates to a chunk spanning[0, total_blocks):suffix_end_blocksequalstotal_blocksand the destination projection is the identity, so the write is addressed byte-for-byte like a non-pipelined transfer.test_whole_prompt_chunk_addresses_like_a_monolithic_slicepins that by building the same slice with and withoutchunkand comparing the resultingWriteMeta.This gap was invisible in CI because it is masked whenever the generation server has the same prefix cached: its
RecvReqInfoblock list is trimmed bycache_skip,dst_startrises above the gap, and_align_kv_blockswould have trimmed those blocks anyway. The bug bites only when the two cache states diverge — generation-side reuse off, a cold generation cache, different eviction pressure, or different DP routing.Two projections with different
resident_block_endThe same helper is called on both sides of the transfer with a deliberately different end bound:
resident_block_end_build_prefill_chunkchunk_endchunk_end, while V2 is already incremental._build_kv_write_metatotal_blocksUsing
total_blocksfor the source was the bug fixed inFix resident chunk end calculation(0628b97): it placedresident_starttoo far left, so every intermediate chunk selected blocks from the wrong position in a partially allocated source list.VSWA chunk projection: V1 cache manager
Consider a 16-block prompt, a final 4-block VSWA suffix
[12,16), and an intermediate chunk[11,13). Intervals are end-exclusive, so this chunk has computed logical blocks 11 and 12 and overlaps the final window only at block 12. Page namespNbelow are illustrative physical pages holding logical blockN.V1 reserves physical pages for the entire prompt before all chunks have been computed. The source must therefore be capped before taking the VSWA suffix:
flowchart LR raw["V1 raw pages<br/>p0 ... p15<br/>full prompt reserved"] cap["Cap at chunk_end = 13<br/>p0 ... p12"] trim["Trim below final stale_end = 12<br/>p12"] project["Project chunk [11,13)<br/>p12"] dst["Write destination<br/>logical block 12"] raw --> cap --> trim --> project --> dstWithout the cap, trimming first would produce
p12 p13 p14 p15. Reinterpreting that list as a suffix ending at block 13 could select future pages such asp14and silently pair one with destination block 12.VSWA chunk projection: V2 cache manager
V2 grows and evicts incrementally. At
chunk_end = 13, its live 4-block VSWA range is[9,13), so it exposesp9 p10 p11 p12and has no future pages. It still needs the same final-window trim; otherwise the sender can pair an earlier computed page with destination block 12.flowchart LR raw["V2 raw live pages<br/>p9 p10 p11 p12<br/>range [9,13)"] cap["Cap at chunk_end = 13<br/>no change"] trim["Trim below final stale_end = 12<br/>p12"] project["Project chunk [11,13)<br/>p12"] dst["Write destination<br/>logical block 12"] raw --> cap --> trim --> project --> dstAfter normalization, both managers send exactly one computed source page for the same logical position the receiver requested:
Sender-worker side
_build_kv_write_meta(native/transfer.py) applies the destination projection and then reduces everything to a token-space alignment:slice.chunkselects the path. It is set only by_build_prefill_chunk, so the sender reads the chunk window rather than inferring one, and a monolithic transfer — including a packed beam layout, which the chunked path does not model — cannot accidentally be routed through it.suffix_end_blocksischunk.block_offset + chunk.block_countwhen chunked andtotal_blocksotherwise; per-layer token starts follow from(suffix_end_blocks - n_blocks) * tpb.req_info.dst_start_token(generation-side prefix reuse) and by the SWAstale_end, and_align_kv_blockstrims both arrays to the shared token overlap. That single overlap computation covers all four cases: no prefix cache, context-side prefix cache, generation-side prefix cache, and a chunk that falls entirely inside the generation server's already-cached prefix (which produces an empty transfer).Wire protocol:
sender_slice_idandreceiver_slice_idThe
KV_AGENT_RESULTframe used to carry a single slice id. The receiver used it purely as an index into its own task list, so the field was always semantically the receiver's id — but it was namedsender_slice_idon the receive side, and once chunking existed the sender hardcoded0into it while its real chunk id sat unused inwrite_meta.slice_id. The consequences were a per-chunk RDMA failure that always loggedslice=0, and two send paths that disagreed about what the field meant (_send_failed_result_to_receiveralready sentinfo.slice_id, while_send_kv_result_to_receiversent a literal0).The frame now carries both ids explicitly:
sequenceDiagram participant TxSession participant SenderWorker participant RxSession TxSession->>SenderWorker: KVSendTask.slice_id = chunk index 0..N-1 Note over SenderWorker: WriteMeta.sender_slice_id = task.slice_id<br/>WriteMeta.receiver_slice_id = req_info.slice_id SenderWorker->>RxSession: KV_AGENT_RESULT with both ids, is_last, status Note over RxSession: index _kv_tasks[receiver_slice_id]<br/>log sender_slice_id only_KV_RESULT_PREFIXwidened fromstruct.Struct("<qqq?B")tostruct.Struct("<qqqq?B"); the field order isinstance_rank, unique_rid, sender_slice_id, receiver_slice_id, is_last, status. This is a ctx/gen wire format with no version negotiation — the receiver unpacks whatever arrives against its compiled-in struct, so both servers must run matching builds.NO_SLICE_ID = -1means "no sender slice for this result", used by_send_failed_result_to_receiverwhen a session fails before anyKVSendTaskexists.WriteMeta.sender_slice_id = task.slice_ididentifies the sender's chunk and is carried for logging and cross-side correlation only.WriteMeta.receiver_slice_id = req_info.slice_id if req_info.slice_id is not None else 0is the peer's own task index, echoed back fromRecvReqInfo, and is what resolves the task.RxSession.process_kv_agent_result(peer_rank, receiver_slice_id, sender_slice_id, is_last_slice, status, ...)indexes_kv_tasks[receiver_slice_id]and includessender_slice_idin the assertion message, the bounce-scatter-failure path, the perf warning, the completion debug line, and the FAILED detail — so a chunk-level RDMA failure is attributable to a specific chunk.This is behavior-neutral for the monolithic receiver:
receiver_slice_idis0in every current deployment. It is naming plus an explicit protocol field, laying the groundwork for a future multi-task receiver.KV transfer state
Before pipelined transfer,
DISAGG_CONTEXT_TRANS_IN_PROGRESSwas a faithful proxy for "the fabric may be reading this request's KV pages". It no longer is: after the first non-finalsession.send(slice), chunks are in flight while the request is still inCONTEXT_INIT.Two failures followed from the gate keying entirely off request state. Cancelling a request mid-prefill took the "nothing to cancel" path in
_try_cancel_request, so_handle_responsesterminated it andfree_resourcesreleased pages aKVSendTaskmight still be reading, while theTxSessionleaked in_send_sessionsand the receiver was never notified. Separately,respond_and_send_asyncstarts the timeout clock on the first chunk, but_check_kv_transfer_timeoutonly walkedasync_transfer_manager.requests_in_transfer()— which the request does not enter untilstart_transferon the last chunk — so the entire pipelined phase was unmonitored while its clock ran.The fix is two orthogonal dimensions instead of one overloaded state.
LlmRequestStatekeeps meaning "compute and response phase". Transfer activity is answered by the component that actually owns the resources — the transceiver's session maps — so it cannot drift out of sync the way a mirrored request field would.flowchart LR subgraph phase [Request phase - LlmRequestState] ctxInit[CONTEXT_INIT] --> transProg[DISAGG_CONTEXT_TRANS_IN_PROGRESS] transProg --> complete[DISAGG_CONTEXT_COMPLETE] end subgraph transfer [Transfer ownership - transceiver session maps] nosession[no session] --> active[session in _send_sessions] active --> torn[session closed and deleted] end ctxInit -.->|first non-final send| active torn -.->|safe to free KV| completeSession membership is the right record:
_get_or_create_send_sessioninserts before the firstsend, and every teardown path (cancel_request, the completed and cancelled loops incheck_context_transfer_status,_close_failed_sessions) deletes it.Concretely:
KvCacheTransceiver.has_inflight_transfer(req)andhas_any_inflight_transfer()are non-abstract and default toFalse, which leavesBindKvCacheTransceiveruntouched — the C++ transceiver has no pipelining, so state and transfer activity coincide there.KvCacheTransceiverV2implements both from session membership;get_unique_ridreturningNonefor a non-disagg request naturally yieldsFalse._is_request_in_transmissionreturnsTruewhen either the state says so orhas_inflight_transfer(request)does. Its only caller is_try_cancel_request, so the blast radius is contained: a mid-prefill cancel now routes throughKvCacheTransceiverV2.cancel_request, which cancels theTxSession, notifies the receiver, and returnsFalsewhile any task isTRANSFERRING. The existing retry in_handle_canceled_requeststhen holds the KV pages until the write drains — the same behavior the monolithic path already relies on._send_kv_asyncsnapshotscanceled_req_idsbefore the loop and gates only the intermediate-chunk branch on it. A session whose_terminal_statusis alreadyCANCELLEDwould otherwise be fed another chunk and produce a spuriousFAILEDresult to the receiver. The last-chunk branch is untouched so nothing can be stranded in_requests_in_transfer.PyExecutor._has_any_inflight_kv_transfer()ORskv_cache_transceiver.has_any_inflight_transfer()intoasync_transfer_manager.has_any_inflight_requests(), and replaces the latter at the four loop-level gates that would otherwise suppress timeout checking during the pipelined phase (the three_check_kv_transfer_timeoutcall sites and the KV-pressureRuntimeError)._check_kv_transfer_timeoutgains a sweep over active context-only requests that are absent fromrequests_in_transfer, have a non-Nonepy_kv_transfer_start_time, and have an in-flight transfer._check_disagg_ctx_cache_transfer_statusgains the matching recovery sweep: for a request flaggedpy_kv_transfer_timed_outbut not yet known to the transfer manager, callcancel_requestand setDISAGG_TRANS_ERRORonly when it returnsTrue(session closed, nothing mid-write). That reuses the established lever —_check_cache_transfer_errorspicks it up for non-ADP and_handle_disagg_cache_errors_syncedvotes on it under ADP — so no new divergence path is introduced.