Skip to content

[HiCache] Add KVCR as an L3 storage backend for hint-driven cross-instance KV reuse - #36409

Open
linhu-nv wants to merge 4 commits into
sgl-project:mainfrom
linhu-nv:linhu/kvcr-hicache-backend
Open

linhu-nv wants to merge 4 commits into
sgl-project:mainfrom
linhu-nv:linhu/kvcr-hicache-backend

Conversation

@linhu-nv

@linhu-nv linhu-nv commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Implements RFC #32903. Router-side companion: ai-dynamo/dynamo#13591 (this PR is independently mergeable — without it the hint is simply never populated and the backend runs as a local-only L3 tier).

Motivation

SGLang's existing HiCache L3 backends (mooncake, hf3fs, eic, …) all reach cross-instance reuse the same way: put KV in a shared external store every worker can address by content hash. That works well when such a store exists, but it is a deployment prerequisite, and it writes every offloaded page whether or not anyone will read it.

This PR adds a differently-shaped backend: peer-to-peer, pull-on-demand, steered by the router. KVCR (nvidia-kvcr, a framework-neutral KV cache runner with its own DRAM tier and a NIXL data path) already holds each worker's offloaded pages locally. The missing piece is knowing which peer holds a given prefix — SGLang cannot know that, and a destination worker cannot probe a peer's residency without a round trip per page. A KV-aware router already knows, because it builds a global prefix index from the BlockStored/BlockRemoved events SGLang publishes. So this PR makes that hint reachable from the storage backend, and adds the backend that acts on it. KV then moves only when the router has established that a specific peer holds a specific prefix a specific request needs — no shared store, no extra service, no speculative writes.

Two-worker loopback, dense Qwen3-8B, --page-size 64, once the distinct working set exceeds the device pool: +53–59% qps, −50% TTFT. An FP8 MoE model on the same harness: +20% — the dense number should not be extrapolated. Both predate the kvcc→kvcr rename. No cost when the backend is off: the hint is None on every path that does not set it, and the event change adds trailing nulls to a msgpack array.

Modifications

One self-contained new backend (+2287), a thin per-request hint seam through existing code (+308 / −30 across 14 files), and tests (+2848). Nothing outside the backend changes behaviour unless --hicache-storage-backend kvcr is selected.

1. New backend — python/sglang/srt/mem_cache/storage/kvcr/. kvcr_store.py (the whole HiCacheStorage surface: deposit/get, remote-hint path, per-rank control endpoints, counters), router_hint.py (the dynamo wire seam — normalizing block hashes between the router's u64 and SGLang's page-hash prefix), pin_adapter.py, kvcr_config.py, and a reviewer's README.md covering how to run it, what was verified, and every known gap. Registered in backend_factory.py; v2 zero-copy path only.

2. A per-request KV router hint, threaded end to end. GenerateReqInput.kv_router_hint → TokenizedGenerateReqInput → Req → Scheduler.prefetch → StorageOperation.router_hint → HiCacheStorageExtraInfo.extra_info → batch_exists_v2/batch_get_v2. An opaque Optional[dict] passed through untouched; the generic controller stays backend-agnostic by stashing it under a literal key in the free-form extra_info dict already threaded into every v2 call, and a test pins that literal against the backend's matching constant (a rename on one side is otherwise invisible — every hint just stops being found). Reviewer note: SGLang has two prefetch controller stacks (HiCacheController and HybridCacheController); both had to be threaded or the untouched one raises TypeError. Both host-memory modes (cache and buffer_only) work, and cost nothing extra, because the hint rides the operation rather than the host tier.

3. KV event layout aligned with what routers parse positionally — and a live bug fix. BlockStored/BlockRemoved/AllBlocksCleared gain the trailing slots a cross-framework KV router expects at fixed positions (lora_name, extra_keys, then group_idx, kv_cache_spec_kind, kv_cache_spec_sliding_window, locality, ownership). SGLang populates almost none of them but must hold their places: these structs serialize as arrays, so a field appended into a slot the router reads as something else is silently misinterpreted rather than rejected. That is exactly what cache_salt was doing — it sat directly after medium, which is the router's lora_name: Option<String> slot, and a map landing there fails to decode and takes down the entire published batch, so one salted request could blind the router to every event beside it. The salt now goes in the positional extra_keys slot (prefix-tagged to match dynamo's DYNAMO_CACHE_SALT_PREFIX, pinned by a test) while the typed BlockStoredWithMetadata field stays parked past every position the router interprets. KVCacheEventRecorder.enqueue coalescing now compares the new fields, since it rewrites the tail's block-hash list in place.

