Repository navigation
Add an opt-in DSA indexer query split to the HiCache W4AFP8 image - #300
Merged
Merged
Conversation
GLM-5.3 Flash's DSA indexer is ReplicatedLinear, so every attention-TP rank computes fp8_mqa_logits and the K-pool top-k for all prefill query rows; at long context that is the only prefill cost that grows with context. With SGLANG_DSA_INDEXER_QSPLIT=1 each rank scores a quarter of the rows against the same gathered keys and the int32 top-k rows are all-gathered. It also shrinks the per-rank logits scratch, which is what lets 16384-token chunks fit on the long tier. Inert unless enabled. Evidence (gpu31/gpu32, 2026-09-24, every A/B in both island orders): stored long-tier traffic paired TTFT 0.81-0.85x with 100% completion; the chunk-16384 burst keeps 9.5 GB free vs 0.65 GB without; GSM8K, passkey retrieval and teacher-forced agreement are at the noise floor. The image built from this recipe passes test-cpu.sh (new step 4) and reproduces the cold-prefill speedup on GPU (+16.4% at 512K, chunk 16384). The publish workflow pins move to the new recipe hashes and the default tag to glm53-hicache-w4afp8-v2, so the v1 image #294 pins is untouched. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
✅ OpenCodeReview: Review failed: 0 finding(s); 7 of 7 selected item(s) failed. |
lloydmak99
approved these changes
Sep 24, 2026
lloydmak99
left a comment
lloydmak99
left a comment
Contributor
There was a problem hiding this comment.
Additive, opt-in change: adds a v2 HiCache W4AFP8 image with a DSA indexer query-split patch gated behind SGLANG_DSA_INDEXER_QSPLIT (default off), nothing in this PR enables it and no prod compose file is touched. The hash chain is fully self-consistent and the qsplit patch's out_rows=per padding checks out against upstream, so this is safe to merge.
Optional follow-ups (non-blocking):
.github/workflows/publish-glm53-admission-reserve.yaml:18— thetaginput help text still listsglm53-hicache-w4afp8-v1, but the case statement now defaults to-v2. Cosmetic UI string only; no effect on the computed default.docker/sglang-glm53-hicache-w4afp8/dsa-indexer-qsplit.diff(module-level_QSPLIT_MIN_ROWS = int(os.environ.get(...))) — a non-integerSGLANG_DSA_INDEXER_QSPLIT_MIN_ROWSwouldValueErrorat import; worth a guard if this env var is ever surfaced to operators. Default"1024"is safe and unset here.
Checks: sha256sum -c SHA256SUMS 8/8 OK and recomputed Dockerfile/PROVENANCE/SHA256SUMS match the workflow's EXPECTED_*; git apply --numstat on the patch clean with consistent hunks; repo validators (validate_glm53_prod_config.rb, validate_glm53_dcgm_metrics.rb, validate_public_metrics.py, validate_streaming_keepalive.py, prepare_glm53_w4afp8_base.py --check + tests) all OK; CI green on head; patch semantics cross-checked against upstream sglang. Not run: in-container image build / test-cpu.sh (needs the private base image and a TP>1 GPU host) — covered by build-time hash enforcement and the author's gpu31 4/4 build at head.
pranavraja99
added a commit
that referenced
this pull request
Sep 25, 2026
…302) * Canary the DSA indexer split and chunk 16384 on the long-context r2 Pins r2 to the v2 engine image published from #300 (8ff1a487b98a), sets SGLANG_DSA_INDEXER_QSPLIT=1 and raises its prefill chunk to 16384. r1 is left completely untouched -- same #294 image, chunk 8192, pdi 1, split unset -- so it stays a live control and a targeted `compose up` recreates r2 alone. Structurally r2 now needs its own image AND environment: a YAML merge key replaces a list rather than extending it, so r2 cannot inherit the anchor's environment and add one variable. The generator derives r2's block from the anchor at generation time rather than duplicating it as a literal, so the two cannot drift. The chunk and the split are one change. At 16384 without the split a concurrent long burst left 0.04-0.65 GB free per GPU, the condition that preceded the gpu02 crash; with it, 9.3-9.5 GB. The validator now rejects --chunked-prefill-size 16384 on any replica that does not also set SGLANG_DSA_INDEXER_QSPLIT=1, and rejects the variable on any image that does not carry the patch. Both gates were verified by mutation, including the case where the expected spec itself is edited -- the first placement of the chunk gate sat in validate_w4afp8_base, where it could never fire. Lab evidence (gpu31, candidate vs #294): TTFT 0.61-0.78x, TPOT 0.53-0.85x, minimum free device memory 3.7 -> 9.3 GB, quality at the noise floor. The all-gather's cost under CC/PPCIe inside a CVM is still unmeasured; gpu31 is bare metal with CC off, so this canary is the first measurement of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make the generated header describe the canary, and enforce marker counts Review found the header still claimed "8192-token prefill chunks", "Both pin fde25985...", "flags differ only in --dist-init-addr and r2's canary pdi 2" and "(16384 fell to 0.04 GiB and is rejected)" -- all false once r2 moved to the v2 image at chunk 16384. That header is the rollback and incident reference for a file operators are told not to hand-edit, so a stale one is worse than none: it would have pointed an on-call at the wrong image during a rollback. It now states plainly that the replicas are not identical and lists each one's image, chunk, interval and provenance, plus why the chunk and the split are one change. Also enforce the occurrence count in resolve_engine_image_labels instead of unpacking it and ignoring it. find() silently takes the first hit, so if an "instance:N" marker ever stopped being replica-unique the nearest-placeholder binding could attach the wrong replica's image label. It now fails loudly, matching the exact-count discipline replace_exact uses elsewhere in the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
pranavraja99
added a commit
that referenced
this pull request
Sep 25, 2026
…unk 8192 (#307) * Isolation arm: keep the DSA indexer split, drop r2 back to chunk 8192 The 2026-09-25 gpu02 canary ran split + c16384 + pdi 2 and inverted the lab result at the tail. Over a 4 h steady-state window (cold start excluded): TTFT p95 r1 47.3 s r2 76.9 s 1.62x WORSE TTFT p50 r1 3.654 s r2 3.768 s 1.03 ITL p50 r1 18.08ms r2 17.53ms 0.97 (decode win reproduced) ITL p90 r1 78.6 ms r2 68.8 ms 0.87 (decode win reproduced) The lab predicted TTFT 0.61-0.78x. Not explained by traffic volume (681 vs 688 req/h), cache warmth (79.7% vs 81.6% hit) or queueing (r2's average queue depth was LOWER, 1.23 vs 1.56). r2 did draw ~23% more uncached prefill tokens per request, which it absorbed for +3% median TTFT but +62% at p95: worse only at the tail, i.e. only on the largest prefills. The chunk and the split both bite exactly there, and the canary changed both at once. This arm holds the split and returns the chunk to 8192 so they can be told apart: if p95 recovers the chunk was at fault, if it stays bad the all-gather under CC/PPCIe is -- the risk #300 flagged as untested, and which gpu31 cannot test with CC off. The validator gate is one-directional by design (c16384 REQUIRES the split, not the reverse), so this combination is permitted without weakening the check that guards the 0.04-0.65 GB free crash condition. Tests: the two c8192/c16384 asymmetry cases are vacuous now that both replicas run 8192, replaced with drift cases off 8192. The pairing-gate test now CONSTRUCTS the forbidden combination (raise chunk, drop split) and asserts the specific pairing error, instead of passing because an unrelated equality check fired first. 13 tests; the 2 failures under local Ruby 2.6 are the pre-existing filter_map gap, identical on origin/main. gpu02 was rolled back to v0.0.443 before this was written; r1 was never touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Put the DSA indexer split on both long-tier replicas, revert r2 to chunk 8192 Two changes, one cause. r1 crashed on 2026-09-25 at 18:07:58. deep_gemm.fp8_mqa_logits (dsa_indexer_kpool.py:919) asked for one fp32 buffer of new_tokens x total_context -- 11.96 GiB at ~392K context with an 8192 chunk -- against 11.88 GiB free. That is an ordinary request for this tier, not a pathological one. r1 lacked the indexer split, so all four ranks built the full buffer. So both replicas move to the v2 image with SGLANG_DSA_INDEXER_QSPLIT=1, set on the anchor rather than per-replica so there is exactly one definition. Each rank then scores 1/TP of the rows and that allocation falls to roughly 3 GiB. And r2 returns to chunk 8192. The 2026-09-25 canary ran the split at 16384 and inverted the lab result at the tail: TTFT p95 76.9 s against r1's 47.3 s (1.62x) over a 4 h steady-state window, though the decode win did reproduce (ITL p90 0.87x). Traffic volume (681 vs 688 req/h), cache warmth (79.7% vs 81.6%) and queue depth (r2's was lower) did not explain it. gpu02 was already rolled back to v0.0.443; this brings the repo back in line and keeps the crash headroom. THE SPLIT IS A MITIGATION, NOT THE FIX. It divides the buffer by TP; it does not bound it, so the same crash returns at ~4x the context or at TP1. The fix is calling _should_chunk_mqa_logits -- defined but never called, at dsa_indexer_kpool.py:862 -- and chunking the logits against free memory, which dsa_indexer.py already does for the non-kpool path at :1165. That is a separate change and not a copy-paste: the kpool path uses a different layout (pooled seq lens, ragged q ks/ke, topk offsets) and needs correctness tests against the unchunked path. Also worth recording: r1 was not healthy when it died. 1.29M pending tokens, a full queue, and a 20 MB mapping had just failed with 1.5 MB free. Chunking bounds the spike but load contributed, so neither change should be read as making that exact scenario survivable. This leaves no unsplit control on the tier, so the split's cost under CC/PPCIe is now measured against pre-change history rather than a live sibling. Deploy r2 first and watch p95 before moving r1. Tests: the image-patch case previously swapped R2_IMAGE for IMAGE, which is a no-op now that both are v2 and would have passed without exercising anything; it names the v1 digest explicitly instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Sep 25, 2026
Brings in #299, #300, #302 and #307. Conflicts in the W4AFP8 long-context generator, validator and test are resolved by taking main's per-replica logic (both replicas on the v2 QSPLIT image at chunk 8192, r1 pdi 1 / r2 pdi 2, r2's own environment list derived from the anchor) and re-applying this branch's tracing additions on top: --enable-trace and the OTLP endpoint on both replicas, SGLANG_TRACE_ASYNC=1 / SGLANG_TRACE_LEVEL=0 in the anchor environment (and therefore in r2's derived list), the -trace-async-armed variant suffix on both variants, and the glm53-trace-control job. The compose file is regenerated, not hand-merged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Adds a third source patch,
dsa-indexer-qsplit.diff, todocker/sglang-glm53-hicache-w4afp8and bumps the publish default tag toglm53-hicache-w4afp8-v2. No prod file changes.What it fixes: the DSA K-pool indexer is a
ReplicatedLinear, so every TP rank computesfp8_mqa_logitsand the top-k for all rows of a prefill chunk.How: with
SGLANG_DSA_INDEXER_QSPLIT=1, each rank scores 1/TP of the rows, then the int32 top-k indices are all-gathered.SGLANG_DSA_INDEXER_QSPLIT_MIN_ROWSrows (default 1024).Evidence (gpu31, 2026-09-23/24)
Cold prefill (chunk 16384): 1.04× / 1.08× / 1.12× / 1.16× faster at 128K / 256K / 384K / 512K.
Stored traffic (three rounds, both island orders): TTFT 0.81–0.85×.
Memory: the indexer score buffer shrinks by TP×, which makes chunk 16384 safe. Minimum free was 9.3–9.5 GB, vs 0.04 GB without the split (the setting that preceded the gpu02 crash).
Ship qualification of the full candidate (this image with the split on + pdi 2 + chunk 16384) vs GLM-5.3 Flash: W4AFP8 + HiCache long-context file for gpu02 (both replicas = campaign-2 L2) #294, same GPUs, stored traffic:
0 restarts, 0 OOMs, 0 fatal log lines.
Quality:
Write-up: Notion
Verification
BUILD_SOURCE_REVISION=99bed39): "Verified and applied 3 patches",sha256sum --checkOK.EXPECTED_*hashes match the recipe's Dockerfile, PROVENANCE and SHA256SUMS.test-cpu.sh: 4/4 steps pass on that image. The new step 4 covers opt-in, the min-rows reload, and the method being present.nearai.sglang.dsa_indexer_qsplit=v1-opt-in.After merge
variant=hicache-w4afp8,source_revision=<merge commit>).SGLANG_DSA_INDEXER_QSPLIT=1and chunk 16384 on the r2 canary from Canary --prefill-decode-interval 2 on the W4AFP8 long-context r2 #299.SGLANG_DSA_INDEXER_QSPLIT=1is also set.Untested: the all-gather's cost under CC/PPCIe in a CVM. The r2 canary is where that shows.
🤖 Generated with Claude Code