Repository navigation
[Disagg][StagingBuffer][2/2] Support radix cache - #30545
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
0d3192c to
ce4170f
Compare
ce4170f to
9037dcf
Compare
e3527fd to
3e0c9cc
Compare
ef2a87f to
cdbab30
Compare
56250c0 to
462b1f7
Compare
Builds on the staging lifecycle hardening in the previous commit. The heterogeneous-TP staging path identifies chunks positionally (chunk_idx = start_page // full_chunk_pages) against a uniform prefetched ring-allocation grid, so any send whose boundaries do not land on that grid silently breaks the protocol. A prefill radix-cache hit inflates the first send past one grid slot (and skews every later chunk index), causing ring-allocation overruns, decode-prefix scatter offset corruption, and a blind tail scatter from chunk_infos[-1]. Prefill: floor non-last sends to the staging grid (base = decode_prefix_len) and split every send into one TransferKVChunk per grid slot; re-enable the cached-prefix early-send under staging, capped at an at-rest prefix snapshot (prefix_indices grows under overlap scheduling to cover launched-but-unresolved chunks). Decode: scatter is fully arrival-driven (the last chunk now also sends CHUNK_READY); a room completes only after all-ranks Success with every allocation scattered and every scatter event finished, and stalls past the disaggregation waiting timeout surface as KVPoll.Failed; scatter offsets account for the decode-side cached prefix; writer fan-in accounting moves to the handler so room teardown can purge partial counts. Adds test_staging_radix_grid.py covering the grid math, arrival-driven completion, fan-in accounting, release_room, and the decode-prefix page alignment guard.
The existing hetero-TP staging e2e uses the default chunked-prefill-size, so the shared gsm8k few-shot prefix fits in a single grid slot and never exercises the radix-hit grid-split or decode-prefix scatter-offset paths. Add a prefill-TP4->decode-TP2 class with chunked-prefill-size 256 and decode radix enabled, so the shared prefix spans several grid slots and both sides reuse cached prefix -- the configuration that was broken before this change.
Per .claude/rules/no-getattr-defensive.md, declare fields at construction and access them directly instead of defensive getattr/hasattr: - TransferKVChunk.staging_counted (bool = False) replaces the dynamically set _staging_counted flag. - StagingAllocator._scatter_stream (None) replaces the lazily monkey-patched attribute; _scatter_region uses "is None" and release_room accesses it directly. - Receivers always initialize chunk_staging_infos = [] so readers access it directly (the receiver-None checks are kept -- those are real). - Direct access for the always-set manager fields enable_staging / _staging_ctx and for receiver.bootstrap_infos.
The staging teardown-ordering fix only dropped per-room state on Success, so a Failed last chunk left transfer_infos / req_to_decode_prefix_len behind (cleaned up later only via the sender failure path). Restore the original behavior of tearing down on the last chunk regardless of Success/Failed -- but keep the outstanding-chunk guard so a deferred earlier chunk is never stranded, and keep a non-last Failed chunk's room (more chunks may follow). Applied symmetrically to the nixl and mooncake workers. Also seed _staging_outstanding on the transfer-worker test mock, which builds its manager via object.__new__ and so must mirror __init__.
0fef2a0 to
ad0747a
Compare
| if elapsed > self.completion_timeout: | ||
| logger.error( | ||
| "[STAGING] room=%s not complete %.0fs after all-ranks Success " | ||
| "(a scatter never arrived); failing the request.", | ||
| room, | ||
| elapsed, | ||
| ) | ||
| decode_req._staging_failed = True |
There was a problem hiding this comment.
Should this also respect SGLANG_DISAGGREGATION_WAITING_TIMEOUT?
There was a problem hiding this comment.
Yes, self.completion_timeout is initialized from SGLANG_DISAGGREGATION_WAITING_TIMEOUT in __init__, so this check already follows that env var.
ShangmingCai
left a comment
There was a problem hiding this comment.
Others look good. But the logic is becoming more and more complicated, we should do some cleanup in the near future.
register_decode_req raised (killing the prefill scheduler) when a decode request's cache_protected_len was not page-aligned. Mark the request failed via the existing _staging_failed flag instead -- the staging poll path surfaces it as KVPoll.Failed -- so one misconfigured decode endpoint cannot take down the whole prefill scheduler.
a3126da to
02644c3
Compare
|
/rerun-test test/registered/disaggregation/test_disaggregation_different_tp.py |
|
Results for 🚀 |
|
/rerun-test test/registered/disaggregation/test_disaggregation_different_tp.py |
|
Results for 🚀 |
|
/rerun-failed-ci |
Co-authored-by: Shangming Cai <csmthu@gmail.com>
Co-authored-by: Shangming Cai <csmthu@gmail.com>
A prefill send could cross a staging slot boundary when a radix-cache hit or scheduler slicing left the send cursor off the chunked-prefill grid, so a single chunk exceeded one staging slot and failed geometry validation on decode (pages > slot capacity). Anchor the grid at the decode prefix length recorded at bootstrap, hold back partial slots on non-final sends, and split each send into one sub-send per staging slot. Only the final sub-send carries state indices and is marked last. Prefetch and the sender share staging_grid_tokens(). Backport of the prefill grid segmentation from upstream sgl-project#30545. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Motivation
The heterogeneous-TP staging path identifies chunks positionally (
chunk_idx = start_page // full_chunk_pages) against a uniform prefetched ring-allocation grid, so any send whose boundaries do not land on that grid silently breaks the protocol. A prefill radix-cache hit inflates the first send past one grid slot (and skews every later chunk index), causing ring-allocation overruns, decode-prefix scatter-offset corruption, and a blind tail scatter fromchunk_infos[-1]. As a result the staging buffer could not be combined with radix cache.Modifications
Builds on the robustness groundwork in #31217.
base = decode_prefix_len) and split every send into oneTransferKVChunkper grid slot; re-enable the cached-prefix early-send under staging, capped at an at-rest prefix snapshot (prefix_indicesgrows under overlap scheduling to cover launched-but-unresolved chunks).CHUNK_READY); a room completes only after all-ranks Success with every allocation scattered and every scatter event finished, and stalls past the disaggregation waiting timeout surface asKVPoll.Failed; scatter offsets account for the decode-side cached prefix; writer fan-in accounting moves to the handler so room teardown can purge partial counts.test_disaggregation_different_tp.py) withchunked-prefill-size 256+ decode radix, so the shared prefix spans several grid slots and both sides reuse the cached prefix — the configuration that was broken before this change.Accuracy Tests
Qwen3.5-397B-A17B-FP8,
DEP4prefill →TP4decode, staging buffer + decode radix cache both enabled — the combination this PR fixes (a radix-cache hit under staging previously corrupted decode output / wedged the ring).gsm8k's few-shot prefix fits within one grid slot, so it mainly confirms staging adds no regression; the needle probe uses a genuine ~127.5k-token shared cached prefix and directly exercises the radix-hit grid-split and decode-prefix scatter-offset paths.
Speed Tests and Profiling
Topology-level Pareto comparison on 8×GB300 (Qwen3.5-397B-A17B-NVFP4, AgentX 256k-context workload, one 4-GPU prefill node + one 4-GPU decode node). The heterogeneous run enables the staging buffer.
At essentially matched cost efficiency (~27.3k total tokens/s/GPU), the DEP4-prefill → TP4-decode point has 9.61% higher output TPS/user in this comparison.
Checklist
CI States
Latest PR Test (Base): ✅ Run #30984792496
Latest PR Test (Extra): ✅ Run #31090378169