4. Guards for the rank layouts this backend cannot address. A KVCR block key is sha256(token ids)#<segment> — it names the tokens and nothing about which slice of the model produced the bytes. _rank_port_offset separates (dp, attn_cp, attn_tp) by giving each rank its own control port; two coordinates have no such separation and both fail open, returning wrong KV with no error anywhere. pp_size > 1: pp_rank is not in the port offset, so pp0/tp0 and pp1/tp0 derive the same port and a hint aimed at one stage reaches the other. should_split_heads: head splitting exists so peers at different TP degrees can share a prefix, but this backend emits one key per page either way, so those peers agree on a key while holding different head slices. Both now raise at construction. Keyed on should_split_heads rather than tp_lcm_size deliberately — the controller accepts tp_lcm_size everywhere but resolves it to False for rank-replicated (MLA) models and non-page_head layouts, where the pooled key is correct; a test pins that distinction. Relatedly, and following the existing mooncake precedent, kvcr joins the set of backends for which HiCache draft L3 is disabled: on the generic draft path its v1 stubs would report a successful write then return None for every read, while a target-KV prefetch still admits the node, so load-back would copy stale bytes into draft host pages (output stays correct since proposals are verified, but acceptance silently degrades).

5. Diagnostics for three silent HiCache failure modes. All upstream behaviour, not backend-specific, but each presents as "the L3 backend is broken" and none logs anything today. Warnings only, no behaviour change. Offload stalled (hiradix_cache.py): _update_host_leaf_status admits a node to evictable_host_leaves only after the device tier drops it, so if the device pool is larger than the host pool, the host pool fills first, GPU eviction never fires, and L2+L3 both stop for the life of the process — it does not recover as entries age out. Leaked prefetch reservation (cache_controller.py): prefetch_tokens_occupied is reserved at creation and released only on the completion path, so a prefetch ending elsewhere leaks it permanently; once past the cap, L3 stops silently and query_storage_hit_length keeps reporting hits no batch_get follows. Rate-limited and leaked look identical from outside, so the counter is now logged. flush_cache refused (scheduler.py): the existing warning reports only request counts, so an in-flight HiCache op yields "0 queued, 0 running, still refused"; it now also reports write-through / load-back / prefetch / backup depths.

6. batch_get_v1/batch_set_v1 raise instead of returning None. The non-abstract default was pass, but callers index the result or wrap it in all(...), so an unimplemented backend surfaced as a TypeError on the prefetch daemon thread, which HiCache never restarts — silently disabling L3 for the process lifetime. Now NotImplementedError. Included because it is what makes the draft-L3 guard above honest; happy to split out.

Accuracy Tests

No kernel or model-forward changes. The real correctness risk here is different — a remote fetch returning the wrong KV would produce plausible, wrong output with nothing logged — so testing targeted that. Under failure, a remote fetch must degrade to recompute and never admit wrong KV: source frozen (SIGSTOP), source dead, and source restarted each matched the source-cached control arm word for word, 4/4, with the expected key present; no bad KV entered the radix tree in any run. TP=2 bring-up found and fixed two silent per-rank bugs (a bind collision, and both ranks dialing the same source port) whose symptom was correct-looking output built from the wrong shard, so the test compares generated text rather than token counts. Methodology note that mattered: the control arm has to be B-hinted vs A-cached, not A-fresh vs B-hinted — the latter forks on sampling and produces false failures on its own.

Unit tests live in test/registered/ and are all CPU-tier (register_cpu_ci, suite base-a-test-cpu) — no GPU and no nvidia-kvcr wheel required, since the KVCR-dependent cases skip cleanly when the wheel is absent. New: test_kvcr_router_hint_schema.py (the dynamo wire seam — page hash → event i64 → router u64 → page key round trip, malformed-payload fail-closed, the two-literal key agreement), test_kvcr_router_hint_e2e.py (hint path against the real KVCR core), test_kvcr_tp_and_concurrency.py (per-rank endpoints, concurrency, sidecar pools, the parallelism guards), test_kvcr_config_validation.py, test_hicache_offload_stall.py, test_hicache_storage_thread_survival.py (a raising backend must not kill the storage thread). Amended: test_kv_events.py (the full positional wire layout, pinned slot by slot) and test_radix_cache_unit.py (coalescing must not merge across the new fields). 87 passing in-container against nvidia-kvcr 873391c. The parallelism guards were verified pre-fix: with the guard removed both rejection cases fail with RuntimeError not raised, while both acceptance cases stay green (they guard the opposite direction — over-rejection).

Speed Tests and Profiling

See Motivation for the headline A/B. Capacity/sizing was swept separately across 4 configurations, one variable at a time, 95–130 distinct prefixes each.

Known gaps

Listed up front rather than left to be found; all are in the backend README too. (1) Host pool must be ≥ device pool or offload stops permanently — upstream HiRadixCache behaviour; this PR adds the warning, not a fix. (2) The default prefetch timeout silently discards a successful remote fetch. 1.0 + 0.25 × tokens/1024 is under the measured remote-fetch latency (1.2–3.9 s for 31 pages), and losing the race throws the fetch away after every page has arrived: increment is refused on a terminated op, completed_tokens stays 0, nothing warns. Worked around with prefetch_timeout_base: 15.0, but the right fix is probably upstream — a backend-declared default, or not discarding a transfer that completed between the termination decision and the increment. Direction welcome. (3) A peer restart costs ~34 s of degraded P2P — KVCR-side, reported separately; transient, self-healing, no wrong answers. (4) local_dram_bytes ≥ 32 GiB fails to start — NIXL registration exceeds KVCR's 10 s progress-thread join timeout; KVCR-side. (5) The source offers no framework memory as a NIXL source — pin_adapter.py declines every pin, since safely pinning a HiCache host page needs a residency index inside HiRadixCache this backend does not have; everything served comes from KVCR's own tier, so the cost is a miss, never a wrong result. (6) No DeepSeek / GLM-class model support, failing closed — those models keep indexer/SWA/full-KV in separate pools with separate index spaces while KVCR's framework_dram registers a single region with one global slot size; the fix is a KVCR core API change (framework_dram → a list of (region, slot_size) declarations registered per class, essentially what mooncake already does with its multi-suffix keys). (7) No model/tenant key namespace — mooncake folds model_name and a backend tag into its key prefix; this backend does not. Fails closed today because each worker's tier is private, but it is the right next hardening step. (8) clear() and get_stats() not implemented — flush_cache logs success and clears nothing, and storage metrics come back empty. Both fail closed; get_stats() is cheap and could land here if reviewers want it.

Questions for reviewers

Sections 3, 5 and 6 are independent of the backend and fix pre-existing issues — split into a separate PR, or keep them here where the motivation is visible? And on known gap 2, the prefetch-timeout discard looks worth fixing properly upstream rather than working around in config; preference on which direction?

Checklist


CI States

Latest PR Test (Base): ❌ Run #35312087938
Latest PR Test (Extra): ❌ Run #35312087785
Latest PR Test (AMD ROCm 10): ❌ Run #35312087950

@ishandhanani ishandhanani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking review: public KVCR compatibility and remote-operation lifecycle need resolution before this backend can be used. The remaining findings cover clear semantics, public API propagation, and TP port validation. Please split generic typed/versioned KV-hint transport from backend-specific action execution.

FrameworkDramInput,
KVCRBackendConfigs,
KVCRConfig,
LocalDramInput,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Public KVCR is currently incompatible. Installing the public source at facdc780 fails before server startup: its config exports LocalDramInfo and it has control_channels.py; neither LocalDramInput nor peer_control_channel exists. The pinned README revision is also unreachable, and this PR adds no package dependency or CI install. Please port this adapter to a reachable release/API and add an optional dependency plus an import smoke test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebased on the latest kvcr public repo.

op_handle,
timeout,
)
return {}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Timing out cannot discard this core operation. On a hinted pull whose source exits before write_done, this returns and drops only SGLang bookkeeping. Public KVCR cancellation is currently a no-op, so the core/NIXL operation remains outstanding with no source progress to complete it. Repeated dead-peer retries can accumulate unbounded work. Require a terminal/reclamation contract from the core, or make retry and teardown bounded before enabling remote hints.

