Repository navigation
[Kimi-K3] Deferred decode-side KV release (#35049 + #35360) on the zmq-mitigation base - #36610
Draft
hanming-lu wants to merge 3 commits into
Draft
hanming-lu wants to merge 3 commits into
hanming-lu wants to merge 3 commits into
Conversation
Co-authored-by: Shangming Cai <csmthu@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
This is the exact Kimi-K3 serving tree used to A/B-test the deferred
decode-side KV release fix on a GB300 PD deployment. It is
kimi-k3-zmq-max-sockets-mitigation(6dfdebf) with the deferred-release workcherry-picked on top, so the branch can be built into a serving image and
compared directly against the unpatched build.
Cherry-picks, in order:
05c7ebf64c[Disagg][StagingBuffer][2/2] Support radix cache— prerequisite for the two below97dedd1ce9[PD] Deferred decode-side KV release for aborts mid-transferadca19c497[PD] Deferred decode-side KV release for the NIXL backendThree conflicts came up, all purely additive (two
from typing import ...lines, plus new blocks in
environ.pyandmooncake/conn.py); each wasresolved by taking the incoming side.
The resulting tree hash is
a7845862cd43eecbd923fff557df1f180fcd8502, whichmatches the tree that was actually exercised on hardware — that equality is the
point of this branch, so a reviewer can confirm the image and the measurements
came from the same bytes.
What the measurements showed
Reproduced on a 2-prefill/1-decode Kimi-K3 deployment (mooncake, dp16 prefill ×2
send_kvcacheuntil released, so the abort/free/realloc ordering is enforcedrather than raced. Greedy decoding, 512 output tokens.
Without the fix, request B was allocated exactly the 32 KV pages and the
mamba slot that request A had just freed. A's stale write then landed in them
mid-generation: a checksum over 64 head-of-prompt KV rows (rows decode never
rewrites) changed 4 ms after prefill logged
WRITE_END ret=0, and B's outputdiverged from its own baseline at token 269, with 241 of 512 tokens differing.
B returned HTTP 200 and
finish_reason=lengththroughout, so nothing surfacedto the client. Notably A's prefill had already returned
finish_reason={'type': 'abort'}seconds earlier — the scheduler considered therequest finished while its transfer worker went on to write successfully.
With the fix, when the stalled write completes inside the hold window,
decode logs
DEFER_HOLDinstead ofFREE, B is not admitted while the pagesare held, and B's output is bit-identical to baseline.
One caveat worth reviewer attention
In every control run the hold ended via the timeout, not an ack:
The drain ack is sent by the prefill transfer worker, which is precisely the
thread that is stuck, so it cannot arrive while the write is stalled and
SGLANG_DISAGGREGATION_DEFERRED_DECODE_KV_RELEASE_TIMEOUTalways decides. Whenthe write outlives that timeout the original window reopens — in that
configuration B diverged at token 314 (197/512 tokens) with the fix enabled.
So on this workload the feature bounds the exposure to the timeout rather than
closing it. Whether that is acceptable depends on how long a real stalled RDMA
transfer can run; it may be worth either blocking the release until the ack
regardless of elapsed time, or having the sender drop the write when it observes
the room already failed at the point of send rather than only at dequeue.
Draft: opened to share the tested tree and these results, not proposing new code.
Test plan
git rev-parse HEAD^{tree}=a7845862cd43eecbd923fff557df1f180fcd8502,matching the tree that ran on hardware (independently re-derived by redoing
the cherry-picks from
6dfdebf0fbin a clean worktree).and driven through the three scenarios in the table above.
two different prefill engines returned byte-identical output ids.
PRs ship their own tests (
test_deferred_decode_kv_release.py,test_nixl_deferred_kv_release.py) but they were not run here. Each arm wasmeasured once, so there is no variance estimate.
CI States
Latest PR Test (Base): ❌ Run #34829317759
Latest PR Test (Extra): ❌ Run #34829317288
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.