Skip to content

[Bugfix][KV Connector] Fix DecodeBenchConnector prefix block selection - #54878

Merged
ivanium merged 3 commits into
vllm-project:mainfrom
majunze2001:fix/decode-bench-prefix-block-selection
Sep 4, 2026
Merged

ivanium merged 3 commits into
vllm-project:mainfrom
majunze2001:fix/decode-bench-prefix-block-selection

Conversation

@majunze2001

@majunze2001 majunze2001 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Purpose

DecodeBenchConnector currently selects the first ceil(num_external_tokens / block_size) blocks in every KV cache group. That is only correct when a request has no local prefix-cache hit. When a local prefix is already computed, the external-token interval follows that prefix, but the connector still fills from block zero. It can therefore overwrite shared prefix-cache blocks and leave the newly allocated suffix blocks unfilled.

Select the block interval that actually covers the externally computed tokens, using each KV cache group's effective block size. Keep all fixed per-request state for CircularBufferSpec groups because those buffers are not indexed by absolute token position, and exclude null padding blocks before sending block IDs to the worker.

The tests cover a cached prefix with different block IDs per group, an unaligned interval start, retained circular-buffer state, and null padding in a sliding-window group.

No public API or configuration behavior changes, so no documentation update is needed.

Duplicate-work check

No linked issue exists for this bug. The following searches found no open PR or issue addressing DecodeBench's external-suffix block selection after a local prefix-cache hit:

gh pr list --repo vllm-project/vllm --state open \
  --search 'DecodeBenchConnector'
gh pr list --repo vllm-project/vllm --state open \
  --search 'DecodeBench prefix cache'
gh pr list --repo vllm-project/vllm --state open \
  --search 'DecodeBench block selection'
gh issue list --repo vllm-project/vllm --state open \
  --search 'DecodeBench prefix cache'

The broad connector search returned #54734, #53695, and #40795. Those PRs cover connector capability validation, ROCm unified-attention support, and DP load balancing respectively; none changes DecodeBench prefix block selection. The subsequently merged #54879 makes worker-side dummy filling safe for CircularBufferSpec, but it retains the front-slice selection that this PR fixes. The two changes are complementary.

Test Plan

Run the focused connector suite, pre-commit hooks for the changed files, and a diff whitespace check:

.venv/bin/python -m pytest \
  tests/v1/kv_connector/unit/test_decode_bench_connector.py -v

.venv/bin/pre-commit run --files \
  vllm/distributed/kv_transfer/kv_connector/v1/decode_bench_connector.py \
  tests/v1/kv_connector/unit/test_decode_bench_connector.py

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

Run OpenAI-compatible serving E2Es with prefix caching and DecodeBench enabled:

VLLM_LOGGING_LEVEL=DEBUG .venv/bin/vllm serve <model> \
  --enable-prefix-caching \
  --kv-transfer-config \
    '{"kv_connector":"DecodeBenchConnector","kv_role":"kv_both"}'

For each server, issue forward and reverse shared-prefix requests, replay an exact prompt, and overlap a long producer request with a shorter consumer. Test full attention with Qwen/Qwen3-0.6B, hybrid GDN/full attention with Qwen/Qwen3.5-0.8B, and circular-buffer execution with Qwen/Qwen3.8-Flash-Next-FP8.

For the targeted Qwen3.5 hybrid prefix-hit run, add --prefix-cache-retention-interval 544 --enable-prompt-tokens-details, let the seed request decode 220 tokens to create a locally computed aligned checkpoint, and submit the returned prompt and generated token IDs as two subsequent prompts. Use temporary debug-only logging to compare each group's scheduler-selected physical block IDs with the IDs received by the worker, then remove the logging after the run.

Test Result

  • test_decode_bench_connector.py: 15 passed. The suite includes a scheduler-level prefix-cache regression test plus explicit checks for the prefix offset, unaligned start rounding, circular-buffer retention and safe filling, null-padding exclusion, DCP geometry, and multi-group worker routing.

  • Pre-commit passed on both changed files, including Ruff, formatting, mypy, SPDX, forbidden-import, and configuration-validation hooks.

  • git diff --check origin/main...HEAD passed.

  • Qwen/Qwen3-0.6B full-attention serving E2E: 7/7 requests returned HTTP 200. Forward/reverse shared prefixes, exact replay, and concurrent producer/consumer requests completed. The initial request filled 102 blocks for 1,620 external tokens; reuse requests selected one suffix block for four external tokens, and the prefix-cache hit rate reached 56.8%. The server remained healthy after the request matrix.

  • A second Qwen/Qwen3-0.6B serving E2E used temporary debug-only logging in the test checkout to record the scheduler's positional selection and the worker-received IDs. The probe did not alter selection or filling and was removed after the run. All three requests returned HTTP 200:

    Request External token range Block positions Selected IDs Worker IDs
    Seed [0, 1620) [0, 102) [1, ..., 102] [1, ..., 102]
    Prefix reuse [1616, 1620) [101, 102) [102] [102]
    Exact replay [1616, 1620) [101, 102) [102] [102]

    All three requests had the same 102 allocated block IDs ([1, ..., 102]). The reuse and replay traces therefore demonstrate that the connector skipped the 101 cached-prefix blocks, selected physical suffix block 102, and sent that same ID to the worker. The normal worker log then reported one block and four external tokens filled for each request. The old front-slice would have selected physical block 1. The server remained healthy after the probe.

  • Qwen/Qwen3.5-0.8B general hybrid serving E2E: 7/7 requests returned HTTP 200. All four cache groups were filled with per-group counts (1, 1, 1, 3), and the shared-prefix/replay/concurrent request matrix completed without a CUDA error. This baseline validates multi-group execution.

  • Qwen/Qwen3.5-0.8B targeted hybrid prefix-hit E2E: 3/3 requests returned HTTP 200. A 3,088-token seed decoded 220 tokens to create a locally computed checkpoint at token 3,264. Both 3,308-token follow-up requests resumed from that checkpoint, leaving the external interval [3264, 3307). For every cache group, the scheduler selected only block position [6, 7), the worker received the same physical ID selected for that group, and DecodeBench filled one block per group for the remaining 43 external tokens. The local prefix-cache hit rate reached 67.3%, the final health check passed, and the temporary debug-only logging was removed after the run.

  • Qwen/Qwen3.8-Flash-Next-FP8 circular-buffer serving E2E with DecodeBench's default fill configuration: 5/5 requests returned HTTP 200. A short request, 512-token decode, 1,024-token decode, and concurrent 512/128-token requests all reached finish_reason="length". DecodeBench selected all six cache groups, filled the circular-buffer group with the representation-safe zero from [Bugfix][KV Connector] Safely fill circular buffers in DecodeBench #54879, and retained the configured 0.015 fill for the other five groups. The server remained healthy after the workload with no request-time illegal memory access or HTTP error.

