Repository navigation
fix(nixl): prevent early staging buffer reuse and duplicate sends - #38100
Open
fly-go-run wants to merge 1 commit into
Open
fly-go-run wants to merge 1 commit into
fly-go-run wants to merge 1 commit into
Conversation
Signed-off-by: 刘旭 <liuxu49@58.com>
fly-go-run
marked this pull request as ready for review
September 5, 2026 04:16
fly-go-run
requested review from
ByronHsu,
Duyi-Wang,
HaiShaw,
ShangmingCai,
hnyls2002 and
sogalin
as code owners
September 5, 2026 04:16
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.
Closes #38099.
Motivation
Summary
While investigating incorrect output in a production deployment with separate prefill and decode workers, we found two defects in NIXL staging sends.
A worker uses one staging buffer to prepare the key/value (KV) cache data for each destination. It can prepare B's data while A's asynchronous transfer is still reading that buffer. If B instead has to wait for staging space, the worker puts the entire data chunk back in its queue and can resend A, including its attached state and metadata.
This PR fixes those two send-path defects. They are plausible contributors to the production symptoms, but the particular production failure has not been reproduced with this patch alone.
Modifications
TransferKVChunk, so a retry processes only unfinished destinations.The change uses the existing NIXL completion states. Other paths that prepare data in separate buffer regions still wait for their transfers before reusing those regions. Handling unfinished transfers after a transport error remains separate work in #36612 and #36707.
Accuracy Tests
Validation
Tested against
db89f639ef475821e6669958cfe22caf914df022:test_nixl_backend_basic.py: 47 passed, plus 2 subtests.test_nixl_deferred_kv_release.py,test_nixl_sender_failure_cleanup.pyandtest_disaggregation_wire.py: 42 passed, plus 10 subtests.git diff --checkalso passed.An additional local review ran four checks through the actual staged-send method, including another request running between retries and repeated waits for space. All four passed with the fix. Three multi-destination cases failed on the original code; the single-destination case passed on both. These extra checks are local review artifacts, not part of the PR's test file.
These tests check the real worker logic on macOS/Python 3.13. Triton imports are stubbed for the CPU tests,
torch.compileis disabled, and GPU operations and transfers are simulated. Separately, a local implementation of these fixes has already been deployed to production and load-tested on real GPUs, with normal operation reported. That deployment includes other fixes; the exact minimal patch in this PR has not had a standalone GPU comparison.Speed Tests and Profiling
The deployed local implementation has undergone production GPU load testing. An isolated before/after throughput comparison for this exact upstream patch has not been run.
Waiting for completion reduces overlap between destinations sharing the source buffer. The current placement also waits for staged KV before sending the final state and metadata, so a single-destination request may lose some overlap too.
A targeted GPU test with a destination temporarily lacking staging space would provide additional coverage for the retry behavior. Separate source buffers could restore overlap, but would require a larger change.
Checklist
Related: #36893 (similar Mooncake retry issue), #36612 and #36707 (error handling), and #37697 (out-of-order decode completion).
CI States
Latest PR Test (Base): ❌ Run #33943413770
Latest PR Test (Extra): ❌ Run #33943413643
Latest PR Test (AMD ROCm 7.2): ❌ Run #33943413697