Skip to content

[Bugfix] Fix KV cache pages for hybrid models with draft layers - #62800

Open
seyfal wants to merge 12 commits into
vllm-project:mainfrom
eagle-hq:upstream-pr/kv-cache-pages-hybrid-draft
Open

seyfal wants to merge 12 commits into
vllm-project:mainfrom
eagle-hq:upstream-pr/kv-cache-pages-hybrid-draft

Conversation

@seyfal

@seyfal seyfal commented Oct 21, 2026 •

Copy link
Copy Markdown

Overview

This PR fixes how vLLM picks KV cache pages when a model mixes layer types, which is what speculative decoding with a draft model on a hybrid (attention plus linear-attention) target does. FlashInfer now chooses kernel pages per layer, padded manager pages can split into kernel pages, and hybrid block alignment counts the drafter's KV. It is independent of #62961, #62962, #62963 and #62964, and of #63279, which carries the FA4 dense warmup coverage that started in this PR.

Claims

  • FlashInfer kernel pages follow each attention group's own head counts, with query heads from its layers and KV heads from its spec. An MHA draft layer next to a GQA target otherwise gets the target's large pages, and its metadata builder rejects them at startup. AttentionBackend.get_supported_kernel_block_sizes_for_layers(kv_cache_spec, layer_names) carries this and defaults to get_supported_kernel_block_sizes. The model runner and the NIXL and Mooncake connectors pass each group's layer names, so P/D transfer uses the same kernel blocks as the runner.
  • A padded manager page splits into uniformly strided kernel pages, and manager-page boundaries are kept. Interleaved layers and non-divisible padding still fail with a clear error.
  • Hybrid block alignment includes the drafter's KV bytes per token. When a draft has more KV per token than the target, it sets the common page size, and every recurrent-state page is padded up to it. For a 64-layer Qwen3.5-style GDN model with a one-layer draft that is a 5.2x padding, and the KV pool holds about 61 concurrent requests of 2.5k tokens.

Validation

AI assistance (Claude) was used to prepare this change; I reviewed every changed line and ran the tests listed below.

The changed test files cover per-layer head counts and connector grouping, the attention utilities, the kernel block selection and the GPU model runner. I broke the head-count fix on purpose to check that a test catches it. On B300, making FlashInfer ignore the layer's heads fails the MHA-draft case.

On CPU (VLLM_TARGET_DEVICE=cpu, one pytest process per file), I ran the attention, KV-cache, spec-decode and worker files this PR touches or depends on.

Build Files Passed Failed Skipped
main (98468d0f66) 84 1335 143 957
this PR 84 1347 143 961

The 84 files include the NIXL and Mooncake connector tests. The 143 failures, and 35 errors at collection, are the same cases on both builds, and pre-commit passes on the range.

On B300 with FlashInfer installed, the FlashInfer and connector cases in test_group_head_counts.py pass (5 of 5), and test_attn_utils.py passes 62 of 62.

I served Qwen3.8-27B at TP4 on 4x B300 with this PR on main (98468d0f66), with the default attention backend (FlashInfer), VLLM_USE_V2_MODEL_RUNNER=1 and VLLM_GPU_SYNC_CHECK=error, with 33 SPEED-Bench prompts (greedy, 384 tokens each). The server logged no GPU-sync errors. At batch 1 the outputs are bitwise identical to main (33 of 33) and decode speed matches (226.8 tok/s against 226.0, one server each). At 16 concurrent, two servers of main itself agree on 22 of 33 outputs on this base, so equality there is not a usable check. This PR against main gives 23 of 33 in the same job, which is within that spread.

End to end, a chain speculator with an MHA draft on a GQA hybrid target starts with the default attention backend, where without this PR it starts only when forced to FLASH_ATTN, and it serves concurrent and mid-prefill (16k-token) requests. A TP8 hybrid target with a draft serves 20 of 20 requests within the near-tie criterion against no speculation. I ran these two on the previous base (56e67b46a3), before the head-count fix moved from the cache spec to the backend method.

pre-commit run --from-ref origin/main --to-ref HEAD
VLLM_TARGET_DEVICE=cpu python -m pytest -q tests/v1/attention/test_group_head_counts.py tests/v1/attention/test_kpool_indexer_block_sizes.py tests/v1/worker/test_attn_utils.py tests/v1/worker/test_gpu_model_runner.py
python -m pytest -q tests/v1/attention/test_group_head_counts.py tests/v1/worker/test_attn_utils.py   # on B300
VLLM_USE_V2_MODEL_RUNNER=1 VLLM_GPU_SYNC_CHECK=error vllm serve Qwen/Qwen3.8-27B --tensor-parallel-size 4 --max-model-len 8192 --max-num-seqs 16 --no-enable-prefix-caching