Model-quality evaluation is not applicable. DecodeBench intentionally fills KV caches with dummy values for decoder performance testing, so generated text has no semantic-accuracy expectation. The real-model E2Es instead verify request completion, local-prefix reuse, per-group filling, and circular-buffer execution.

AI assistance disclosure

OpenAI Codex was used for this PR.

Select the per-group blocks covering externally computed tokens after a local prefix-cache hit, while retaining fixed circular-buffer state.

Co-authored-by: OpenAI Codex <noreply@openai.com>
Signed-off-by: Jeff Ma <jeffjma@umich.edu>
@mergify mergify Bot added bug Something isn't working kv-connector labels Sep 2, 2026
…ix-block-selection

Signed-off-by: Jeff Ma <jeffjma@umich.edu>

# Conflicts:
#	tests/v1/kv_connector/unit/test_decode_bench_connector.py
@coderabbitai

coderabbitai Bot commented Sep 3, 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: 175fbe74-42d9-4fb6-900a-0fd69ce7ffa6

📥 Commits

Reviewing files that changed from the base of the PR and between ba9c7a3 and 3b44404.

📒 Files selected for processing (2)
  • tests/v1/kv_connector/unit/test_decode_bench_connector.py
  • vllm/distributed/kv_transfer/kv_connector/v1/decode_bench_connector.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
    • Improved external block selection across cache groups, including unaligned intervals and per-group block sizes.
    • Corrected prefix-cache handling to fill only missing suffix blocks.
    • Improved sliding-window cache handling by excluding null padding blocks.
    • Added validation for allocated block ranges and more accurate block-group reporting.
    • Improved handling of circular-buffer cache groups.

Walkthrough

The decode benchmark connector now selects external KV blocks per cache group, validates token boundaries, supports circular-buffer groups, excludes null blocks, and logs allocated group counts. Tests cover prefix suffix filling, group-specific block sizes, unaligned intervals, hashed metadata, and sliding-window padding.

Changes

External KV block selection

Layer / File(s) Summary
Per-group block selection
vllm/distributed/kv_transfer/kv_connector/v1/decode_bench_connector.py
The scheduler stores KV cache groups, derives group block sizes, validates processed-token ranges, handles circular-buffer groups, excludes null blocks, and updates selection logging.
Selection validation coverage
tests/v1/kv_connector/unit/test_decode_bench_connector.py
Tests cover prefix suffix filling, per-group block sizes, unaligned external intervals, group-specific block hashes, and null padding in sliding-window groups.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 3b444

This change corrects external KV-block selection for locally cached prefixes across cache-group geometries, including circular and sliding-window cases. Current coverage indicates the intended behavior is ready to merge.

Sequence Diagram(s)

sequenceDiagram
  participant DecodeBenchConnectorScheduler
  participant KVCacheGroups
  participant AllocatedBlocks
  DecodeBenchConnectorScheduler->>KVCacheGroups: Read per-group block sizes
  DecodeBenchConnectorScheduler->>AllocatedBlocks: Validate processed-token boundary
  AllocatedBlocks-->>DecodeBenchConnectorScheduler: Return allocated block metadata
  DecodeBenchConnectorScheduler->>AllocatedBlocks: Select external blocks and exclude null blocks
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files.
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.
Title check ✅ Passed The title clearly and concisely identifies the main change: fixing DecodeBenchConnector prefix block selection in the KV connector.
Description check ✅ Passed The description directly explains the prefix-cache block-selection bug, the implemented fix, affected behavior, tests, and validation results.

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.

@majunze2001
majunze2001 marked this pull request as ready for review September 3, 2026 17:15

@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.

Signed-off-by: Jeff Ma <jeffjma@umich.edu>
@ivanium
ivanium enabled auto-merge (squash) September 3, 2026 20:19
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 3, 2026
@ivanium

ivanium commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

/ci run

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87138 for commit 3b44404abf10.

@ivanium
ivanium merged commit e352986 into vllm-project:main Sep 4, 2026
29 checks passed
ItsRoy69 pushed a commit to ItsRoy69/vllm that referenced this pull request Sep 10, 2026
vllm-project#54878)

Signed-off-by: Jeff Ma <jeffjma@umich.edu>
Co-authored-by: OpenAI Codex <noreply@openai.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 kv-connector 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