feat(kv-router): Route vLLM CPU KV events to HostPinned and count lower-tier applies - #10368
Conversation
|
👋 Hi Change72! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
StorageTier::from_kv_medium now treats the bare string "CPU" as an alias for the canonical "CPU_PINNED" name. This lets KV-event publishers that emit medium="CPU" (notably vLLM's OffloadingConnector via CPULoadStoreSpec.medium()) route into the HostPinned tier on the router side, rather than silently falling back to the GPU/Device tier through from_kv_medium_or_default's default branch. Add three wire-contract tests in zmq_wire::tests: - cpu_medium_alias_routes_to_host_pinned_tier: pins the alias mapping and end-to-end tier classification via convert_event. - cpu_event_with_placeholder_payload_is_dropped_safely: defensive contract; underspecified BlockStored payloads (block_size=0, empty token_ids) produce zero indexable blocks and bump the unpublished- block warning counter, so the router never inserts garbage entries. - cpu_event_with_full_payload_is_indexable: happy path; fully-populated CPU BlockStored decodes to a HostPinned PlacementEvent with one indexable block per block_hashes entry. Existing 457 tests in dynamo-kv-router continue to pass. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Change72 <cguo51@asu.edu> Signed-off-by: Change72 <changg@nvidia.com>
The primary (Device) indexer increments `kv_cache_events_applied` on every event it applies, but `LowerTierIndexer::worker` previously took its `metrics` parameter as `_metrics` and dropped it on the floor. As a result, HostPinned/Disk/External traffic was invisible on `/metrics` even when CPU offload was firing and the routing pipeline was using it. Prebind the same `PreBoundEventCounters` the primary tier uses and call `.inc(kind, result)` after each Stored/Removed/Cleared apply on both `WorkerTask::Event` and `WorkerTask::EventWithAck` paths. Same counter, no new label introduced, so dashboards keep working as-is and the counter now reflects total events applied across all tiers. The trait parameter is intentionally kept as `_metrics` to minimize the diff surface against the existing `SyncIndexer::worker` signature; the underscore prefix is just the Rust convention to silence the "unused parameter" warning and using it from the body remains valid. `#[allow(clippy::used_underscore_binding)]` documents the choice for future readers. Verified end-to-end with `vllm/benchmarks/multi_turn/benchmark_serving_multi_turn.py` on L4 + Qwen3-0.6B + OffloadingConnector: the counter now ticks for HostPinned events as expected and the same workload reports ~2x more applied stored events compared to before this fix. cargo test -p dynamo-kv-router --lib: 476 / 476 pass. Signed-off-by: Change72 <changg@nvidia.com>
5b7725f made LowerTierIndexer record kv_cache_events_applied when given a metrics handle, but the construction sites never passed one: both LocalKvIndexer's lazily created per-tier indexers and the LowerTierIndexers registry built their ThreadPoolIndexer with the metrics-less constructor, so HostPinned/Disk/External traffic was applied correctly yet stayed invisible on /metrics (only Device-tier counts showed up). Add LowerTierIndexers::new_with_metrics and keep a metrics handle on LocalKvIndexer, wiring it through at all four production assembly sites. The metrics-less new() remains for tests and documents that production assembly should pass metrics. Verified on a real single-GPU dynamo serve + vLLM (OffloadingConnector, chunked offload, small CPU pool) run: with the wiring, kv_cache_events_applied matches the captured wire exactly -- stored ok = 685 (331 GPU + 354 CPU) and removed ok = 24 (real CPU evictions), previously 332 / 0. cargo test -p dynamo-kv-router --lib: 545 / 545 pass. cargo test -p dynamo-llm --lib kv_router: 166 / 166 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Change72 <changg@nvidia.com>
db0ec35 to
1d4e7b0
Compare
Signed-off-by: Change72 <changg@nvidia.com>
Walkthrough
ChangesLower-tier indexer metrics propagation and CPU medium mapping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Change72 <changg@nvidia.com>
|
/ok to test 17e2672 |
Trace the exact code paths for both fixes: - GAP-1: SGLang StorageMedium.CPU event on HiCache offload - GAP-2: PR ai-dynamo#10368 from_kv_medium() CPU mapping Co-Authored-By: Claude <noreply@anthropic.com>
…er-tier applies (ai-dynamo#10368) Signed-off-by: Change72 <cguo51@asu.edu> Signed-off-by: Change72 <changg@nvidia.com> Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit d8a94ef)
- Narrow the normalizer bypass to hash-only lower tiers (Disk / External) so CPU / HostPinned offload events keep flowing through salted cache-namespace propagation (no ai-dynamo#10368 regression). - Gate both consolidator ingresses on locality as well: drop REMOTE / unknown-locality events, not just non-device media, so a device-medium event tagged REMOTE is never tracked as a local G1 block. Adds a RawKvEvent::locality() accessor and extends the two consolidator G1-only tests with REMOTE / unknown cases. Signed-off-by: Change72 <changg@nvidia.com>
What
This PR makes Dynamo's KV router consume and observe vLLM native CPU-offload KV events.
Changes:
medium="CPU"as an alias for the existingHostPinnedtier, in the sharedStorageTier::from_kv_mediumnormalization layer (so the alias also applies to thekv_consolidatorconsumer — verified consistent:CPU→HostPinned).CPU_PINNEDandCPU_TIER1behavior.Stored/Removed/Clearedapplies inkv_cache_events_applied.dynamo-llmrouterassembly path.
Where should the reviewer start?
lib/kv-router/src/protocols.rs— themedium="CPU"->StorageTier::HostPinnedalias.lib/kv-router/src/zmq_wire/tests.rs— focused wire tests for placeholder and full CPUBlockStoredpayloads.lib/kv-router/src/indexer/lower_tier.rs— lower-tier worker now incrementskv_cache_events_appliedusing the same(event_type, status)labels as the primary indexer.lib/kv-router/src/indexer/lower_tier_indexers.rs,lib/kv-router/src/indexer/local.rs, andlib/llm/src/kv_router/indexer/mod.rs— metricshandles are threaded into lazily-created lower-tier indexers.
Why
vLLM's native
OffloadingConnectorpublishes CPU-tier KV events withmedium="CPU". Dynamopreviously did not classify that string as
HostPinned, so the event could not be routed to thelower-tier indexer as intended.
After adding the alias, the lower-tier path also needed observability. The first metrics patch made
LowerTierIndexer::workerincrement the samekv_cache_events_appliedcounter as the primarydevice-tier indexer, but the
dynamo-llmrouter assembly path still constructed lower-tierindexers without a metrics handle. The final wiring patch passes the shared metrics handle through
LocalKvIndexerandLowerTierIndexers::new_with_metrics, including lazily-createdHostPinned/Disk/External indexers.
This is why there are two metrics commits:
The metrics change is intentionally additive to the existing counter rather than adding a new
storage_tierlabel. The existingkv_cache_events_applied{event_type,status}metric representsthe total number of indexer-applied events. Before this PR, lower-tier applies were missing from
that total; after this PR, device and lower-tier events contribute to the same counter. Adding a
tier label would be a useful follow-up if operators need per-tier event accounting, but it would be
a metric-schema change across both primary and lower-tier indexers and is outside this PR.
Relationship to vLLM #43468
This PR is the Dynamo side of the self-describing CPU-event path. It expects CPU
BlockStoredevents to carry enough payload for Dynamo to reconstruct local block hashes. The vLLM-side PR
provides that payload for native
OffloadingConnector, including chunk mode.For chunked offload, vLLM intentionally emits plain fan-out. Overlapping chunks may repeat the same
per-block hash on the wire. Dynamo's standard worker publisher path already runs
EventDedupFilter, which ref-counts duplicate per-worker/tier hash announcements before they reachlower-tier indexing.
This PR does not rely on a
remove_blocks_implskip-absent-hashes change.Tests
Test commands run:
Focused coverage:
cpu_event_with_placeholder_payload_is_dropped_safelycpu_event_with_full_payload_is_indexableEnd-to-end validation
Validated with vLLM PR #43468 on a real single-GPU L4 stack:
Qwen/Qwen3-0.6Bfactor=3)self_describing_kv_events=trueEventDedupFilterin the pathWire capture and router metrics reconciled exactly:
kv_cache_events_applied: stored ok = 685, removed ok = 24BlockNotFound: 0Related Issues
Relates to vLLM native CPU offload KV-event integration:
No Dynamo issue is currently linked.