Skip to content

[Bugfix][KV Offload] Ensure tracker progress for oversized offers - #54759

Merged
orozery merged 1 commit into
vllm-project:mainfrom
positive666:codex/kv-offload-tracker-progress-20260828
Sep 2, 2026
Merged

orozery merged 1 commit into
vllm-project:mainfrom
positive666:codex/kv-offload-tracker-progress-20260828

Conversation

@positive666

@positive666 positive666 commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

When one store offer contains more candidates than max_tracker_size, the current per-key LRU updates can evict candidates from the same offer before the next observation. With store_threshold > 1, repeated identical offers then keep every count at 1 and CPU KV offload never starts.

Change

  • Update and protect already tracked candidates before admitting new keys.
  • Evict only tracked candidates absent from the current offer.
  • Stop admitting new keys when every tracked entry is protected by the offer.
  • Add a regression test covering four candidates, tracker capacity three, and threshold two.

The tracker remains bounded by max_tracker_size, and processing remains linear in the number of offered keys.

Value

This restores forward progress for valid CPU KV offload configurations. It is a liveness fix rather than a microbenchmark optimization: affected requests previously produced no CPU stores at all.

Validation

/test-venv/bin/python -m pytest tests/v1/kv_offload/cpu/test_manager.py -q
28 passed

pre-commit run ruff-check --files vllm/v1/kv_offload/cpu/manager.py tests/v1/kv_offload/cpu/test_manager.py
Passed

pre-commit run ruff-format --files vllm/v1/kv_offload/cpu/manager.py tests/v1/kv_offload/cpu/test_manager.py
Passed

pre-commit run typos --files vllm/v1/kv_offload/cpu/manager.py tests/v1/kv_offload/cpu/test_manager.py
Passed

pre-commit run mypy-3.12 --hook-stage manual --files vllm/v1/kv_offload/cpu/manager.py tests/v1/kv_offload/cpu/test_manager.py
Passed

git diff --check origin/main...HEAD
Passed

Serving A/B used Qwen3-0.6B in the default compiled/CUDA-graph mode with store_threshold=2, max_tracker_size=3, and six candidates per offer:

  • Baseline: zero store bytes after six repeated target requests.
  • Fixed: all six candidates eventually stored; 11,010,048 store bytes, 14,680,064 load bytes, and 128 external prefix-hit tokens.
  • Generated token IDs remained [13483, 25293] in baseline and fixed runs.

Duplicate-work check

#52227 moved reuse counting from lookup to store offers, but retained sequential per-key insertion and does not prevent churn within an oversized offer. Searches for store_threshold max_tracker_size, KV offload tracker progress, and oversized offer found no open PR implementing this fix. Open PRs #52103 and #53087 address event provenance and HIT_PENDING timeouts respectively.

AI assistance

AI assistance was used for fix bug. The submitter reviewed every changed line and the validation results.

Protect candidates already observed in the current offer before admitting new keys, so an offer larger than the tracker capacity cannot churn admission counts forever.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Cheng Rui <286040359@qq.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the bug Something isn't working label Sep 1, 2026
@bojiang3

bojiang3 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Reviewed this — the fix is correct and the failure it describes is real. I
pulled both versions of the tracker logic out and ran the scenario from the
description (offer [a,b,c,d], max_tracker_size=3, store_threshold=2,
identical offer repeated):

before: final counts={'b': 1, 'c': 1, 'd': 1}
        keys passing threshold per offer: [[], [], [], []]      <- never stores

after:  final counts={'a': 4, 'b': 4, 'c': 4}
        keys passing threshold per offer: [[], [a,b,c], [a,b,c], [a,b,c]]

So with an offer wider than the tracker, every candidate really is evicted by a
later candidate of the same offer before it can be observed twice, and CPU
offload never starts. Matches the description exactly.

I also checked the invariant the new code depends on — that popitem(last=False)
can only ever evict a key that is not in the current offer. Since the first
loop move_to_ends every already-tracked key of the offer, the LRU end holds
only unprotected entries, and num_unprotected stops admission before that runs
out. Fuzzed it over 3000 random offers against a capacity-3 tracker: no
protected key was ever evicted.

Nothing else from me — LGTM.

@orozery orozery added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 2, 2026
@orozery

orozery commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

/ci run

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ @positive666, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #86835 for commit 9ae70da554e2.

@orozery orozery left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @positive666 !

@orozery
orozery merged commit 2a4e3cc into vllm-project:main Sep 2, 2026
96 of 99 checks passed
mylibrar pushed a commit to tanyuqian/vllm that referenced this pull request Sep 3, 2026
…lm-project#54759)

Signed-off-by: Cheng Rui <286040359@qq.com>
Co-authored-by: Codex <noreply@openai.com>
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…lm-project#54759)

Signed-off-by: Cheng Rui <286040359@qq.com>
Co-authored-by: Codex <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants