[Bugfix] Free out-of-window KV blocks on the processed-token basis - #50
Draft
ivanium wants to merge 1 commit into
Draft
[Bugfix] Free out-of-window KV blocks on the processed-token basis#50ivanium wants to merge 1 commit into
ivanium wants to merge 1 commit into
Conversation
Under async scheduling / PP the scheduler runs ahead of the GPU: `request.num_computed_tokens` optimistically includes tokens of steps whose output has not been processed yet. `remove_skipped_blocks` (SWA / chunked-local / mamba) used that optimistic count as the free boundary, so blocks that an in-flight step's attention window still reads were returned to the pool at schedule time. A KV-consumer connector can reallocate them as load destinations and overwrite them via a transfer that is not ordered against the compute stream's read, corrupting the in-flight step's output (vllm-project#47282). Rejected spec tokens can also roll the boundary back below already-freed blocks. Track `Request.num_in_flight_tokens` (scheduled minus processed) and free on `num_computed_tokens - num_in_flight_tokens`: blocks slide out of the window one step later, once no unprocessed step can read them. This subsumes MambaManager's speculative-token subtraction (same rollback hazard, now handled exactly for prefill chunks and PP depth too), so that local fix is removed. Sync scheduling has no in-flight tokens and is unchanged. The SWA / chunked-local per-request bound grows accordingly: a request now holds up to `max_concurrent_batches - 1` in-flight chunks on top of its window, so `max_admission_blocks_per_request` accounts for it in both startup pool sizing and the runtime admission gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fvie7xNFmjJzS8e1izTXBm Signed-off-by: Yifan Qiao <yifanqiao@inferact.ai>
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.
Problem
With async scheduling (now default) or PP, the scheduler runs ahead of the GPU:
request.num_computed_tokensis advanced at schedule time.remove_skipped_blocks(SWA / chunked-local / mamba) used this optimistic count as the free boundary, causing two bugs:num_speculative_tokenssubtraction); SWA / chunked-local were exposed.Fix
One mechanism fixes both: track
Request.num_in_flight_tokens(scheduled minus processed, structurally symmetric across preemption / spec rejection / failed-KV-load) and free on the settled basisnum_computed_tokens - num_in_flight_tokens.Invariant: a block is returned to the pool only when it is outside the attention window of every unprocessed step and no rollback can make it needed again. Sync scheduling has zero in-flight tokens — behavior unchanged, no gating flag.
Consequential (not an independent fix): the SWA / chunked-local per-request admission bound grows to
W - 1 + max_concurrent_batches * max_num_batched_tokens, applied in the single source of truth used by both startup pool sizing and the runtime gate. MambaManager's local subtraction is removed (subsumed exactly, now also covering prefill chunks and PP depth).Alternatives considered: vllm-project#47653 (BlockPool fence) and a scheduler-callback defer both fence the free downstream; this PR corrects the lying input instead — no new queues, no BlockPool state, no callbacks. Request-level finish/preempt frees remain behind the vllm-project#45357 fence (write-after-free, unreachable by a lagged basis).
Tests
tests/v1/core/test_swa_inflight_window_free.py— counter symmetry, out-of-window blocks held until the reader step is processed, sync unchanged, admission cap.test_deferred_block_free,test_single_type_kv_cache_manager,test_kv_cache_utils,test_async_scheduler,test_prefix_caching, offloading-connector scheduler suite (82),test_scheduler.pyfailure set byte-identical to clean main (35 pre-existing env failures).AI assistance was used for this change; every line reviewed by the submitter.
🤖 Generated with Claude Code