feat(router): expose host/disk cache hit weights via CLI and env - #10157
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR introduces two new KV router configuration parameters— ChangesKV Router Cache Hit Weight Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
The pyo3 `KvRouterConfig` binding already accepts `host_cache_hit_weight` and `disk_cache_hit_weight` as constructor kwargs, and the selector reads them at every routing decision (`compute_logit_cached_blocks` in `selector.rs`), but neither is reachable from `dynamo.frontend` / `dynamo.router` CLI or via the matching `DYN_ROUTER_*` env var. Without these, deployments running a backend that exposes a lower-tier KV cache (for example, vLLM's `OffloadingConnector` for CPU offload) cannot tune how strongly the router prefers a worker holding the prefix in CPU vs disk vs device memory short of building `KvRouterConfig` from Python directly. Add two flags symmetric to `--router-prefill-load-scale`: --router-host-cache-hit-weight / DYN_ROUTER_HOST_CACHE_HIT_WEIGHT --router-disk-cache-hit-weight / DYN_ROUTER_DISK_CACHE_HIT_WEIGHT Defaults match the existing pyo3 defaults (0.75 / 0.25), so behavior is unchanged for deployments that do not set the new flag or env var. The `load_aware` preset preserves both values, mirroring how it already preserves `prefill_load_scale`. No Rust changes: pure CLI plumbing through the existing `_KV_ROUTER_FIELDS` / `kv_router_kwargs()` pattern that `prefill_load_scale` already follows. Tests: - Eight new cases in `test_kv_router_args.py` cover CLI flag, env var, default, `kv_router_kwargs()` flow-through, and load-aware preset preservation for both fields. Docs: - `docs/components/router/router-configuration.md` documents the two new flags in the Routing Behavior list and adds a tuning-guidelines paragraph for lower-tier weights. Signed-off-by: Change72 <cguo51@asu.edu>
Signed-off-by: Change72 <cguo51@asu.edu>
144bfdc to
8828237
Compare
8828237 to
1dc3a80
Compare
|
/ok to test f96a405 |
…dynamo#10157) Signed-off-by: Change72 <cguo51@asu.edu> Signed-off-by: Muqi Li <muqi1029@gmail.com>
The column was mixing two incompatible quantities. Most cells came from
the release bodies, but the bodies themselves are not uniform: v1.1.0's
figure counted only external contributors, from a list its own wording
calls partial ("first-time external contributors ... include"). GitHub's
New Contributors lists answer a third question -- first-ever merged PR
over the compare range -- and put the pre-v1.0.0 releases at 11 where
their announcements say 20 and 14.
Settle on the bodies, which is what every other column in this table
already uses, and require the figure to be release-wide and complete.
v1.1.0 fails both tests and becomes a dash rather than an undercount.
v1.3.0 moves 23 -> 24. The published body states 24 and enumerates 24;
23 came from a pre-release draft. @Change72 was raised as a name that
should not be on that list, on the grounds that PR #10157 predates the
v1.2.1 tag. It does by date, but release/1.2.x was cut from main on
2026-05-12 and #10157 merged 2026-06-02, so it is absent from v1.2.1 and
first ships in v1.3.0 -- inside the compare range, which makes them a
genuine first-timer here. Left in place.
Footnote the empty cells on the rendered page too. They are permanent
now rather than pending, so a reader deserves to know why they are empty
and that first-timers are counted release-wide.
Signed-off-by: Dan Gil <dagil@nvidia.com>
The column was mixing two incompatible quantities. Most cells came from
the release bodies, but the bodies themselves are not uniform: v1.1.0's
figure counted only external contributors, from a list its own wording
calls partial ("first-time external contributors ... include"). GitHub's
New Contributors lists answer a third question -- first-ever merged PR
over the compare range -- and put the pre-v1.0.0 releases at 11 where
their announcements say 20 and 14.
Settle on the bodies, which is what every other column in this table
already uses, and require the figure to be release-wide and complete.
v1.1.0 fails both tests and becomes a dash rather than an undercount.
v1.3.0 moves 23 -> 24. The published body states 24 and enumerates 24;
23 came from a pre-release draft. @Change72 was raised as a name that
should not be on that list, on the grounds that PR #10157 predates the
v1.2.1 tag. It does by date, but release/1.2.x was cut from main on
2026-05-12 and #10157 merged 2026-06-02, so it is absent from v1.2.1 and
first ships in v1.3.0 -- inside the compare range, which makes them a
genuine first-timer here. Left in place.
Footnote the empty cells on the rendered page too. They are permanent
now rather than pending, so a reader deserves to know why they are empty
and that first-timers are counted release-wide.
Signed-off-by: Dan Gil <dagil@nvidia.com>
The column was mixing two incompatible quantities. Most cells came from
the release bodies, but the bodies themselves are not uniform: v1.1.0's
figure counted only external contributors, from a list its own wording
calls partial ("first-time external contributors ... include"). GitHub's
New Contributors lists answer a third question -- first-ever merged PR
over the compare range -- and put the pre-v1.0.0 releases at 11 where
their announcements say 20 and 14.
Settle on the bodies, which is what every other column in this table
already uses, and require the figure to be release-wide and complete.
v1.1.0 fails both tests and becomes a dash rather than an undercount.
v1.3.0 moves 23 -> 24. The published body states 24 and enumerates 24;
23 came from a pre-release draft. @Change72 was raised as a name that
should not be on that list, on the grounds that PR #10157 predates the
v1.2.1 tag. It does by date, but release/1.2.x was cut from main on
2026-05-12 and #10157 merged 2026-06-02, so it is absent from v1.2.1 and
first ships in v1.3.0 -- inside the compare range, which makes them a
genuine first-timer here. Left in place.
Footnote the empty cells on the rendered page too. They are permanent
now rather than pending, so a reader deserves to know why they are empty
and that first-timers are counted release-wide.
Signed-off-by: Dan Gil <dagil@nvidia.com>
Overview:
Expose
host_cache_hit_weightanddisk_cache_hit_weightthrough thedynamo.frontend/dynamo.routerCLI and matchingDYN_ROUTER_*env vars.These fields already exist on the pyo3
KvRouterConfigbinding and are usedby the selector when scoring cached blocks. The missing piece is the
operator-facing CLI/env surface: today deployments can tune
--router-kv-overlap-score-creditand--router-prefill-load-scale, but thelower-tier cache-hit weights are only reachable by constructing
KvRouterConfigfrom Python directly.This is strictly additive. The new defaults match the existing binding
defaults (
0.75for host,0.25for disk), so behavior is unchanged unless anoperator opts in via the new flags or env vars.
Details:
New
KvRouterArgGroupoptions:--router-host-cache-hit-weightDYN_ROUTER_HOST_CACHE_HIT_WEIGHT0.75--router-disk-cache-hit-weightDYN_ROUTER_DISK_CACHE_HIT_WEIGHT0.25Both fields are added to
_KV_ROUTER_FIELDSandKvRouterConfigBase, sokv_router_kwargs()forwards them intoKvRouterConfig(**kwargs)through thesame path used by
prefill_load_scale. Theload_awarepreset preserves bothvalues.
The practical use case is lower-tier KV cache exposed by a KV connector, such
as vLLM CPU offload or a disk-backed tier. These weights control whether a
worker with a full host/disk-tier prefix match should beat another worker with
a smaller device-local match.
Test coverage (added in
test_kv_router_args.py):kv_router_kwargs()forwarding for both fields.--load-awarepreservation of both weights.Docs (
docs/components/router/router-configuration.md):Validation:
pytest components/src/dynamo/common/tests/configuration/test_kv_router_args.pypasses locally (41/41, 8 new + 33 pre-existing).
pre-commit runpasses on the changed Python and docs files.Where should the reviewer start?
components/src/dynamo/common/configuration/groups/kv_router_args.py-- CLI/env plumbing and
kv_router_kwargs()forwarding.components/src/dynamo/common/tests/configuration/test_kv_router_args.py-- focused coverage for CLI, env vars, defaults, forwarding, and
--load-aware.docs/components/router/router-configuration.md-- user-facing flag descriptions and tuning guidance.
No selector, Rust config, or pyo3 signature changes are needed because those
already accept and consume these kwargs.
Related Issues:
pre-issue discussion. The source diff here is ~31 lines exposing
pre-existing config fields (the rest is tests and docs), and there is no new
feature surface beyond what the pyo3 binding already supports -- happy to
file an issue retroactively if maintainers prefer.
Summary by CodeRabbit
New Features
--router-host-cache-hit-weightand--router-disk-cache-hit-weightCLI parameters to configure cache hit credit weighting in KV router (defaults: 0.75 for host, 0.25 for disk). Supports environment variable configuration.Documentation