feat(tier-normalization): normalize device tier names and default unknown tier weight to 0 - #695
Open
Alex-ai-future wants to merge 5 commits into
Open
feat(tier-normalization): normalize device tier names and default unknown tier weight to 0#695Alex-ai-future wants to merge 5 commits into
Alex-ai-future wants to merge 5 commits into
Conversation
Alex-ai-future
requested review from
dannyharnik,
kfirtoledo,
liu-cong and
vMaroon
as code owners
July 8, 2026 06:49
github-actions
Bot
added
the
size/L
…nown tier weight to 0 Two problems addressed: 1. Unknown device tiers (e.g. fs, obj) defaulted to weight 1.0, silently inflating pod scores above known-tier pods like CPU(0.8). Now defaults to 0 with a one-time warning per unknown tier. 2. Different components use different names for the same physical tier (e.g. vLLM emits FS, PVC Evictor emits SHARED_STORAGE). Added a tier alias registry that normalizes these to canonical names so store/remove events build equal PodEntries and match correctly during eviction. Changes: - pkg/kvcache/kvblock_scorer.go: fillMaxWeights default 0 + warning - pkg/kvevents/pool.go: Config.TierAliases, normalizeTier method - pkg/kvcache/kvblock_scorer_test.go: tests for unknown tier weight 0 - pkg/kvevents/pool_test.go: tests for tier alias normalization - docs/configuration.md: document tierAliases and unknown tier behavior Signed-off-by: Alex <alex.tech.lab@outlook.com>
Alex-ai-future
force-pushed
the
fix/tier-normalization
branch
from
July 8, 2026 06:50
b2f092d to
ae0a32a
Compare
Resolves gocritic typeDefFirst lint violation. Signed-off-by: Alex <alex.tech.lab@outlook.com>
…sertions
Previously the test ignored verification results (_ = reqKey). Now it:
1. Verifies PodEntry{pod-1, fs} exists after BlockStored with FS
2. Verifies the entry is evicted after BlockRemoved with SHARED_STORAGE
3. Confirms no pods remain for the requestKey after eviction
Signed-off-by: Alex <alex.tech.lab@outlook.com>
Resolves ineffassign and staticcheck lint failures. Signed-off-by: Alex <alex.tech.lab@outlook.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves KV-cache scoring and event deduplication correctness by (1) preventing unknown device tiers from inflating pod scores and (2) normalizing storage tier names so store/remove events refer to the same canonical tier.
Changes:
- Default unknown device-tier weights to
0.0in the KV-cache scorer and emit a one-time warning per unknown tier. - Add configurable tier alias normalization (with built-in defaults) so differing medium names (e.g.,
SHARED_STORAGEvsFS) map to the same tier. - Add/extend unit tests and update configuration documentation for the new behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/kvevents/pool.go | Adds tier alias registry and normalizes tiers during event processing. |
| pkg/kvevents/pool_test.go | Adds tests validating alias normalization behavior. |
| pkg/kvcache/kvblock_scorer.go | Changes unknown-tier default weight to 0 and logs a one-time warning per tier. |
| pkg/kvcache/kvblock_scorer_test.go | Adds tests ensuring unknown tiers do not inflate scores. |
| docs/configuration.md | Documents unknown-tier weight behavior and the new tierAliases configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+150
| // Build tier aliases: start with defaults, then merge user-provided overrides | ||
| tierAliases := make(map[string]string) | ||
| for k, v := range defaultTierAliases { | ||
| tierAliases[k] = v | ||
| } | ||
| for k, v := range cfg.TierAliases { | ||
| tierAliases[k] = v | ||
| } |
Comment on lines
+1187
to
+1191
| cfg := &Config{ | ||
| TierAliases: map[string]string{ | ||
| "custom_tier": "fs", | ||
| }, | ||
| } |
Comment on lines
+28
to
+30
| // unknownTierWarned tracks which unknown device tiers have already been warned | ||
| // about, so we don't spam the logs on every block. | ||
| var unknownTierWarned sync.Map |
- Lowercase user-provided alias keys/values so normalizeTier matches regardless of how users write them in config (e.g. SHARED_STORAGE → fs) - Use DefaultConfig() in TestNormalizeTierMethod to avoid zero Concurrency which would cause queue index panic Addresses Copilot review comments. Signed-off-by: Alex <alex.tech.lab@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Addresses two problems with device tier handling in the KV cache scorer:
Problem 1: Unknown tier defaults to weight 1.0 → silently inflates scores
fillMaxWeightsfalls back toweight = 1.0for any DeviceTier not in the configuredKVCacheBackendConfig. With the default config (gpu=1.0, cpu=0.8), new tiers introduced by vLLM (e.g.fs,obj) get the same score as GPU:Fix: Default unknown tier weight to
0.0with a one-time warning per unknown tier. Users must explicitly configure new tiers to participate in routing.Problem 2: Tier name mismatch → store/remove don't match
Different components use different names for the same physical tier:
Same physical tier, two different PodEntries. Evict of
"shared_storage"doesn't match the"fs"store entry → stale entries remain.Fix: Added a tier alias registry (
Config.TierAliases) that normalizes alternative medium names to canonical names before constructing PodEntries. Built-in defaults:shared_storage → fs,object_store → obj.Changes
pkg/kvcache/kvblock_scorer.gofillMaxWeightsdefault 0 + one-time warning per unknown tierpkg/kvevents/pool.goConfig.TierAliasesfield,normalizeTiermethod, default aliasespkg/kvcache/kvblock_scorer_test.gopkg/kvevents/pool_test.godocs/configuration.mdtierAliasesconfig and unknown tier behaviorBackward Compatibility
gpuandcpuweights are unaffected.shared_storage → fs,object_store → obj) are additive; users can override via config.