Skip to content

[SimpleCPUOffloadConnector]: Add KV Events - #46397

Open
jonathanc-n wants to merge 1 commit into
vllm-project:mainfrom
jonathanc-n:add-kv-event
Open

jonathanc-n wants to merge 1 commit into
vllm-project:mainfrom
jonathanc-n:add-kv-event

Conversation

@jonathanc-n

@jonathanc-n jonathanc-n commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Purpose

Add KV events.

Changes

Add .medium parameter to specify what medium the KV events are being reported from (defaults to gpu)

Comment on lines +760 to +761
parent_block_hash=None,
token_ids=[],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QQ: can we prepare them correctly rather than skipping them? Would that be super complex?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we need to populate parent_block_hash and token_ids, such that external routers like dynamo can consume the cpu events correctly
(see #43468)

@@ -131,6 +132,7 @@ def __init__(
hash_block_size=self.hash_block_size,
)
self.cpu_block_pool: BlockPool = self.cpu_coordinator.block_pool
self.cpu_block_pool.medium = "CPU"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's add the definition in vllm/distributed/kv_events.py?

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.

@zhewenl @jonathanc-n #46544 already adds MEDIUM_CPU = "CPU" to
vllm/distributed/kv_events.py (next to MEDIUM_GPU) — the definition you're
asking for here. Centralizing the wire medium names there is the direction
@orozery suggested in #38260, so once #46544 lands this PR can just import
MEDIUM_CPU instead of the "CPU" literal. Disjoint files otherwise, no conflict.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense thanks

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.

@jonathanc-n @zhewenl Hi, #46544 has been merged today. You can safely use MEDIUM_CPU in vllm/distributed/kv_events.py now.

Comment on lines +760 to +761
parent_block_hash=None,
token_ids=[],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we need to populate parent_block_hash and token_ids, such that external routers like dynamo can consume the cpu events correctly
(see #43468)

@mergify

mergify Bot commented Jul 10, 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, @jonathanc-n.

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 Jul 10, 2026
Signed-off-by: Jonathan <chenleejonathan@gmail.com>
@mergify

mergify Bot commented Jul 20, 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, @jonathanc-n.

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.

4 participants