Consider KV cache storage tier in scoring by avinxshKD · Pull Request #1293 · volcano-sh/kthena

avinxshKD · 2026-07-03T16:06:20Z

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

Makes the kvcache-aware scheduler plugin consider the storage medium of cached KV blocks when scoring pods. Cached blocks keep the existing timestamp behavior, while runtimes that report a medium can now let the router apply tier weights during longest-prefix scoring.

Which issue(s) this PR fixes:
Fixes #1283

Special notes for your reviewer:

The Redis value format remains backward-compatible: existing timestamp-only entries continue to score with weight 1.0.

Does this PR introduce a user-facing change?:

kvcache-aware scoring can now weight KV cache hits by reported storage medium.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

gemini-code-assist · 2026-07-03T16:07:54Z

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

avinxshKD · 2026-07-03T16:08:33Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces tier-weighted scoring based on storage mediums (such as GPU and CPU) for the KV cache-aware scheduler plugin, updating both the Go scheduler and the Python event-handling and Redis-management components. Feedback highlights critical performance bottlenecks in the Python kv_cache_manager.py due to sequential hget queries inside loops, which should be batched. Additionally, the Go implementation has logic issues with weight validation: it prevents setting a weight of 0.0 (overriding it to defaults or 1.0) and lacks validation to ensure weights do not exceed 1.0, which could cause calculated pod scores to exceed the maximum limit of 100.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

hzxuzhonghu

Please first update the proposal.

How about sglang? Does it already handle the medium info?

YaoZengzeng · 2026-07-07T09:13:35Z

-	MaxBlocksToMatch int `yaml:"maxBlocksToMatch,omitempty"`
+	BlockSizeToHash  int                `yaml:"blockSizeToHash,omitempty"`
+	MaxBlocksToMatch int                `yaml:"maxBlocksToMatch,omitempty"`
+	TierWeights      map[string]float64 `yaml:"tierWeights,omitempty"`


The user guide needs to be updated regarding how to use this feature.

already on it, pls wait.

YaoZengzeng · 2026-07-07T09:16:37Z