return sock.getsockname()[1]


class KVCRStore(HiCacheStorage):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 — Implement clear or report it as unsupported. KVCRStore inherits HiCacheStorage.clear(), which is a no-op. The HiCache control path nevertheless reports clear success, leaving KVCR-resident data available after the caller believes it has cleared storage. Override clear() with synchronized discard/reset behavior, or expose lack of support to the caller.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. clear() now raises NotImplementedError (kvcr_store.py:1572).

KVCR has no bulk invalidation, so the inherited pass made /flush_cache report success while every deposited block stayed resident and peer-visible. clear_storage_backend already treats a raise as "this backend cannot" (hiradix_cache.py:860 returns False), so nothing else breaks.

Comment thread python/sglang/srt/entrypoints/engine.py Outdated
# Opaque per-request KV router hint naming the peer that holds a
# reusable prefix; forwarded untouched to the HiCache storage backend.
# See GenerateReqInput.kv_router_hint.
kv_router_hint: Optional[Dict] = None,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 — The synchronous Engine API needs the same propagation. async_generate() exposes and forwards this field, but Engine.generate() neither accepts it nor puts it in GenerateReqInput. Synchronous in-process users therefore cannot use hints. Add one tested transport path across both APIs; that is a natural place for the generic typed/versioned hint envelope.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Declared and forwarded on both entrypoints — generate() at engine.py:525/:568, matching async_generate() at :408/:451.

configured = int(self._config.control_port)
if configured <= 0:
return _ephemeral_port()
return configured + _rank_port_offset(self._storage_config)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 — Validate the rank-expanded port range. A permitted base port of 65535 with TP=2 makes rank 1 use port 65536 here. Socket failures then occur asynchronously from the progress thread. Validate base + max_rank_offset <= 65535 (and endpoint input ranges) during configuration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in two places, since neither alone is enough: kvcr_config.py:132 bounds the base port, and kvcr_store.py:581 re-checks base + highest rank offset, since only the store knows the rank layout. The ceiling comes from _highest_rank_port_offset (:400), which mirrors _rank_port_offset with every coordinate maxed — docstring says the two must be edited together.

Rank-dependent, which is what made it nasty: rank 0 binds fine and only the high ranks fail, so it presents as "P2P works for some workers".

@ishandhanani ishandhanani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rereview at 0b06fd644f: the latest test reduction is a net scope improvement, but it does not resolve the backend blockers. In a fresh editable build, public KVCR c573de3 installs but KVCRStore fails to import; the 20 retained real-backend tests then all skip behind their ImportError guard. The earlier clear, synchronous API, and rank-port comments remain unresolved. Please split a neutral, fully propagated hint envelope from this backend-specific adapter, and remove external-product coupling from SGLang documentation and behavior.

can notice. Both knobs are operator-settable, so the ordering is
enforced here rather than left as a comment on the defaults.
"""
if self.get_timeout_s * 1000.0 <= self.operation_timeout_ms:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Ordering two timers is not a DMA-completion fence. The public core can release a cancelled native transfer without a terminal/quiescence acknowledgement, while _drain_until returns and HiCache can reuse the destination pages after get_timeout_s; the target also accepts a successful late delivery. Thus a source write that starts near its deadline can still DMA into pages reused by a later prefetch, producing silent wrong KV. Keep those pages until a real terminal/cancellation fence, or require that contract from the core; comparing these two wall-clock settings cannot establish safety.

request_id = self._hint_request_id()
self._kvcr.submit_hint(
(),
src=hint.source_control_endpoint,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — A public application client controls src. /generate deserializes GenerateReqInput directly, the parser accepts any nonempty source_control_endpoint, and this passes it to submit_hint; eager control connection is enabled by default. A caller can therefore make a KVCR-enabled worker attempt a control-plane connection to an arbitrary endpoint, despite the model comment calling hints trusted-router-only. Inject this only at an authenticated internal boundary, or validate/authorize the endpoint before submission.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. _split_control_endpoint (kvcr_store.py:276) validates transport, port range, and bind wildcards before the endpoint reaches submit_hint. Rejects non-tcp://, port 0 and >65535, missing host, and 0.0.0.0 / :: / [::] /*. Loopback stays dialable — colocated workers are the normal topology.

To be explicit: this checks whether the endpoint is dialable, not whether the peer is one we should trust. Nothing in the hint is authenticated. kv_hints is documented as router-set, but enforcing that is the ingress's job (RFC #36224) and isn't reconstructible in the backend — the _parse_hint docstring now says so rather than leaving it assumed.

Comment thread python/sglang/srt/managers/io_struct.py Outdated
@@ -939,6 +947,7 @@ def __getitem__(self, i):
),
routed_dp_rank=self.routed_dp_rank,
disagg_prefill_dp_rank=self.disagg_prefill_dp_rank,
kv_hints=self.kv_hints,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 — Batched hints are not per-request. The new field is a single dict and batch splitting copies that same object into every child. A routed batch with distinct source peers/prefixes cannot express correct hints: all items receive the whole envelope and this backend declines it as malformed or shares the wrong source. Give the neutral envelope the same scalar-or-list normalization and indexed propagation as other request metadata, with a focused transport test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. _normalize_kv_hints (io_struct.py:833) is dispatched from _normalize_batch_inputs (:571), and __getitem__ indexes it (:980).

@linhu-nv

linhu-nv commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @ishandhanani useful pass. All in 596e68dae6, inline replies on #3, #4, #5, #7, #8.

#2 and #6: I did not close this. KVCR's abort() is a no-op stub, so there's no per-op cancellation to call, and a deadline is a timer, not a DMA fence — a descriptor the NIC has already begun can still land after the handle is released.
The commit only makes it observable: an op we gave up on that comes back now gets its own counter and a warning (kvcr_store.py:483, :758) instead of being dropped as an ordinary late tick. The #6 docstring did oversell this and now says what the check actually buys and what closing it needs (a per-op quiescence signal from KVCR, filed upstream). If unfenced-but-observable shouldn't merge, I'll gate the remote path behind an opt-in until KVCR lands abort().

Optional dependency is half-done. Import smoke test is in, so the suite is. green without nvidia-kvcr. Packaging isn't — which CI tier do you want it in?

A KV hint is advisory routing metadata attached by a trusted orchestrator
(a router or a scheduler) after it has picked a worker -- never by an
application client. This adds the transport only: the envelope travels
unchanged from the entrypoint down to the scheduler's Req, and nothing in
the core reads a payload.

The envelope is a list of independent actions, each naming its own type
and version, so a consumer implementing one action type ignores -- rather
than rejects -- an action it does not implement sitting next to it. The
payload is therefore untyped here; the component that implements an
action owns its schema and validates it. No action types are defined by
this change.

Wire-compatible with vLLM's KVHintsEnvelope (vllm-project/vllm#53423) so
one orchestrator can address either engine with the same envelope.

Path: GenerateReqInput (HTTP/Engine) and the gRPC KVHintsEnvelope message
-> normalization (single, per-batch-item, and parallel-sample expansion)
-> TokenizedGenerateReqInput over msgpack IPC -> Req.
Plugs nvidia-kvcr in as a `HiCacheStorage` backend
(`--hicache-storage-backend kvcr`), giving HiCache an L3 tier that can
serve a prefix another instance computed:

- offload: the host tier writes pages into KVCR's DRAM tier (deposit).
- local fetch: pages come back from that tier.
- remote fetch: when the request carries a KV-hint envelope naming a
  peer, KVCR pulls those pages from that peer over NIXL instead of
  recomputing them.

The hint is what makes the remote case possible at all -- SGLang cannot
know which peer holds a prefix, and the destination cannot probe a
peer's residency. `router_hint.py` reads the `kv.fetch@1.0` action out
of the envelope the request already carries, normalizes the router's
block hashes onto SGLang page keys, and re-emits the envelope KVCR's
own parser consumes. A hint that is malformed, empty, or names an
unreachable peer degrades to a local miss; it never raises into a
HiCache storage thread, which would permanently disable L2/L3.

Both prefetch controller stacks are threaded (HiCacheController and
HybridCacheController), and both host-memory modes work -- the hint
rides the operation, not the host tier.

Config validation refuses two combinations that otherwise fail
silently: a remote-hint setup with no advertisable control endpoint,
and a get timeout shorter than the core's own operation deadline
(which would let an uncancellable transfer write into host pages
HiCache has already reused).

Verified end to end on two instances behind a dynamo router, in both
host-memory modes, under TP>1/DP>1 and concurrency, and against a
frozen/dead/restarted source. See the README in the backend directory
for how to run it, the version pins, and the known gaps.
A storage backend whose transfers are driven by daemon threads in the
scheduler process -- the HiCache prefetch daemon, or a P2P backend's own
progress thread -- only runs when the main loop lets go of the GIL. An
iteration that ran a forward already yielded; an iteration that scheduled
nothing spins pure Python, and CPython will not preempt it for a thread it
cannot see waiting. Measured on a two-instance P2P pair: a fetch from an
*idle* source missed 5 of 8 times and blew a 2s budget at 5.07s, while the
same fetch from a *busy* source passed every time -- the forward was the
only thing releasing the GIL. So the starvation is worst exactly where the
loop is quiet, which is the case a throughput benchmark never exercises.

Generalizes the yield sgl-project#38504 added to the on_idle stall path:

- The yield moves into `_yield_to_storage_threads()` and is called from
  every loop that can complete an iteration without launching a batch,
  including the overlap loops (base, disagg decode, disagg prefill) where
  `on_idle` is skipped while `last_batch` is still pending, and from
  `maybe_sleep_on_idle` on ranks that have no idle sleeper at all
  (--sleep-on-idle defaults off, and init_idle_sleeper nulls the sleeper on
  every rank that is not pp0/attn-tp0/attn-cp0).
- A backend may now declare a poll cadence via
  `HiCacheStorage.idle_poll_timeout_ms()`, in which case the yield parks for
  it rather than calling `sleep(0)`. `sleep(0)` hands the GIL straight back
  if nothing else is runnable; a transfer needing several handoffs to finish
  needs a real window. The same cadence caps the `--sleep-on-idle` park,
  which otherwise waits a second on request sockets that will never see
  traffic for work a peer is driving. Shorten, never lengthen: the floor is
  1ms, and a backend declaring nothing keeps today's behavior.
- `HiCacheStorage.tick()` gives a backend a scheduler-thread callback out of
  `check_hicache_events`, so one whose state machine only advances when
  somebody calls it does not need a polling thread of its own. It must not
  block -- the caller is the thread that runs forward passes.

Both seams are no-ops for every in-tree backend.
A KVCR worker only serves a peer's pull while somebody calls
``poll_completed()``, and an idle worker has no traffic of its own to poll
for. That was a daemon thread, ``kvcr-source-pump``, polling every 5ms.

The thread was the problem. Its wakeups only land when the scheduler
releases the GIL, and a scheduler with nothing to run never does -- measured
~1.08s of pure wakeup delay inside a 1.15s source-side serve, against a
2s fetch budget. Moving the poll onto ``HiCacheStorage.tick()`` puts it on
the thread that was holding the GIL in the first place, so the handoff count
is zero, and drops a thread, its fault-tolerance state machine, and its
join-on-close hazard.

``idle_poll_timeout_ms()`` returns 1ms while this backend is a live source,
matching kvcr's own progress-thread idle wait, so ``--sleep-on-idle`` parks
for a tick interval instead of a second on sockets a peer's pull never
touches. Returning 0 was the first attempt and was wrong: a loop that never
parks starves the threads driving the transfer it is waiting for.

Close gets simpler and stricter. Retiring the core under ``_poll_lock``
fences both pollers -- holding the lock means no poll is in progress,
clearing the field means none starts -- which replaces a join that could
time out and then had to leak the core rather than close under a live poll.

Also fixes the timeout ordering the config validates. ``operation_timeout_ms``
is when the core starts cancelling, not when it stops driving; a source that
misses it keeps going until ``abandon_timeout_ms``. So ``get_timeout_s`` must
outlast the abandon deadline, not the operation one, or HiCache frees host
pages a peer may still write into. ``abandon_timeout_ms`` is now an explicit
knob (the core requires >= 2x operation_timeout_ms and refuses to construct
otherwise, so it is checked at parse time rather than mid-boot on the
scheduler thread), and ``get_timeout_s`` moves 30s -> 45s to stay above it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation hicache Hierarchical Caching for SGLang memory-pool unified-radix-cache

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants