Skip to content

[Bugfix][KV Cache] Emit BlockRemoved only after the last physical copy is evicted - #55092

Open
skajre wants to merge 1 commit into
vllm-project:mainfrom
skajre:feat/precise-block-removed-events
Open

skajre wants to merge 1 commit into
vllm-project:mainfrom
skajre:feat/precise-block-removed-events

Conversation

@skajre

@skajre skajre commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Purpose

A prefix-cache block hash can map to multiple physical KV cache blocks. For example, when an identical block-aligned prompt is served repeatedly, vLLM may reuse earlier prefix blocks but recompute the final prompt block to obtain next-token logits. The recomputed blocks have the same hash while retaining different physical block IDs.

Previously, evicting any one of these physical copies emitted a BlockRemoved event. An external KV event consumer could therefore invalidate the hash even though another physical copy was still resident and available for prefix-cache lookup.

For example, consider two cached requests:

Request A: H1 -> H2 -> H3
Request B: H1 -> H2 -> H3 -> H4 -> H5 -> H6 -> H7

Suppose H3 is backed by two physical blocks, P3-A and P3-B. Evicting P3-A previously emitted BlockRemoved(H3) even though P3-B remained resident. The external cache view could then appear as:

H1, H2, H4, H5, H6, H7

Because block hashes form a prefix chain, a new request matching H1 -> ... -> H7 would be scored as a two-block cache hit rather than the seven-block hit actually available in the engine. A prefix-aware router could consequently choose the wrong engine, or trigger unnecessary recomputation or remote loading of H3 through H7.

This PR makes BlockRemoved a logical cache-key disappearance event:

  • Remove the requested physical block from BlockHashToBlockMap and determine how many copies remain under the same BlockHashWithGroupId key.
  • Suppress BlockRemoved while at least one same-hash physical copy remains.
  • Emit the existing BlockRemoved event only after the last physical copy in that KV cache group disappears.
  • Keep copy accounting internal to BlockPool; the event schema and ZMQ wire format remain unchanged.
  • Preserve group isolation. Copies are scoped by (block_hash, group_idx).

The internal removal result remains available to hash relocation paths such as move_block_hashes, so all hashes formerly owned by the source physical block are re-pointed correctly.

Test Plan

pytest -q \
  tests/v1/core/test_prefix_caching.py \
  tests/v1/core/prefix_cache/test_partial_prefix_cache_primitives.py \
  tests/distributed/test_kv_cache_events.py \
  tests/distributed/test_events.py \
  tests/v1/kv_connector/unit/offloading_connector/test_events.py

ruff check \
  vllm/v1/core/block_pool.py \
  tests/v1/core/test_prefix_caching.py

ruff format --check \
  vllm/v1/core/block_pool.py \
  tests/v1/core/test_prefix_caching.py

git diff --check

End-to-end test:

  1. Start a single-GPU Qwen3-8B full-attention server with prefix caching, block size 16, and an 8-block GPU cache.
  2. Send three identical sequential 32-token completion requests without prompt_logprobs, producing three physical copies of the final block hash.
  3. Send twelve unique 32-token requests to force eviction.
  4. Consume events using the existing BlockRemoved schema.

Test Result

156 passed, 14 warnings in 26.84s
All checks passed
2 files already formatted
git diff --check: passed

End-to-end result:

target_blockstored_before_pressure=3
target_blockremoved_before_pressure=0
target_blockremoved_after_pressure=1
wire_remaining_copy_counts_occurrences=0
subscriber_decode_errors=0

Observed target event stream:

BlockStored(target_hash)  x 3
BlockRemoved(target_hash) x 1

No documentation update is required because this change preserves the public event schema and strengthens the existing BlockRemoved semantics.

Signed-off-by: Jinjie Shao <shaojinjiesjj@gmail.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 bug Something isn't working kv-cache-manager labels Sep 3, 2026
@ai-jz

ai-jz commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Hi @skajre — thanks for working on this. Emitting BlockRemoved only after the last physical copy is evicted looks important for keeping KV-event consumers accurate.

Are you planning to continue this PR? I’d be happy to help with testing or follow-up changes if useful.

@skajre

skajre commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi @skajre — thanks for working on this. Emitting BlockRemoved only after the last physical copy is evicted looks important for keeping KV-event consumers accurate.

Are you planning to continue this PR? I’d be happy to help with testing or follow-up changes if useful.

Thanks for reaching out! Yes, I plan to continue working on this PR and move the feature forward. I’ve already run the relevant tests around the BlockRemoved behavior, and the results look good so far. I’d be very happy to collaborate—additional testing or help with follow-up changes would be greatly appreciated. I’ll keep the PR updated as I make progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working kv-cache-manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants