Repository navigation
[Fix] hicache: preserve salted KVCR peer-hint identities - #4
Merged
ziqifan617 merged 1 commit intoOct 23, 2026
Merged
ziqifan617 merged 1 commit into
ziqifan617 merged 1 commit into
Conversation
ziqifan617
force-pushed
the
codex/kvcr-hicache-salted-peer-hints
branch
from
October 23, 2026 16:31
ec3fdb1 to
75590f3
Compare
ziqifan617
changed the base branch from
ziqifan617/kvcr-hicache-performance
to
ziqifan617/kvcr-hicache-storage
October 23, 2026 16:31
ziqifan617
marked this pull request as ready for review
October 23, 2026 16:36
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.
Summary
Keep tenant isolation enabled without losing legitimate same-tenant KV reuse across workers.
The parent stack already includes
cache_saltin HiCache storage keys. This PR fixes a compatibility gap in peer fetching: the router and storage can assign different IDs to the same salted cache page, but native peer-hint membership previously assumed those IDs were interchangeable.This fixes a false cache miss for the same tenant. It is not a fix for a demonstrated cross-tenant KV leak, and it does not remove the salt or change persisted keys.
Example: reusing a tenant's document on another worker
Assume the same model and cache layout on two serving replicas, called worker 1 and worker 2.
cache_salt="tenant-A"and a long document followed by "Summarize this." Worker 1 processes the document and stores its KV in KVCR.The problem is that the same cached document page has two names:
event-A-123storage-A-789Before this fix, worker 2 effectively asks: "Fetch
storage-A-789; the router says worker 1 hasevent-A-123." Native membership compares the storage-page ID against the advertised event IDs, finds no match, and rejects the peer hint. The correct KV exists on worker 1, but worker 2 can miss that reuse opportunity and recompute the prefix.After this fix, the framework reconstructs a request-local binding between the two IDs using the same tokens, salt, namespace and complete preceding prefix. The adapter translates the hint only when its advertised event ID covers the exact requested storage page. The actual salted storage key remains unchanged.
Tenant B, using
cache_salt="tenant-B", produces different IDs. An announcement for tenant A's pages does not match B's binding and does not enable B to reuse A's KV.Why does this appear with cache salt but not ordinary unsalted requests?
With neither
cache_saltnorextra_key, both paths use the same original token-hash chain with no namespace seed. Their page IDs agree.When namespacing is enabled, the recipes differ:
extra_keyandcache_salt.Consequently, supplying the same salt to both workers does not make the router ID equal the storage ID: the layers encode it differently. A non-null
extra_keycan also activate this mismatch without a salt.The salt itself is not the problem. The bug is the missing translation between these two identity formats. "Legacy" in the source comments means the existing event format retained for compatibility; this PR does not deprecate it or cache salt.
extra_keyStack and review base
Direct stacked follow-up to sgl-project#45647. This is a core correctness fix and does not depend on the optional performance work in sgl-project#45649.
ziqifan617/kvcr-hicache-storage705e7a2913d1a6459ef3363f2b8f3e2753a27485Modifications
Accuracy Tests
Checks on this rebased core-only PR source
F401,F821,F823,UP037), formatting, AST parsing of all 11 changed Python files andgit diff --checkpassed.Prior validation of the binding implementation, not a fresh run of this commit
On the earlier performance-stack runtime
142e34c04b(not this core-only base), the private binding backport passed 72 adapter CPU cases and a real GLM-5.3-Flash source-to-destination continuation canary:zai-org/GLM-5.3-Flash@46a050bec13002ad29ee795c16e61cf4fa3c4a9c.That earlier run also used a separate allocator-only overlay and the existing
--enable-fp32-lm-headworkaround, neither included here. Router IDs came from actual source events, but the source-selection envelope was assembled by the canary. Same-node TCP/shared-memory, not cross-host RDMA or a deployed Dynamo frontend. The 72-case count includes the separate allocator regression and is not claimed as the exact test count of this PR.Before this rebase, a Linux-suite rerun was attempted through the retained Pod identity guard, but the named Pod was no longer present. The guard stopped before remote exec. No replacement allocation or GPU workload was started. The earlier native canary is evidence for the binding implementation, not fresh qualification of this rebased core-only commit. Complete Linux CPU suites and core-only native continuation revalidation remain merge gates.
Suggested focused Linux CPU checks (with the normal SGLang test dependencies installed):
Speed Tests and Profiling
No sustained-load, throughput, latency or speedup claim. This correctness fix does not relax native deadlines. Full runtime/performance qualification remains separate.
Security boundary
This preserves cache-reuse namespace separation under trusted, correctly assigned tenant salts. It is not an authentication/ACL, encryption or resource-quota feature. Empty salt intentionally aliases the unsalted namespace.
Checklist