Not a duplicate

I ran the duplicate-work searches from AGENTS.md §1 (open PRs, with keywords on kernel block size, padded pages, hybrid drafter block size and Mamba page padding). They found related work, and none of it makes the same fix. This PR uses no code from them.

Includes and credits

This PR contains no code from other open PRs.

Details

  • Query heads stay out of the cache spec, which describes only the KV cache layout (as in [Bugfix][Spec Decode] Use group geometry for FlashAttention metadata #53002). FlashInfer reads them from the group's layers when kernel blocks are chosen at KV cache initialization. Callers without layer names, and calls with no current config, keep the model-wide answer, so other models pick the same pages as before.
  • The alignment change builds a fresh spec for the draft, so the draft does not inherit the target's value-head dimension. Speculative-state reservations are unchanged.
  • All GPU measurements are on B300.

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.

@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 nvidia mrv2 Model Runner V2 specific bug Something isn't working labels Oct 21, 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.

🚀

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

This PR mixes two independent changes: the KV-cache bugfixes and the FA4 warmup coverage. I’d suggest splitting it into two PRs.

# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
from itertools import product
from types import SimpleNamespace

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.

I think we can drop this test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Dropped the CPU key-listing tests and kept only the native compilation-cache test, which is the one that catches the large-page table bug. It's in #63279

# Large batches use a separate varlen batch-lookup specialization.
batch_sizes = WarmupChoices(2, vllm_config.scheduler_config.max_num_seqs)
split_counts = WarmupChoices(1, 2)
causal_modes = WarmupChoices(True, False)

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.

Are the causal=False warmups ever used? get_warmup_keys only accepts FullAttentionSpec and SlidingWindowSpec

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, these specs always run causal attentio. Removed causal from the warmup key. Fixed in #63279

window_size: tuple[int, int],
softcap: float,
is_single_batch: bool = True,
batch_size: int = 1,

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.

On the generic branch, batch_size (and max_seqlen_q) are raw values in the CompileKey, but FA4 doesn’t key on either. It keys on derived flags, e.g., q_stage/is_static_persistent, and the batch-lookup flags (cu_total_m_blocks/blocks_to_batch_idx, set when batch_size > BIN_BATCH_SEARCH_THRESH).

As written, WarmupChoices(2, max_num_seqs) duplicates a kernel whenever max_num_seqs <= 256 and quietly depends on a private FA threshold.

The key should use these flags directly, as the previous q_stage/is_single_batch key did, with concrete shapes picked only in compile(). For example, use_batch_lookup: bool, warmed only when max_num_seqs > BIN_BATCH_SEARCH_THRESH (imported from FA, not copied).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. The key now holds FA4's own flags (q_stage, is_static_persistent, use_batch_lookup) and compile() picks the shapes, with the batch lookup warmed only when max_num_seqs exceeds BIN_BATCH_SEARCH_THRESH imported from FA4. Each key compiles a distinct kernel (5 from 5 keys at max_num_seqs=256, 8 from 8 at 1024), in #63279

@seyfal seyfal changed the title [Bugfix] Fix KV cache pages for hybrid models with draft layers and FA4 warmup page tables [Bugfix] Fix KV cache pages for hybrid models with draft layers Oct 23, 2026
@seyfal

seyfal commented Oct 23, 2026

Copy link
Copy Markdown
Author

Thanks, I split it. #62800 keeps the KV cache page fixes (per-layer FlashInfer kernel pages, padded page splits and the drafter's KV in hybrid block alignment), and the FA4 dense warmup coverage moved unchanged to #63279. Neither depends on the other, so they can land in any order.

@mergify

mergify Bot commented Oct 26, 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, @seyfal.

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 26, 2026
@seyfal
seyfal force-pushed the upstream-pr/kv-cache-pages-hybrid-draft branch from aedb33b to fbae492 Compare October 26, 2026 23:58
@mergify mergify Bot removed the needs-rebase label Oct 26, 2026
FlashInfer large kernel pages require GQA on Blackwell. Selecting them from the target model configuration also assigned them to MHA draft layers, whose metadata builder then rejected the page size. Carry query-head geometry as non-layout cache metadata and refresh it when either runner forms attention groups. Use the per-layer Q/KV ratio for page selection, retaining conservative small pages without config or known query geometry.

Test full and sliding-window specs plus MHA/GQA, TP-sized heads, unknown geometry and missing config. Mutation checks fail when either geometry publication or backend selection is reverted.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Distribute explicit manager-page padding evenly over kernel subblocks instead of rejecting every padded split. Keep manager-page boundaries and reject interleaved layers and nondivisible padding. Test the TP8 hybrid page geometry, padding isolation and manager-block copies.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Register dense FA4 warmup for all model families and cover packed-query staging, persistent and non-persistent prefill, SplitKV, and the configured maximum batch. Keep the existing specialized hd256 and SM90 hd512 paths unchanged.

Normal Blackwell dense attention previously relied on dummy graph shapes, leaving short two-stage and SplitKV prefills to compile during inference. Add CPU regression coverage and a native compilation-cache test. Runtime attention dispatch is unchanged.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Use ceiling division for the synthetic block-table capacity. A page larger
than the representative key length otherwise creates an empty table with
a broadcast batch stride, warming a different native specialization from
serving.

Extend the existing regressions to non-dividing and larger-than-context
pages, including the native forward compilation cache. Runtime attention
dispatch and arithmetic are unchanged.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Reverts e993e19 and 73d0962. Review asked to keep the FA4 dense
warmup coverage apart from the KV cache page fixes, so those two commits
move unchanged to a separate PR based on current main, and this PR keeps
only the KV cache page fixes.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Upstream main now type-checks tests/v1/attention, and
Attention.get_kv_cache_spec returns KVCacheSpec | None. Assert the
AttentionSpec type before reading its head counts.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
@seyfal
seyfal force-pushed the upstream-pr/kv-cache-pages-hybrid-draft branch from fbae492 to 8fbddda Compare October 27, 2026 00:21

@TheEpicDolphin TheEpicDolphin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although tracking num_query_heads in AttentionSpec is a straightforward way to address the issue for FlashInfer, I don't think it's the right long-term solution. KVCacheSpec properties should describe the memory layout of the KV cache, and q heads have no effect on that. It also gets dropped when specs are merged, since FullAttentionSpec.merge doesn't carry it over. #53002 avoided adding q heads to the spec for the same reason.

Metadata builders already get the number of q heads for an attention group from get_num_attention_heads_from_layers, which takes a list of layer names. I think the best direction is a get_supported_kernel_block_sizes_for_layers(kv_cache_spec, layer_names) method on AttentionBackend that defaults to get_supported_kernel_block_sizes, with FlashInfer overriding it to derive the num q heads. What do you think about this direction?

@mergify

mergify Bot commented Nov 3, 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, @seyfal.

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 Nov 3, 2026
@seyfal

seyfal commented Nov 4, 2026

Copy link
Copy Markdown
Author

@TheEpicDolphin Makes sense, I'll switch to that. I'll also take the KV heads from the spec while I'm in there, because the current check reads them from the model config too, so the draft gets the target's counts. I'll push it together with the rebase.

Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
Query heads describe the attention kernel, not the KV cache layout, so
they no longer live on AttentionSpec. AttentionBackend gets
get_supported_kernel_block_sizes_for_layers(kv_cache_spec, layer_names),
which defaults to get_supported_kernel_block_sizes. Kernel block
selection passes each attention group's layer names, and FlashInfer
takes the query heads from those layers and the KV heads from the spec.
Without matching layers it keeps the model-wide head counts.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
…oups

The NIXL and Mooncake connectors pick one kernel block size for the
transferred blocks. They now group layers by backend, spec and query
heads, as the model runner does, and pass each group's layer names to
select_common_block_size. Otherwise a FlashInfer draft layer that the
runner splits into small kernel pages would still be registered with
the target's large pages.

customize_attention_spec keeps its old signature: it runs before the
current vLLM config is set, so a layer name would not change its result.

Assisted-by: Claude
Signed-off-by: Seyfal Sultanov <seyfal.s@outlook.com>
@mergify mergify Bot added kv-connector mooncake Mooncake KV-transfer / EC-transfer nixl NIXL KV connector / multi-tiered KV offloading (KVCR) and removed needs-rebase labels Nov 4, 2026
@seyfal

seyfal commented Nov 5, 2026

Copy link
Copy Markdown
Author

@TheEpicDolphin Pushed. It now uses get_supported_kernel_block_sizes_for_layers, and the NIXL and Mooncake connectors pass the group layer names too, so P/D picks the same kernel blocks as the runner. PTAL when you get a chance.

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 mooncake Mooncake KV-transfer / EC-transfer mrv2 Model Runner V2 specific nixl NIXL KV connector / multi-tiered KV offloading (KVCR) nvidia

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants