feat(llmkube): tune vllm staging config and document VRAM ceiling - #4179
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughChangesThe PR adds July 2026 vLLM/SGLang measurement documentation and benchmark scripts, documents model-cache recovery procedures, and updates Qwen36 vLLM deployment settings plus SGLang hardware-constraint comments. Qwen36 runtime update
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/llm-hosting/vllm-vs-sglang-2026-07.md (1)
163-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVersion the benchmark harness or record immutable provenance.
Because
.vllm-opt/is untracked and local-only, future readers cannot reproduce or audit these measurements. Commit the scripts, or record their hashes together with exact commands and configuration snapshots.🤖 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 `@docs/llm-hosting/vllm-vs-sglang-2026-07.md` around lines 163 - 166, Update the benchmark documentation around concsweep.py and spectest.py to provide immutable provenance: either version the .vllm-opt harness scripts in the repository or record each script’s hash, exact execution command, and configuration snapshot used for the measurements. Ensure future readers can reproduce and audit both benchmarks.
🤖 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 75-83: The paragraph incorrectly presents findings from the
invalid 0.95 configuration as applicable to 0.875 production configs. Update the
conclusions around the cited SGLang single-stream comparison and OOM behavior to
either add supporting 0.875 measurements or explicitly scope both findings to
the 0.95 configuration, and remove the claim that they carry over.
- Around line 123-136: Update the downloader flow described here to verify the
completed "$dest" file size against the response Content-Length, using the check
only as a truncation guard. Ensure failed or mismatched downloads are not
treated as valid by the existing fallback, while preserving the current behavior
unless the artifact and its .etag are both validated and published atomically.
---
Nitpick comments:
In `@docs/llm-hosting/vllm-vs-sglang-2026-07.md`:
- Around line 163-166: Update the benchmark documentation around concsweep.py
and spectest.py to provide immutable provenance: either version the .vllm-opt
harness scripts in the repository or record each script’s hash, exact execution
command, and configuration snapshot used for the measurements. Ensure future
readers can reproduce and audit both benchmarks.
🪄 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: fc4d7aef-8810-4b2d-bd98-1305c3618f2f
📒 Files selected for processing (3)
docs/llm-hosting/vllm-vs-sglang-2026-07.mdkubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yamlkubernetes/apps/ai/llmkube/models/qwen36-27b-vllm.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Tanguille/LLMKube(auto-detected)
Records the 2026-07 measurement round on the R9700 (Qwen3.6-27B AWQ). The R9700 is shared with Jellyfin transcoding, so the 0.875 VRAM fraction is a hard ceiling rather than a tunable. Both manifests now carry that rationale inline: 0.95 grows the SGLang pool 183,240 -> 261,019 tokens but OOMs on a 13.8K-token prefill, so it is not a valid operating point either way. vLLM staging config (still replicas 0): - pin v0.26.0, measured identical to the nightly on KV pool and throughput - parallelSlots 8 -> 32, doubles aggregate throughput at concurrency 16 - maxModelLen 180000 -> 64000, since 180000 needs ~6.55 GiB KV against a ~4.6 GiB ceiling at 0.875 - set --kv-cache-memory explicitly, raising the pool 48,000 -> 121,302 tokens within the same budget - switch the model pin to the boot-validated QuantTrio AWQ variant The concurrency plateau near 40 tok/s turned out to be KV-pool starvation rather than a GPU decode ceiling: per-stream throughput collapsed 4.85 -> 2.45 going from 8 to 16 streams under a small pool and held at ~4.6 under a large one. Growing the pool within 0.875 is the remaining lever. Also documents an llmkube cache-integrity hazard hit during this round: a config patch during a model download truncates the artifact, and etag-based revalidation cannot detect it.
- scope the 0.95 engine comparison as 0.95-only rather than claiming the single-stream and OOM findings carry over to the 0.875 production configs - narrow the Content-Length suggestion to a truncation guard, since it is not integrity and does not make the artifact and its .etag publish atomically - commit concsweep.py and spectest.py so the recorded numbers are reproducible instead of citing an untracked local directory - cut the duplicated figures and stacked caveats around the comparison table
873c840 to
227008e
Compare
@@ spec.files @@
# inference.llmkube.dev/v1alpha1/Model/ai/qwen36-27b-vllm
! - four list entries removed:
- - model-00001-of-00004.safetensors
- - model-00002-of-00004.safetensors
- - model-00003-of-00004.safetensors
- - model-00004-of-00004.safetensors
! + eight list entries added:
+ - model-00001-of-00008.safetensors
+ - model-00002-of-00008.safetensors
+ - model-00003-of-00008.safetensors
+ - model-00004-of-00008.safetensors
+ - model-00005-of-00008.safetensors
+ - model-00006-of-00008.safetensors
+ - model-00007-of-00008.safetensors
+ - model-00008-of-00008.safetensors
@@ spec.quantization @@
# inference.llmkube.dev/v1alpha1/Model/ai/qwen36-27b-vllm
! ± value change
- compressed-tensors
+ AWQ
@@ spec.source @@
# inference.llmkube.dev/v1alpha1/Model/ai/qwen36-27b-vllm
! ± value change
- hf://cyankiwi/Qwen3.6-27B-AWQ-INT4@e5cc0400fb2403c437c2c40a7c52fb5ae93fda18
+ hf://QuantTrio/Qwen3.6-27B-AWQ@9b507bdc9afafb87b7898700cc2a591aa6639461
@@ spec.extraArgs @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b-vllm
! + two list entries added:
+ - "--kv-cache-memory-bytes"
+ - "4294967296"
@@ spec.image @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b-vllm
! ± value change
- vllm/vllm-openai-rocm:nightly@sha256:bd34cea1fe22f0c9f8fef0e362eac2d8b9292971a114eb95d2cfa1026862f5cd
+ vllm/vllm-openai-rocm:v0.26.0@sha256:5709fafe47123becb2f5e61c32d0b97beff1a629bb40bb753c15464f69a97a18
@@ spec.parallelSlots @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b-vllm
! ± value change
- 8
+ 32
@@ spec.vllmConfig.maxModelLen @@
# inference.llmkube.dev/v1alpha1/InferenceService/ai/qwen36-27b-vllm
! ± value change
- 180000
+ 64000
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/bench/concsweep.py`:
- Around line 46-49: Update the per-stream throughput calculation in the
concsweep aggregation block to divide agg by len(ok), so it reflects only
successful streams; retain the zero-success guard to avoid division by zero.
- Line 10: Validate the command-line port before constructing the request URL:
in docs/llm-hosting/bench/concsweep.py at lines 10-10 and
docs/llm-hosting/bench/spectest.py at lines 7-7, parse sys.argv[1] as an integer
and reject values outside 1..65535, then use the validated integer when building
the localhost URL.
In `@docs/llm-hosting/bench/spectest.py`:
- Around line 13-16: Update the benchmark request and validation around the
`prompt` construction and response handling so the expected `handler_0` through
`handler_12` prefix is defined explicitly and the measured result fails unless
the model output begins with that exact verbatim prefix. Ensure generation can
complete the reproduction by requesting a fixed prefix or increasing
`max_tokens` and permitting EOS, rather than reporting throughput for formatted,
divergent, or truncated output.
In `@kubernetes/apps/ai/llmkube/models/qwen36-27b-vllm.yaml`:
- Around line 131-134: Replace the invalid --kv-cache-memory argument in the
vLLM command configuration with --kv-cache-memory-bytes, preserving the existing
4294967296 value and surrounding profiler sizing settings.
🪄 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: 09a11340-435a-40c3-9b72-7db6d3ecc8e9
📒 Files selected for processing (5)
docs/llm-hosting/bench/concsweep.pydocs/llm-hosting/bench/spectest.pydocs/llm-hosting/vllm-vs-sglang-2026-07.mdkubernetes/apps/ai/llmkube/models/qwen36-27b-sglang.yamlkubernetes/apps/ai/llmkube/models/qwen36-27b-vllm.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Tanguille/LLMKube(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/llm-hosting/vllm-vs-sglang-2026-07.md
… scripts --kv-cache-memory only worked via argparse prefix matching; v0.26.0 defines --kv-cache-memory-bytes. Bench scripts now validate the port argument, divide per-stream throughput by successful streams, and verify the spectest continuation is a verbatim prefix of the source.
|
Resolved the remaining review remarks in 2c8e9ee:
Separately, the |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/llm-hosting/bench/spectest.py (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake
zipstrictness explicit.Ruff B905 flags this call. Since unequal lengths are intentional for prefix comparison, use
strict=Falseto document that behavior and keep the lint check clean; verify the repository’s Python target supports this parameter.Proposed fix
-n = next((i for i, (a, b) in enumerate(zip(expect, got)) if a != b), min(len(expect), len(got))) +n = next((i for i, (a, b) in enumerate(zip(expect, got, strict=False)) if a != b), min(len(expect), len(got)))🤖 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 `@docs/llm-hosting/bench/spectest.py` at line 40, Update the zip call in the comparison expression computing n to pass strict=False explicitly, preserving intentional prefix comparison for unequal expect and got lengths. Confirm the repository’s supported Python target includes the strict parameter before applying the change.Source: Linters/SAST tools
🤖 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/bench/spectest.py`:
- Around line 38-42: Update the success condition in the verbatim comparison
around expect, got, and n so it reports OK only when got starts with the
complete expect string. Ensure truncated or empty continuations are reported as
DIVERGED, while preserving the existing divergence-position calculation and
output format.
---
Nitpick comments:
In `@docs/llm-hosting/bench/spectest.py`:
- Line 40: Update the zip call in the comparison expression computing n to pass
strict=False explicitly, preserving intentional prefix comparison for unequal
expect and got lengths. Confirm the repository’s supported Python target
includes the strict parameter before applying the change.
🪄 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: 97ef0099-a6f8-4b1c-a32f-67851d78beaa
📒 Files selected for processing (4)
docs/llm-hosting/bench/concsweep.pydocs/llm-hosting/bench/spectest.pydocs/llm-hosting/vllm-vs-sglang-2026-07.mdkubernetes/apps/ai/llmkube/models/qwen36-27b-vllm.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Tanguille/LLMKube(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (3)
- kubernetes/apps/ai/llmkube/models/qwen36-27b-vllm.yaml
- docs/llm-hosting/bench/concsweep.py
- docs/llm-hosting/vllm-vs-sglang-2026-07.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/bench/spectest.py`:
- Around line 44-45: Update the mismatch-position fallback in the comparison
expression before the “verbatim” print so that, when no differing character is
found, it uses the shorter of expect and got rather than len(got). Preserve the
existing first-mismatch calculation and DIVERGED state behavior.
🪄 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: 1a2fde44-b128-4ed9-8027-4d42f1929e46
📒 Files selected for processing (1)
docs/llm-hosting/bench/spectest.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Tanguille/LLMKube(auto-detected)
Records the 2026-07 measurement round on the R9700 (Qwen3.6-27B AWQ) and applies what it settled.
The binding constraint
The R9700 is shared with Jellyfin transcoding, so 0.875 is a hard ceiling, not a tunable. The ~4.6 GB
rocm-smireports free at 0.875 is the transcoding reservation. Both manifests now carry that rationale inline so it does not get "optimized" away later.Independently, 0.95 is not a valid operating point for SGLang anyway: it grows the pool 183,240 -> 261,019 tokens, then OOMs on a 13.8K-token prefill (
tried to allocate 160 MiB, 0 bytes free). It survives short-prompt decode and dies on the shape we actually serve at p50 35K.vLLM staging config
Still
replicas: 0. This makes staging correct and documented, it does not cut production over.v0.26.0parallelSlots8 -> 32maxModelLen180000 -> 64000--kv-cache-memoryNote the model-source change triggers a re-download into the staging cache PVC on next reconcile.
What this overturned
The decode ceiling was KV-pool starvation, not GPU bandwidth. Per-stream throughput collapsed 4.85 -> 2.45 tok/s going from 8 to 16 streams under a small pool, and held at ~4.6 under a large one, nearly doubling aggregate. Earlier rounds read that plateau as a fixed per-step overhead. Growing the pool within 0.875 is the remaining lever, not kernel-level decode work.
Benchmarks taken right after a boot are unreliable. One config read 32.36 tok/s on its first run and 39.04 / 39.09 on two immediate repeats. Some earlier single-run comparisons were reading warmup as a config difference.
Image tags are not version-ordered. The nightly self-reports a stale
0.23.1rc1base tag, andv0.26.0was built earlier than the nightly despite the higher number.llmkube cache-integrity hazard
Hit during this round and documented in the new doc. Patching an InferenceService while its
model-downloaderis mid-transfer truncates the artifact. The corruption is invisible to every later boot until the engine fails to load, because the downloader writes straight to the final path, persists the ETag from response headers before the body completes, and treats file presence as validity. Recovery needs deleting the artifact and its.etag.Worth an upstream issue; there is currently no content-integrity check at all since sha256 pinning was rejected on the Model CRD.
Still open
vllm#45238), likely worth more than any decode tuning on this workload#31648(mamba LRU, prefix hit 0.61 -> 0.83) needs a fork rebuildValidation
kustomize build kubernetes/apps/ai/llmkube/modelsclean. SGLang verified serving at 0.875 with a live request; pool 183,240 tokens plus 9.01 GB host KV and 7.14 GB host mamba, 3.71 GB GPU headroom left.Summary by CodeRabbit