Repository navigation
[Bugfix][KV Offload] Release pending CPU lookup pins on cache reset - #59862
Merged
ivanium merged 2 commits intoOct 4, 2026
Merged
Conversation
Release lookup-only CPU block references before resetting the cache. Cover reset before GPU allocation in eager/lazy and single/multi-group cases. Co-authored-by: OpenAI Codex Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
ivanium
approved these changes
Oct 4, 2026
ivanium
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Thanks for the fix!
Contributor
|
/ci run |
ivanium
enabled auto-merge (squash)
October 4, 2026 02:54
|
✅ @zupengwang, CI is now available for this PR.
|
|
✅ Triggered Buildkite CI #92778 for commit |
Collaborator
CI selector (shadow): 3 test steps (6 jobs) instead of 62 (79 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Selector would run (3)
Would skip (today's rules run them) (59)
Would add (today's rules do not run them) (0)none AMD mirrors: would skip (51)
AMD mirrors: would add (0)none 2 changed files · base |
michael-gpu
pushed a commit
to practical-inference/vllm-ampere-extended
that referenced
this pull request
Oct 20, 2026
…llm-project#59862) Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
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.
Overview
Release SimpleCPUOffload lookup-only CPU block references during cache reset. A request waiting for GPU allocation can otherwise prevent
reset_prefix_cache(reset_connector=True)from succeeding even after GPU blocks and in-flight transfers have drained.Claims
Validation
Validated against upstream
5f30fc7031cae49bf51073fc953d419b08f8887c, using its matching precompiled native extensions.reset()returns False)All applicable changed-file hooks, mypy 3.10/3.12 and
git diff --checkpassed. The existing Buildkite KV Offload suite collects the new test; test tethering passed. Upstream CI has not run for this candidate.The GPU diagnostic used MRV2, FP16, TP=1, eager execution and 128 MiB each of GPU/CPU KV cache. After a real CPU store/load round trip with 256 cached tokens, it explicitly paused a queued request after the production CPU lookup. Baseline reset returned False; the patch released all 16 lookup pins, reset successfully and generated the same token
[19]with zero cached tokens. Actual allocation pressure is covered separately by the real Scheduler diagnostic, whose worker token output is synthetic. This is a focused correctness check, not a performance or broad model-quality claim. Multi-node, disk/GDS and other GPU backends were not validated.Details
get_num_new_matched_tokens()pins matching CPU blocks untilupdate_state_after_alloc()consumes the lookup. Failed GPU allocation leaves that lookup in_pending_cpu_hits. Reset already abandons transfers safely but omitted these pre-allocation references.Reuse
_free_pending_cpu_hit()before clearing the pending lookup map. These references have no associated worker DMA; submitted transfers retain their separate pins and existing completion-based cleanup. A waiting request performs a fresh lookup when scheduling resumes. No API, storage format or normal scheduling policy changes are introduced.Duplicate screening on 2026-10-03 covered open/closed/merged PRs, related issues, recent commits and the contributor's existing PRs. #42289 introduced temporary lookup pins; #39726 introduced reset support. Both are merged. The active #57816 addresses eager-store placement after MRV2 preemption, a different state transition. No active implementation of this lookup-pin reset fix was found. This draft does not claim to close an existing issue.
AI assistance: OpenAI Codex performed investigation, implementation, local/remote validation and drafting. The human contributor has confirmed review of the final change and authorized submission. The reported validation was performed with Codex; independent human reruns have not been confirmed.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.