Skip to content

[Bugfix][KV Offload] Store the chunk of a loaded partial tail - #61375

Open
ntovas wants to merge 1 commit into
vllm-project:mainfrom
ntovas:kv-offload-store-partial-tail-chunk
Open

ntovas wants to merge 1 commit into
vllm-project:mainfrom
ntovas:kv-offload-store-partial-tail-chunk

Conversation

@ntovas

@ntovas ntovas commented Oct 13, 2026 •

Copy link
Copy Markdown

Overview

After a load that ends in a partial tail, the OffloadingConnector scheduler never stores the chunk that holds the tail. Each later lookup of the same prefix stops at the same partial tail, and the request computes all tokens after it again. This PR starts the store of a full-attention group at the chunk of the partial tail.

Related: #61019 (draft) changes the copy-on-write hand-off store of partial tails with EAGLE/MTP groups. It does not change next_stored_chunk_idx, and its head (3276fdd) merges with this PR without a conflict.

Duplicate check: I searched the open and closed issues and PRs for next_stored_chunk_idx, "partial tail" and "offload". #56795, #52771 and #51761 report other defects of the store index. No issue or PR reports this case.

AI assistance was used for this PR (Claude Code): the root-cause analysis, the patch, the test and this description. I reviewed every changed line and the test results.

Claims

  • After a load that ends in a partial tail, the connector stores the chunk of the tail when the chunk is full. The next lookup of the prefix then goes past it.
  • On a hybrid MLA + KDA model with partial tails and 32 concurrent agent sessions, the prompt tokens left to compute after the cache hits fall from 12,220 to 3,613 per warm turn, and the output throughput rises from 317 to 515 tok/s.

Validation

Unit test (new): test_partial_tail_load_keeps_tail_chunk_storable loads 28 tokens with 16-token chunks. Chunk 0 is a full hit, and chunk 1 holds the partial tail. The test checks that the full-attention group starts its store at chunk 1, and that the Mamba group keeps the skip.

pytest tests/v1/kv_connector/unit/offloading_connector/test_scheduler.py \
  -k test_partial_tail_load_keeps_tail_chunk_storable
# main:     1 failed (AssertionError: assert 2 == 1)
# this PR:  1 passed

pytest tests/v1/kv_connector/unit/offloading_connector/
# this PR:  564 passed, 2 skipped (the two skipped tests need a GPU backend)

ruff check, ruff format --check, typos and tools/pre_commit/mypy.py 3.11 pass on the changed files. CI runs these tests in the v1/kv_connector/unit step.

End to end, on an older base: nightly 21d93d0 (2026-10-05). The lines that this PR changes are the same in that base and in main. Both runs have the same other local patches.

  • Model: GLM-5.3-Flash (MLA and KDA layers; the KDA groups use Mamba cache mode align), W4A16 checkpoint with BF16 activations, 2× H200, TP=2, no expert parallelism.
  • MTP with 3 probabilistic draft tokens, block verification and disable_eagle_block_drop.
  • --prefix-match-unit 64. The attention block is 2,304 tokens, so most loads end in a partial tail.
  • OffloadingConnector with a 330 GiB CPU tier and no disk tier.
  • Load: simulated coding-agent sessions, 16 and then 32 concurrent sessions, 8 minutes per level. Each session starts new, and its context grows from about 75K to 140K tokens (6K shared prefix). One run per configuration; the run-to-run difference of this load is about ±1 % at 16 and ±3 % at 32 sessions.
Metric (16 / 32 sessions) Without the fix With the fix
Output tok/s 474 / 317 472 / 515
Computed prefill tokens per request 6,561 / 19,597 6,603 / 9,213
Prompt tokens left after the cache hits, warm turns at 32 sessions 12,220 3,613
Prompt tokens from the CPU tier, 32 sessions 74.7 % 89.2 %
Warm TTFT p50 0.9 / 5.9 s 0.9 / 2.1 s

Without the fix, the CPU-tier hit of a session at 32 sessions stays at one boundary for several turns. For example, it stays at 114,752 tokens for prompts of 138K, 139K, 144K and 145K tokens, and each of these turns computes 24K–30K tokens again. At 16 sessions the computed prefill does not change (6,561 and 6,603 tokens per request).

Details

Root cause: update_state_after_alloc sets num_chunks = cdiv(num_cached_tokens, tokens_per_chunk). This count includes the chunk of a partial tail. For the chunk-level policy, next_stored_chunk_idx = num_chunks then treats that chunk as a hit chunk. The load fills the chunk only up to the tail boundary, and the request computes the rest of it. The store starts at the next chunk, so the full chunk is never stored, and its key never becomes a hit. Example: a load to 114,752 tokens with 2,304-token chunks gives num_chunks = 50, and the store skips chunk 49.

Fix: for a full-attention group (sliding_window_size_in_chunks is None), the store starts one chunk earlier when the load ends in a partial tail. Sliding-window, chunked-local and Mamba groups keep the skip: their block for that chunk can hold the tail state, which is not the state at the chunk end. The Mamba groups store their boundary states through the copy-on-write path (requires_cow_source).

Scope: the change has an effect only when supports_partial_tail is true (a Mamba group in align mode, one block per chunk) and a load ends in a partial tail. In all other cases next_stored_chunk_idx is unchanged.

Cost: one more chunk store per load that ends in a partial tail. The CPU tier did not have that chunk before.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (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.

Generated with Claude Code

After a load that ends in a partial tail, update_state_after_alloc sets
next_stored_chunk_idx to cdiv(num_cached_tokens, tokens_per_chunk). This
skips the chunk that holds the partial tail. The request then computes
the rest of that chunk, but the connector never stores it. The next
lookup of the same prefix stops at this chunk, finds the same partial
tail again, and the request computes all tokens after it again.

For a full-attention group, start the store at the chunk of the partial
tail. Sliding-window and Mamba groups keep the skip: their block for that
chunk can hold the tail state, which is not the state at the chunk end.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Giannis Ntovas <ntovas.gi@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-connector labels Oct 13, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@mergify

mergify Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @ntovas.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 29, 2026

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

Rebased locally on 971378f466: scheduler.py merges cleanly; the only conflict is in test_scheduler.py, where main added test_sync_partial_tail_load_targets_hashed_blocks and test_sync_load_budget_is_shared_per_step at the same spot, and keeping both sides resolves it. On main the new test still fails (assert 2 == 1); after the rebase it passes, and tests/v1/kv_connector/unit/offloading_connector/ gives 679 passed, 2 skipped.

The test asserts next_stored_chunk_idx, not the store. With the 30-token helper request each group has one key, so chunk 1 can't be stored in that geometry. A 40-token variant that loads the partial tail, steps to token 40, and asserts the full-attention group stores its chunk-1 key from block 32 fails on main and passes here, also with eagle=True and with a sync load. I'd assert the store that way.

@ILikeSahne

Copy link
Copy Markdown

Encountered the same Issue with Qwen3.8-27B (GDN hybrid) on vLLM 0.31.0, CPU offloading + --prefix-match-unit 16 using an RTX 5090. Once the GPU starts evicting, conversations get stuck at an old partial tail and recompute everything after it.

7 parallel conversations, 40k-60k context, 77 turns: 62 turns stuck and 551k tokens recomputed without the fix, 0 stuck and 581 tokens with it.

This branch has not been deployed

No deployments
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 needs-rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants