Skip to content

Conversation

@sdavidbd
Copy link
Contributor

@sdavidbd sdavidbd commented Nov 6, 2025

Purpose

Fixes #26675, following the approach suggested by @markmc in #27026.
This change temporarily disables graph capture for the KV connector path by skipping calls to KV connector layer APIs when the connector metadata is unset.

Test Plan

Reproduce the scenario described in #26675 (full-graph capture with KV connector).
Run with and without the fix applied.

Test Result

With the fix applied, the assertion reported in #26675 no longer reproduces.
Full-graph capture completes successfully with the KV connector enabled.

Signed-off-by: David Ben-David <[email protected]>
Co-authored-by: Mark McLoughlin <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively resolves a bug related to CUDA graph capture when using a KV connector. The core of the fix is to prevent calls to KV connector APIs when connector metadata is not set, which is the case during graph capture. This is cleanly implemented by introducing a has_connector_metadata method on the base connector class and adding checks in the attention layer. The accompanying change in MultiConnector to correctly propagate metadata state by calling the superclass methods is a crucial and well-spotted related fix. The changes are logical, well-contained, and directly address the issue described. The approach is sound, and I have no further comments.

@mergify mergify bot added the kv-connector label Nov 6, 2025
Copy link
Member

@markmc markmc left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

@markmc markmc added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 7, 2025
@heheda12345 heheda12345 merged commit cc07976 into vllm-project:main Nov 11, 2025
57 checks passed
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Nov 13, 2025
…vllm-project#28253)

Signed-off-by: David Ben-David <[email protected]>
Co-authored-by: David Ben-David <[email protected]>
Co-authored-by: Mark McLoughlin <[email protected]>
Signed-off-by: xuebwang-amd <[email protected]>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
…vllm-project#28253)

Signed-off-by: David Ben-David <[email protected]>
Co-authored-by: David Ben-David <[email protected]>
Co-authored-by: Mark McLoughlin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

[Bug]:AssertionError: assert self._connector_metadata is not None when using graph mode

3 participants