Skip to content

[Bugfix][KV Offload] Skip cleaned-up async lookup batches - #55075

Merged
orozery merged 6 commits into
vllm-project:mainfrom
Alex-ai-future:bugfix/kv-offload-skip-cancelled-lookup-flush
Sep 6, 2026
Merged

orozery merged 6 commits into
vllm-project:mainfrom
Alex-ai-future:bugfix/kv-offload-skip-cancelled-lookup-flush

Conversation

@Alex-ai-future

@Alex-ai-future Alex-ai-future commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Purpose

Skip a secondary-tier async lookup when its last requester finishes before
the scheduler flushes that step's lookup batch.

User case

  1. Request A adds KV key K to the current step's async lookup batch.
  2. A is cancelled or finishes before on_schedule_end() calls flush().
  3. Cleanup removes A's lookup state, but previously flush() still submitted
    K to the background worker.

The fix revalidates each entry against the scheduler-owned lookup state before
queueing it. It drops only missing or superseded generations; a key shared by
another active request remains eligible. It deliberately does not attempt to
cancel batches that are already queued or backend calls already in flight.

No open PR was found for this flush-before-submit cancellation case. This is
complementary to #54872, which protects the result side after a batch has
already been submitted.

Test Plan

.venv/bin/python -m pytest \
  tests/v1/kv_offload/tiering/test_async_lookup.py -q
pre-commit run --files \
  vllm/v1/kv_offload/tiering/async_lookup.py \
  tests/v1/kv_offload/tiering/test_async_lookup.py
git diff --check

The regression test confirms that cleanup before flush() results in no
batch_lookup() call. The focused suite retains coverage for shared entries
and stale results after submission.

Test Result

15 passed in 1.75s
pre-commit --files: passed
git diff --check: passed

Model evaluation is not applicable; this is a scheduler lifecycle fix.

AI assistance was used for code and test development. The human submitter
reviewed every changed line and the test result.


Checklist
  • Purpose, user case, test plan, and results included.
  • Duplicate-work check completed.
  • AI assistance disclosed.

Alex-ai-future and others added 2 commits September 3, 2026 11:26
Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Alex <jihui.huang@daocloud.io>
Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Alex <jihui.huang@daocloud.io>

@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 3, 2026
@orozery orozery added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 3, 2026
@orozery

orozery commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

/ci run

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

@Alex-ai-future, 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 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87053 for commit 59bffc322629.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Queued 3 failed job(s) for retry in Buildkite CI #87053.

@orozery

orozery commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

/ci retry

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Queued 2 failed job(s) for retry in Buildkite CI #87053.

@orozery

orozery commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

/ci retry

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Queued 2 failed job(s) for retry in Buildkite CI #87053.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87175 for commit 19b60ecf97b4, running 2 failed step(s) from Buildkite CI #87053.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 1fd4abdc-0183-4edd-9316-745a43a58f08

📥 Commits

Reviewing files that changed from the base of the PR and between a1541f5 and 85c8615.

📒 Files selected for processing (2)
  • tests/v1/kv_offload/tiering/test_async_lookup.py
  • vllm/v1/kv_offload/tiering/async_lookup.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • vllm/v1/kv_offload/tiering/async_lookup.py
  • tests/v1/kv_offload/tiering/test_async_lookup.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Prevented outdated or canceled lookup requests from being submitted during asynchronous flushing.
    • Ensured lookup batches are cleared and validated before processing, avoiding unnecessary batch operations and reducing redundant background work.
    • Improved reliability when lookups are cleaned up before a flush occurs, preventing stale requests from being processed.
    • Added regression coverage to verify that cleaned-up lookups are not submitted during subsequent flushes.

Walkthrough

flush() now discards removed or stale lookup entries before submitting a batch. Tests track batch lookup calls and verify that cleanup prevents submission.

Changes

Lookup batch filtering

Layer / File(s) Summary
Flush filtering and regression coverage
vllm/v1/kv_offload/tiering/async_lookup.py, tests/v1/kv_offload/tiering/test_async_lookup.py
flush() resets and filters the pending batch before queue submission. Tests count batch lookups and verify that cleanup prevents submission.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 85c86

Async KV lookup batching now skips entries removed during request cleanup or superseded by newer generations, avoiding unnecessary backend lookups without changing already queued or in-flight work. The supplied regression coverage supports merge readiness.

Suggested reviewers: orozery

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a bug fix that skips cleaned-up asynchronous KV offload lookup batches. It accurately summarizes the primary change.
Description check ✅ Passed The description directly explains the cleanup-before-flush bug, the state revalidation fix, the test plan, and the test results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Queued 1 failed job(s) for retry in Buildkite CI #87175.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87204 for commit 92930106f315, running 1 failed step(s) from Buildkite CI #87175.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87211 for commit 92930106f315.

@Alex-ai-future

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87230 for commit 8cd238b1a039.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@orozery

orozery commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

/ci run

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87431 for commit 85c8615d84e9.

@orozery
orozery merged commit 9afb878 into vllm-project:main Sep 6, 2026
91 checks passed
ItsRoy69 pushed a commit to ItsRoy69/vllm that referenced this pull request Sep 10, 2026
…ct#55075)

Signed-off-by: Alex <jihui.huang@daocloud.io>
Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: Or Ozeri <oro@il.ibm.com>
Signed-off-by: Jyotirmoy Roy <jyotirmoyroy649@gmail.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.

2 participants