+			found = true
+		}
+	}
+	if !found {


More doc about the case which medium is not specified or unknown.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

volcano-sh-bot · 2026-07-07T09:34:26Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign git-malu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

avinxshKD · 2026-07-07T09:40:25Z

@YaoZengzeng updated the proposal and user guide, PTAL

How about sglang? Does it already handle the medium info?

@hzxuzhonghu SGLang already carries medium in the event pathand. this PR now persists it in Redis so the router can use it for tier-weighted scoring. Also documented that timestamp only entries, missing media, and unknown media fall back to weight 1.0.

FAUST-BENCHOU · 2026-07-08T06:26:16Z

@YaoZengzeng updated the proposal and user guide, PTAL

How about sglang? Does it already handle the medium info?

@hzxuzhonghu SGLang already carries medium in the event pathand. this PR now persists it in Redis so the router can use it for tier-weighted scoring. Also documented that timestamp only entries, missing media, and unknown media fall back to weight 1.0.

I dont think so.Sglang use cpu_pinned.Pls check it clearly
https://github.com/sgl-project/sglang/blob/c9303a08daea4300d17fee6ea6564b1c961673ee/python/sglang/srt/disaggregation/kv_events.py#L85-L86

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

avinxshKD · 2026-07-08T07:08:44Z

@FAUST-BENCHOU thnks, good catch. I checked the linked SGLang code and CPU_PINNED is the value emitted for host-pinned CPU blocks. have updated the defaults, docs, and tests.

hzxuzhonghu

I would suggest you first deep dive what all the supported medium from vllm, sglang And document it clearly

hzxuzhonghu · 2026-07-10T02:50:21Z

 | ------------------ | ------- | -------------------------------------------------------------------------------- |
 | `blockSizeToHash`  | 16      | Number of tokens per block. Must match the vLLM block size for optimal matching. |
 | `maxBlocksToMatch` | 128     | Maximum number of blocks to process per request. Limits Redis queries.           |
+| `tierWeights`      | gpu: 1.0<br />cpu_pinned: 0.8 | Optional weights for reported KV cache storage media. Missing or unknown media use weight `1.0`. |


As only sglang supported cpu_pinned, so this does not apply to vllm?

vllm use cpu

vLLM reports CPU blocks as CPU, while SGLang reports host-pinned blocks as CPU_PINNED, so the docs now show both cpu and cpu_pinned, updated

hzxuzhonghu · 2026-07-10T02:54:31Z

  "pod-name-1.namespace.svc.cluster.local": "1703123456",
-  "pod-name-2.namespace.svc.cluster.local": "1703123789"
+  "pod-name-2.namespace.svc.cluster.local": "1703123789|GPU",
+  "pod-name-3.namespace.svc.cluster.local": "1703123900|GPU,CPU_PINNED"


why both GPU,CPU_PINNED

good point sir, i was trying to show a pod/block being updated over time but that made the stored value look like multiple media at once. I removed it; the Redis value now stores one reported medium per update

hzxuzhonghu · 2026-07-10T02:55:11Z

 ```

 1. **Tokenization**: Convert input text/messages to token sequences using model-specific tokenizers
-2. **Block Division**: Split tokens into fixed-size blocks (configurable, default 128)


why change this value?

sir, the old proposal text said 128, but the implementation default is 16, so now i updated the proposal to match the current code

hzxuzhonghu · 2026-07-10T02:57:28Z

+                        if isinstance(existing_value, bytes):
+                            existing_value = existing_value.decode()
+                        timestamp, _, medium_value = existing_value.partition("|")
+                        mediums = [m for m in medium_value.split(",") if m and m != medium]


please explain why medium is not a single value

I can see vllm only report GPU or CPU not sure sglang

agreed, this was overcomplicated. I changed it back to a single medium value and overwrite the stored value on each block update

hzxuzhonghu · 2026-07-10T02:59:32Z

        return f"{cls.MAPPING_KEY_PREFIX}:{pod_identifier}@{engine_hash}"

+    @staticmethod
+    def _format_matrix_value(timestamp: str, medium: Optional[str] = None,


why call _format_matrix_value

removed it, was unnecessary once we stopped merging media

hzxuzhonghu · 2026-07-10T03:02:14Z

+        if isinstance(existing_value, bytes):
+            existing_value = existing_value.decode()
+
+        mediums = []
+        if existing_value and "|" in existing_value:
+            mediums = [m for m in existing_value.split("|", 1)[1].split(",") if m]


Why donot you override the old value directly?

avinxshKD · 2026-07-10T04:23:28Z

I would suggest you first deep dive what all the supported medium from vllm, sglang And document it clearly

yes sir, Im on it and will get back, thankyou so much for the guidance.

hzxuzhonghu · 2026-07-10T09:47:10Z

+	bestWeight := 0.0
+	found := false
+	for _, name := range strings.Split(medium, ",") {
+		weight, ok := t.tierWeights[strings.ToLower(strings.TrimSpace(name))]


weightForMedium("CPU_PINNED,DISK") returns the configured CPU weight because unknown entries are ignored once any known tier is found. The documented fallback gives an unknown medium weight 1.0, and the highest weight should win, so this combination should return 1.0. Please treat each unknown entry as weight 1.0 and add a mixed known/unknown test.

fixed, weightForMedium now treats the stored medium as a single value nd so unknown or unconfigured media fall back to 1.0

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

avinxshKD · 2026-07-11T08:22:31Z

I would suggest you first deep dive what all the supported medium from vllm, sglang And document it clearly

yes took time, updated both the user guide and proposal. i checked the vLLM and SGLang event definitions and documented the supported media explicitly: vLLM uses GPU, CPU, FS, OBJ
SGLang uses GPU, CPU_PINNED, DISK, EXTERNAL. Missing or unknown media fall back to weight 1.0

avinxshKD · 2026-07-11T08:32:48Z

Hey @hzxuzhonghu PTAL, let me know if anything needs adjustment

Copilot AI review requested due to automatic review settings July 3, 2026 16:06

volcano-sh-bot added the kind/enhancement New feature or request label Jul 3, 2026

volcano-sh-bot requested review from LiZhenCheng9527 and git-malu July 3, 2026 16:06

Copilot AI reviewed Jul 3, 2026

volcano-sh-bot added the size/L label Jul 3, 2026

gemini-code-assist Bot reviewed Jul 3, 2026

View reviewed changes

avinxshKD force-pushed the fix/kvcache-tier-scoring branch from c2dc7c9 to 3bbd9be Compare July 3, 2026 16:20

hzxuzhonghu reviewed Jul 6, 2026

View reviewed changes

YaoZengzeng reviewed Jul 7, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 7, 2026 09:34

avinxshKD force-pushed the fix/kvcache-tier-scoring branch from 3bbd9be to b3fdd7f Compare July 7, 2026 09:34

Copilot AI reviewed Jul 7, 2026

Copilot AI review requested due to automatic review settings July 8, 2026 06:59

avinxshKD force-pushed the fix/kvcache-tier-scoring branch from b3fdd7f to 8b3abfc Compare July 8, 2026 06:59

Copilot AI reviewed Jul 8, 2026

hzxuzhonghu reviewed Jul 10, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 11, 2026 08:00

avinxshKD force-pushed the fix/kvcache-tier-scoring branch from 8b3abfc to 7070f93 Compare July 11, 2026 08:00

Copilot AI reviewed Jul 11, 2026

Consider KV cache storage tier in scoring

e0e3c77

Signed-off-by: Avinash Kumar Deepak <avinash8655279@gmail.com>

avinxshKD force-pushed the fix/kvcache-tier-scoring branch from 7070f93 to e0e3c77 Compare July 11, 2026 08:10

Copilot AI review requested due to automatic review settings July 11, 2026 08:10

Copilot AI reviewed Jul 11, 2026

Uh oh!

Conversation

avinxshKD commented Jul 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot commented Jul 3, 2026

Uh oh!

avinxshKD commented Jul 3, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

volcano-sh-bot commented Jul 7, 2026

Uh oh!

avinxshKD commented Jul 7, 2026

Uh oh!

FAUST-BENCHOU commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

avinxshKD commented Jul 8, 2026

Uh oh!

hzxuzhonghu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avinxshKD commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!