perf(sglang): reclaim mamba VRAM, cache L3 metadata, prune stale generations - #4264
Conversation
…rations Decode was never the problem: solo throughput held flat at 14-15 tok/s across 14d. TTFT was, and it is prefill time over a KV pool that is tighter than it needs to be. Mamba slots and full-KV tokens come out of one byte budget, so unused slots are KV tokens forgone. Measured 4.46G across 60 slots (74.3MB each) against 5.60G for 183,240 KV tokens. Peak slots actually used over 14d is 32 and peak concurrency is 16, so 60 was sized for load that never arrives; it also silently capped max_running at min(32, 60//3) = 20, making maxRunningRequests: 32 dead config. 48 = 16 running x the no_buffer ratio of 3, freeing ~892M for ~29K more KV tokens (+16% pool). The L3 store holds 619,346 files, and every lookup walked it with os.scandir/os.path.exists. Upstream #29716 shipped a metadata cache in v0.5.16 that skips the traversal (P99 TTFT -22.1% at 157K files) but left it off by default. The storage dir is versioned per image digest because reuse across a bump serves stale KV pages, and nothing pruned the old directory: the CRD has no initContainers field, so cleanup was a manual kubectl exec and a missed one orphaned 64G. That is not just disk hygiene. control-1's disk is a sparse zvol on TrueNAS SSD_Pool, which had 60.3G available; orphaned generations consume real pool space and exhausting it stops the VM. The CronJob keeps the newest generation plus anything touched in the last day, so a rollout's draining pod keeps its cache.
|
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 (1)
📝 WalkthroughWalkthroughChangesHiCache lifecycle and SGLang runtime configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CronJob as qwen36-27b-hicache-prune CronJob
participant PVC as qwen36-27b-hicache PVC
participant Shell as pruning command
CronJob->>PVC: Mount at /hicache
CronJob->>Shell: Start scheduled pruning as UID 0
Shell->>PVC: Inspect generation directories by mtime
Shell->>PVC: Delete superseded generations older than 24 hours
Shell->>CronJob: Log remaining generation count
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@@ spec.env @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! - four list entries removed:
- - name: HIP_VISIBLE_DEVICES
- value: "0"
- - name: ROCR_VISIBLE_DEVICES
- value: "0"
- - name: GPU_DEVICE_ORDINAL
- value: "0"
- - name: CUDA_VISIBLE_DEVICES
- value: "0"
! + two list entries added:
+ - name: SGLANG_HICACHE_FILE_BACKEND_ENABLE_METADATA_CACHE
+ value: "1"
+ - name: SGLANG_HICACHE_FILE_BACKEND_METADATA_TTL
+ value: "-1"
@@ spec.extraArgs @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! - six list entries removed:
- - "--dtype"
- - bfloat16
- - "--pre-warm-nccl"
- - "--trust-remote-code"
- - "--disable-custom-all-reduce"
- - "60"
! + one list entry added:
+ - "48"
@@ spec.sglangConfig @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b
! - three map entries removed:
- maxRunningRequests: 32
- quantization: awq
- tensorParallelSize: 1
@@ (root level) @@
# batch/v1/CronJob/ai/qwen36-27b-hicache-prune
! + one document added:
+ apiVersion: batch/v1
+ kind: CronJob
+ metadata:
+ name: qwen36-27b-hicache-prune
+ namespace: ai
+ labels:
+ kustomize.toolkit.fluxcd.io/name: llmkube-models
+ kustomize.toolkit.fluxcd.io/namespace: ai
+ spec:
+ concurrencyPolicy: Forbid
+ failedJobsHistoryLimit: 3
+ jobTemplate:
+ spec:
+ backoffLimit: 2
+ template:
+ spec:
+ containers:
+ - name: prune
+ image: "ghcr.io/home-operations/busybox:1.38.0@sha256:7e2c04dd50ede647bf4a7a4c8dbd629dd4971cd139b9b88fb22bfc3c7a6c13df"
+ command:
+ - /bin/sh
+ - "-c"
+ - |
+ set -eu
+ cd /hicache
+ # Newest mtime = the live generation (write_through writes every prefill,
+ # so anything serving stays fresh). Skip it and anything written in the
+ # last day, so a draining pod keeps its cache through a rollout.
+ newest="$(ls -1dt -- */ 2>/dev/null | head -1)"
+ for d in */; do
+ [ -d "$d" ] || continue
+ [ "$d" = "$newest" ] && continue
+ [ -n "$(find "$d" -maxdepth 0 -mmin +1440)" ] || continue
+ echo "pruning superseded generation: $d"
+ rm -rf -- "$d"
+ done
+ # Names/counts only: du here would stat the whole store (682,508 files on
+ # 2026-07-30) on top of the walk rm already does, just to log a number.
+ echo "remaining: $(ls -1d -- */ 2>/dev/null | wc -l) generation(s)"
+
+ resources:
+ limits:
+ memory: 128Mi
+ requests:
+ cpu: 10m
+ memory: 64Mi
+ volumeMounts:
+ - name: hicache
+ mountPath: /hicache
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ readOnlyRootFilesystem: true
+ restartPolicy: OnFailure
+ securityContext:
+ runAsUser: 0
+ volumes:
+ - name: hicache
+ persistentVolumeClaim:
+ claimName: qwen36-27b-hicache
+ schedule: "30 4 * * *"
+ successfulJobsHistoryLimit: 1
|
resolve_max_num_reqs() takes min(requested, max_mamba_cache_size // 3); with it unset the other branch takes min(estimated, ...) where estimated floors at 2048. Both land on 48//3 = 16, so the value only ever misled about the real concurrency ceiling.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml (1)
96-99: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider adding resource limits alongside requests.
Only
requestsare set for the prune container; withoutlimitsit has no upper bound on CPU/memory during a run over 619K files (e.g. ifdu/rmbehave unexpectedly on a large superseded generation).🤖 Prompt for 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. In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml` around lines 96 - 99, Add CPU and memory limits alongside the existing requests in the prune container’s resources configuration. Choose explicit upper bounds appropriate for its large-file cleanup workload, while preserving the current request values.
🤖 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 `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml`:
- Around line 70-99: Harden the prune container’s securityContext by retaining
runAsUser: 0 while setting allowPrivilegeEscalation: false,
readOnlyRootFilesystem: true, and dropping all Linux capabilities. Apply these
settings to the container named prune, preserving its existing /hicache volume
access and pruning behavior.
---
Nitpick comments:
In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml`:
- Around line 96-99: Add CPU and memory limits alongside the existing requests
in the prune container’s resources configuration. Choose explicit upper bounds
appropriate for its large-file cleanup workload, while preserving the current
request values.
🪄 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: b4fc3f5f-bc6f-443c-89ae-d57b2f766014
📒 Files selected for processing (1)
kubernetes/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)Tanguille/toolhive(auto-detected)Tanguille/2x-R9700-RDNA4-GFX1201-sglang-inference(auto-detected)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml (1)
42-104: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDo not use directory mtime as a proxy for recent cache access.
Directory mtime changes when entries are created or deleted, not when files inside are read. A generation used during a rollback but not written within 24 hours can therefore be deleted, contradicting the stated “recently accessed” retention guarantee. Keep the configured active suffix explicitly and use an access marker or file-level access policy for rollback safety. (man7.org)
🤖 Prompt for 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. In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml` around lines 42 - 104, Replace the mtime-based retention logic in the qwen36-27b-hicache-prune CronJob with explicit protection for the configured active generation suffix and a file-level access policy or access marker that reflects cache use during rollbacks. Do not select or retain generations based on directory mtime; update the pruning loop so only generations confirmed inactive by the new policy and older than the retention window are removed.
🤖 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.
Outside diff comments:
In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yaml`:
- Around line 42-104: Replace the mtime-based retention logic in the
qwen36-27b-hicache-prune CronJob with explicit protection for the configured
active generation suffix and a file-level access policy or access marker that
reflects cache use during rollbacks. Do not select or retain generations based
on directory mtime; update the pruning loop so only generations confirmed
inactive by the new policy and older than the retention window are removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b1a2939b-d99a-4cee-a935-bf57c8222bf8
📒 Files selected for processing (1)
kubernetes/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)Tanguille/toolhive(auto-detected)Tanguille/2x-R9700-RDNA4-GFX1201-sglang-inference(auto-detected)
AI Automated Review (incremental)Analysis engine: qwen-3.6-fast@http://litellm.ai.svc.cluster.local/v1 (openai) Recommendation: Approve This incremental delta (from Change-by-change findings
Standards Compliance
Unknowns or Needs Verification
|
All verified against the running image's source, not inferred: --pre-warm-nccl gated on tp/pp/ep > 1 (distributed/bootstrap.py:109) --disable-custom-all-reduce never built at world_size 1 (parallel_state.py:438) --trust-remote-code model dir has 0 auto_map and 0 .py files --dtype bfloat16 auto resolves to the checkpoint's own dtype quantization: awq auto-detected; awq_marlin hard-returns False off CUDA tensorParallelSize: 1 the default *_VISIBLE_DEVICES x4 container gets one render node; torch reports 1 device Left alone deliberately: --hicache-write-policy restates the default but guards a documented regression, and --disable-overlap-schedule is forced True on this arch without every resolution-pass reader being audited. Also correct two things in the previous commit. The metadata-cache startup scan is a single os.listdir with no per-file stat, measured 0.31s over the live store, so the CrashLoopBackOff warning was unfounded; and its 5s default TTL expires every entry the scan inserts, so pin TTL to the never-expire sentinel.
Drop capabilities, forbid privilege escalation and make the root filesystem read-only; uid 0 is only needed to unlink root-owned generations, and the script never writes outside the mounted /hicache. Verified by running the script unchanged under this exact securityContext on the real busybox image: it pruned the stale generation and kept both the live and the draining one. Add limits alongside the requests, matching how the rest of the repo bounds its workloads, so a large generation cannot crowd the inference server that shares control-1. Also state plainly in the comment that mtime tracks writes rather than reads, and why that is sound here.
|
Addressed in 7f59559. Fixed
That test also caught a real gap in my own earlier validation: I had exercised the script under GNU coreutils, not busybox, so Not taking: "do not use directory mtime as a proxy for recent cache access" The premise is right (mtime tracks writes, not reads) but the failure mode does not exist here. The suggested alternative, reading the configured active suffix, is worse at this altitude: it needs RBAC to read the CR from a job that currently mounts nothing but the PVC, couples the pruner to the manifest's env-var shape, and fails badly if the pod is down longer than the window, where it could delete the only remaining generation. mtime degrades gracefully instead, since "newest wins" always protects something. The genuine residual risk is rolling back to an image more than a day after a bump and finding its cache already pruned. That is a cold-TTFT cost on rollback, not a correctness issue, and it is documented in the manifest. Comment reworded to say "written" rather than "touched" so the mechanism is not overstated. |
@@ data.config.yaml @@
# v1/ConfigMap/observability/kube-state-metrics-customresourcestate-config
! ± value change in multiline text (no inserts, one deletion)
spec:
resources:
- groupVersionKind:
group: kustomize.toolkit.fluxcd.io
[330 lines unchanged)]
webhook_path:
- status
- webhookPath
name: resource_info
- - groupVersionKind:
- group: toolhive.stacklok.dev
- kind: VirtualMCPServer
- version: v1beta1
- metricNamePrefix: toolhive
- metrics:
- - each:
- info:
- labelsFromPath:
- name:
- - metadata
- - name
- type: Info
- help: The current state of a Toolhive VirtualMCPServer resource.
- labelsFromPath:
- exported_namespace:
- - metadata
- - namespace
- phase:
- - status
- - phase
- ready:
- - status
- - conditions
- - '[type=Ready]'
- - status
- name: resource_info
@@ rules @@
# rbac.authorization.k8s.io/v1/ClusterRole/kube-state-metrics
! - one list entry removed:
- - resources:
- - virtualmcpservers
- apiGroups:
- - toolhive.stacklok.dev
- verbs:
- - list
- - watch
|
Comments were half the added lines. Cut to the measured facts and the source citations, and stop stating the same thing twice: the generation-versioning rationale now lives only above STORAGE_DIR, and the mamba clamp formula only above --max-mamba-cache-size, with pointers from the other site. Drop the du on the doomed directory: rm -rf already walks it, so sizing it first paid a second full stat pass of a ~600K-file tree on a sparse zvol purely to decorate a log line. Same reasoning already applied to the live generation one line below. Drop the cpu limit, keep the memory one. The job is iowait-bound unlinking files, where CFS quota cannot throttle at all, and in the cache-hot case it would only stretch the run and widen the contention window it was meant to narrow. Note that raising TP means re-adding the flags dropped as TP=1 no-ops, since nothing else would prompt it. Prune logic re-verified unchanged under busybox with the hardened securityContext: stale generation removed, live and draining kept.
1 participant
Decode never regressed: solo throughput held flat at 14-15 tok/s across 14d. TTFT did, and it is prefill time against a KV pool tighter than it needs to be.
Median prefill matches the 197 tok/s baseline, so the kernel is fine. The trigger was workload: prompts roughly doubled and request rate rose 3-8x.
Mamba cache 60 -> 48
Mamba slots and full-KV tokens come out of one byte budget, so unused slots are KV tokens forgone.
48 = 16 running x the no_buffer ratio of 3. Frees ~892M, worth ~29K more KV tokens (+16% pool).Side effect:
max_running = min(32, 60//3) = 20, somaxRunningRequests: 32has been dead config.L3 metadata cache
The store holds 619,346 files and every lookup walked it with
os.scandir/os.path.exists. Upstream #29716 shipped a metadata cache in v0.5.16 (P99 TTFT -22.1% at 157K files), off by default. Verified present in the running image athicache_storage.py:391.Caveat noted in-file: it adds a one-time startup scan competing with the ~30min startup-probe budget. Needs timing on next cold restart.
Prune CronJob
The storage dir is versioned per image digest because reuse across a bump serves stale KV pages (
config_suffixonly covers--served-model-nameand TP, both constant across bumps). Nothing pruned the old directory: the CRD has noinitContainersfield, so cleanup was a manualkubectl exec, and a missed one orphaned 64G.That is not just disk hygiene:
SSD_Pool/vm/TALOS.block,refreservation none(sparse)Orphaned generations consume real pool space, and exhausting that pool stops the VM. This is also why the L3 cap stays at 64Gi rather than growing: 61G -> 128G would need ~67G of new allocation against 60.3G free.
Prune keeps the newest generation plus anything touched in the last day, so a rollout's draining pod keeps its cache. Tested against a real tree including the empty-PVC case.
Not changed
--schedule-policy lpm,--schedule-conservativeness- plausible but unmeasured here, worth a separate A/B.enable_prefill_delayer- hardassertagainstdisable_overlap_schedule, whichno_bufferrequires. Will not start.Largest remaining term is the prefill p10/p50 spread (34 vs 187 tok/s). Best candidate is upstream #29677 (Triton extend-attention rectangular grid, AMD-only, still open); we are in its worst case with Triton attention forced on gfx1201 plus
--enable-mixed-chunk. Fork patch-chain job, separate from this.Verification
kubectl kustomizerenders;kubectl apply --dry-run=serveraccepts the CronJob./simplifypass applied: dropped adu -sh .that stat-walked the entire 619K-file live tree daily for a log line.Summary by CodeRabbit
New Features
Improvements