feat(sglang): enable HiCache L3 file backend - #4213
Conversation
… headers The ai namespace had accumulated config that restates what the API server or the chart already supplies, plus comments that outlived the thing they described. Removed only fields proven inert: - VirtualMCPServer serviceType/db and HTTPRoute parentRefs group/kind + PathPrefix matches are CRD/Gateway-API defaults. Confirmed with `kubectl apply --dry-run=server`: the API server refills every one, so the stored objects are unchanged. `config.aggregation` was deliberately NOT removed - it is not defaulted when the parent object is absent, and it controls tool-name prefixing. - llmkube-models `timeout: 60m` never applied: the cluster-apps patch strategic-merges `timeout: 5m` onto every child Kustomization. `flate diff` shows no rendered change from deleting it. - llmkube `prometheusRule.enabled: false` and litellm's three secretKeyRef blocks restate a chart default and an envFrom that the CRD supports. - memini dependsOn llmkube-models was already implied through litellm. Comment fixes: the model-cache comment named the two iGPU nodes backwards (qwen3-embedding is on control-2, not control-3), a memory note cited a revision sha that appears nowhere in the repo, and the /dev/dri supplementalGroups block was explained verbatim in three files. Repo-wide consistency: 115 missing `yaml-language-server` schema headers added across 71 files (core/v1 kinds skipped - that host serves no schema for them), and the nine raw-githubusercontent kustomization schema URLs normalised to json.schemastore.org. Docs: deleted the completed sglang-oci-cutover plan per its own exit criterion, repointed three links at a directory that no longer exists, and corrected the sglang pin from v0.5.14 to the v0.5.15 actually running.
Five MCPToolConfigs carried `toolsFilter: []`, which the CRD documents as "If empty, all tools are exposed" - they filtered nothing, and the ten toolConfigRef fields pointing at them were dead weight. Verified on the live cluster rather than from the doc text, because the two paths could have diverged in the operator. Removed the toolConfigRef from MCPServer kubesearch with `kubectl patch` and re-ran an MCP tools/list against the vmcp-resources gateway: 23 tools before, 23 after, names byte-identical, including all ten kubesearch_* entries. Live state has been restored. That test also retired the reason this was previously skipped. The concern was that restarting the MCP servers would park the gateways for several minutes; the observed recovery was 34 seconds, and the aggregating gateway pods never restarted at all. NOT removed, because these three do real work despite looking similar: - grafana: a genuine 33-tool allow list. - karakeep and searxng: no toolsFilter at all - they use toolsOverride to rename tools (search-bookmarks -> search_bookmarks). Dropping those refs would have renamed eleven live tools. The orphaned `github` MCPToolConfig is left alone here; that file is rewritten on feat/github-mcp-http.
…n experiments Renamed sglang-benchmarks.md to engine-benchmarks-gfx1201.md: the file was never SGLang-specific - it is a vLLM / SGLang / llama.cpp comparison for Qwen3.6-27B on RDNA4, and its 2026-06-21 round actually concluded in favour of vLLM. Restored the "Open / next experiments" list, which the previous commit wrongly deleted as stale scaffolding. Five of those are outstanding work, not history. Added the SGLang revalidation item: when upstream rebases the RDNA4 patch series past our FORK_REF pin, re-run the bench/ concurrency sweep and verbatim test and compare against the recorded v0.5.15 numbers before moving the production pin. Reframed the header from "SUPERSEDED / historical" to a dated measurement series with a live experiment queue, and dropped only the generic Process Instructions boilerplate. Updated the one inbound link in sglang-blockers.md.
…uille/cluster into chore/simplify-ai-namespace
Flux's CEL status evaluator returns InProgressStatus when no expression matches (fluxcd/pkg runtime/cel/status_evaluator.go, final return), so an inProgress expression only earns its place if its truth set overlaps failed or current. Here it cannot. The InferenceService phase enum is Pending, Creating, Progressing, Ready, WaitingForGPU, Stopped, Suspended, Failed. inProgress matched Pending/Creating/Progressing/WaitingForGPU, failed matches Failed, current matches Stopped or Ready+Available - three disjoint sets over one scalar field, so every phase lands on the same verdict with the expression removed. Suspended already fell through to InProgress and still does. Also brings this in line with the only two other healthCheckExprs in the repo (cert-manager, rook-ceph), which both use failed + current alone.
…espace # Conflicts: # kubernetes/apps/ai/llmkube/models/vmcp-embedding.yaml
Review caught that sglang-blockers.md and docker/sglang-rdna4/README.md disagreed about the retired PVC-rebuild script. The README also told readers to keep the Dockerfile in sync with a file that no longer exists, and linked the cutover doc this branch removes. Both now name the exact recovery command instead of gesturing at git history: `git show b8f12ae^:kubernetes/apps/ai/sglang/app/scripts/sglang-env-rebuild.sh`, which was checked and returns all 151 lines. The "keep them in sync" instruction is gone, since the Dockerfile is now the only build path.
L3 was recorded as failing restart recovery on 07-15. Two defects sat upstream
of that measurement: the hash extension could not JIT-compile without OpenSSL
headers in the runtime stage, and write_through_selective gated promotion on
hit_count >= 2 so first-pass content never reached L2 for L3 to back up.
Retested 07-27 on sha256:72d934d3 under plain write_through. After a cold
restart, prefetched_tokens_total{storage_backend="file"} read 63,205, and L2
accounting closes exactly: 73,120 newly backed + 63,205 prefetched = 136,325
hicache_host_used_tokens.
L3 gets its own PVC rather than sharing the triton cache, and the evictor is
bounded: it defaults to unbounded, and openebs-hostpath enforces no quota.
MIN_FREE_SPACE guards the node's shared root independently of cache size.
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✅ Created PR with unit tests: #4214 |
…espace # Conflicts: # kubernetes/apps/ai/toolhive/config/github.yaml
…espace # Conflicts: # kubernetes/apps/observability/siren/app/helmrelease.yaml
…se one opencode renders from app-template, and the repo pairs that chart with the bjw-s schema in 52 of 53 cases.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/llm-hosting/vllm-vs-sglang-2026-07.md`:
- Around line 187-189: Update the downloader guidance in the referenced passage
to state that comparing HEAD Content-Length only detects truncation and is not
cache validation. Explicitly retain conditional GET plus digest or format
validation, and require the “revalidated” status to be logged only when the
server returns an actual 304 response.
In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml`:
- Around line 134-149: Update the HiCache configuration around the `hicache`
volume and `/hicache` mount so each model/cache-format revision uses an isolated
revisioned subdirectory, or otherwise ensure the `qwen36-27b-hicache` PVC is
rotated whenever incompatible model, tokenizer, or cache-format changes roll
out. Preserve the existing cache volume wiring while preventing startup scans
from reusing stale pages across revisions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b0c1ddb7-e47e-4eaf-bacc-cae2256d3398
📒 Files selected for processing (3)
docs/llm-hosting/sglang-blockers.mddocs/llm-hosting/vllm-vs-sglang-2026-07.mdkubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Tanguille/LLMKube(auto-detected)
The page filename is sha256(token_ids, prior_hash, page_size) plus
_{model_name}_{tp_rank}_{tp_size}, so it encodes neither the weights revision nor
the KV layout. Renovate bumps both the AWQ revision and the sglang image, and
either would have served KV pages computed by the previous one.
Also drop the claim that a Content-Length compare validates the download cache.
|
Both remarks were valid. Resolved in HiCache stale-page reuse: confirmed and fixed. Traced the key derivation in v0.5.15 rather than assuming. That is exploitable here specifically because Caveat worth stating plainly: that suffix is bumped by hand, so it can rot. It sits directly above the pinned image and revision it tracks to keep the two visible together, but the durable fix is for the backend to fold a model/engine fingerprint into Content-Length: reworded. The point stands, with one correction to the framing. The primary fix is |
# Conflicts: # kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml
# Conflicts: # kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml
@@ spec.env @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! + three list entries added:
+ - name: SGLANG_HICACHE_FILE_BACKEND_STORAGE_DIR
+ value: /hicache/sglang-v0.5.15_awq-f541031d
+ - name: SGLANG_HICACHE_FILE_BACKEND_MAX_SIZE
+ value: 64Gi
+ - name: SGLANG_HICACHE_FILE_BACKEND_MIN_FREE_SPACE
+ value: 100Gi
@@ spec.extraArgs @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! - one list entry removed:
- - write_through_selective
! + three list entries added:
+ - write_through
+ - "--hicache-storage-backend"
+ - file
@@ spec.extraVolumeMounts @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! + one list entry added:
+ - name: hicache
+ mountPath: /hicache
@@ spec.extraVolumes @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! + one list entry added:
+ - name: hicache
+ persistentVolumeClaim:
+ claimName: qwen36-27b-hicache
@@ (root level) @@
# v1/PersistentVolumeClaim/ai/qwen36-27b-hicache
! + one document added:
+ apiVersion: v1
+ kind: PersistentVolumeClaim
+ metadata:
+ name: qwen36-27b-hicache
+ namespace: ai
+ labels:
+ kustomize.toolkit.fluxcd.io/name: llmkube-models
+ kustomize.toolkit.fluxcd.io/namespace: ai
+ spec:
+ resources:
+ requests:
+ storage: 64Gi
+ storageClassName: openebs-hostpath
+ accessModes:
+ - ReadWriteOnce
|
1 participant
L3 was recorded as failing restart recovery on 07-15. That was the wrong conclusion: two defects sat upstream of the measurement, neither a property of L3.
hash_binding.cppon the first prefill and could not find<openssl/sha.h>in the runtime stage. Lazy, so it passed startup and health checks then killed the scheduler mid-request.write_through_selectivegates promotion onhit_count >= 2, so first-pass content never reached L2 and L3 had nothing to back up.Retested 2026-07-27 on
sha256:72d934d3under plainwrite_through.backuped_tokens_total267,978hicache_host_used_tokens267,978 of 274,861prefetched_tokens_total{storage_backend="file"}63,205L2 accounting closes exactly: 73,120 newly backed + 63,205 prefetched = 136,325
hicache_host_used_tokens, so every L2 token is either a fresh write or a read-back from disk.Populate (58,071 tok) PP 196.9 tok/s. Extension probe (58,209 tok) PP 252.9 tok/s, TG 0.56 tok/s. Both ran with production traffic on the GPU, so both are floors. Reuse is validated with an extension probe, not a resend, because DeltaNet hybrids report
#cached-token: 0on identical resends by design.Sizing
The evictor defaults to unbounded and
openebs-hostpathenforces no quota, so without a cap L3 grows until control-1's 500G root fills.MAX_SIZE=64Gibounds the cache;MIN_FREE_SPACE=100Giis the independent guard, since a cache under its own cap can still push the node past kubelet's ~50G nodefs threshold when other tenants grow.L3 gets its own PVC instead of sharing the 10Gi triton cache, so the declared size matches what it holds. The class forbids expansion, so a nominal request could not be corrected later.
Notes
/cache/sglang/hicachepath. It holds prompt-derived data, so deletion needs explicit approval.Summary by CodeRabbit
New Features
Bug Fixes
Documentation