Skip to content

[Bugfix][DCP] Recover failed KV loads across hybrid cache groups - #54733

Open
YukioZzz wants to merge 5 commits into
vllm-project:mainfrom
YukioZzz:yichaozhu/pr53917-scheduler-hybrid-load-recovery
Open

YukioZzz wants to merge 5 commits into
vllm-project:mainfrom
YukioZzz:yichaozhu/pr53917-scheduler-hybrid-load-recovery

Conversation

@YukioZzz

@YukioZzz YukioZzz commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Recover failed external KV loads for hybrid cache groups without assuming a single block table geometry.

This fixes the scheduler-side path that handles KV load failures. Hybrid models can have multiple cache groups with different physical block sizes, so recovery must inspect each group independently and roll the request back to the earliest common scheduler-safe boundary.

Relation To #53917

This PR is split out from the original full-stack #53917. The end-to-end Kimi-K3 SimpleCPU validation was run on the original #53917 final stack, but this PR is not on the normal AgentX/aiperf hit path. It only covers scheduler fallback behavior after an external KV load reports invalid blocks.

Why

The existing path still has a single-group assumption around failed block IDs. When an external load fails for a hybrid cache layout, the recovery path can crash or truncate the wrong state instead of safely recomputing from a valid boundary.

This is not SimpleCPU-specific; it applies to any KV connector that can report failed external loads for a hybrid model.

Correctness Notes

The scheduler receives a single num_computed_tokens value per request, so rollback must land on a common boundary that every KV cache group can represent. This PR scans each group's block table using the resolved manager block size, then rolls back to the earliest invalid token position rounded down to the scheduler block size.

If multiple requests share the same invalid block, only the first request marks that block for recomputation. Later requests that only reference already-marked invalid blocks roll back to their cached scheduler-block boundary, preserving the previous shared-block behavior.

The scheduler reads KVCacheManager.group_block_sizes and KVCacheManager.null_block_id instead of reaching into the coordinator and block pool internals directly. Null blocks are ignored during invalid-block matching and downstream eviction because the Mamba align block table can contain the shared null-block sentinel.

Validation

The same real FullAttention + Mamba align failure-recovery test was run on the base and this PR. It uses scheduler-allocated hybrid block tables and injects an async load failure for a Mamba state block.

base 890563368: ValueError: too many values to unpack (expected 1)
PR head:         passed and rolled back to the expected token boundary

Latest head:

head: d94e67fcf2891dbbd57090efda92d5c5949bd97c
python3 -m pytest -q tests/v1/kv_connector/unit/test_kv_load_failure_recovery.py
result: 15 passed

The recovery scan also ignores the Mamba null-block sentinel, and the focused hybrid eviction test verifies that downstream blocks are collected from every cache group after rollback.

Locate failed external-load blocks with each cache group's physical geometry. Truncate requests at the earliest common safe boundary and evict downstream blocks from every affected group.

The request exposes one cross-group computed-token count, so recovery rounds the earliest invalid position down to the scheduler block LCM. This can recompute a short tail for unitary layouts but prevents hybrid groups from resuming at incompatible boundaries.

Co-authored-by: andyluo7 <andy.luo@amd.com>
Signed-off-by: Yichao Zhu <Yichao.Zhu@amd.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 scheduler labels Sep 1, 2026
Signed-off-by: Yichao Zhu <Yichao.Zhu@amd.com>
Signed-off-by: Yichao Zhu <Yichao.Zhu@amd.com>
Expose group block sizes and the null block id through KVCacheManager so scheduler load-failure recovery does not reach through the block pool and coordinator internals directly.

Signed-off-by: Yichao Zhu <Yichao.Zhu@amd.com>
Clarify that load-failure recovery rolls back to a scheduler-block boundary, reuse the request id local, and make the hybrid geometry unit test derive its token counts from named block-size constants.

Signed-off-by: Yichao Zhu <Yichao.Zhu@amd.com>
@YukioZzz

YukioZzz commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

@orozery @njhill could you take a look at this scheduler fallback fix?
It fixes recovery after a connector reports invalid_block_ids for hybrid cache groups which is a TODO before.

@mergify

mergify Bot commented Sep 13, 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, @YukioZzz.

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant