docs: update README - #935
Merged
Merged
Conversation
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 27, 2026
…spec in sgl-project#936 NOT A FIX. This adds one function and calls it from nowhere. It is committed alone so it cannot be lost in a worktree, and so the successor building sgl-project#936 starts from a written rule rather than from a description of one. WHAT IT IS. `lower_protected_to(req, new_protected_len)` lowers `req.cache_protected_len` by `min`, never assigns and never raises it. `cache_protected_len` means "the tree owns the KV below this" -- the premise both free-sites in `cache_finished_req` rest on and the one `retention_shrinks_protected` states in this same module. Raising it would promise ownership nobody holds; a flat assignment would clobber a HIGHER claim a later legitimate match had earned. So: min. WHY IT EXISTS. When the tree does NOT take a request's prefix, the promise is false for the rows that request still holds, and the two branches that free FROM `cache_protected_len` while inserting nothing then skip over them: * `unified_radix_cache.cache_unfinished_req`, the `effective_cache_len <= 0` skip branch -- frees `kv_indices[req.cache_protected_len:]` (:1196) and returns (:1212) without inserting and without lowering cpl; * `unified_radix_cache.cache_finished_req`, the `is_insert=False` branch (:1130-1131) -- identical shape. The `effective_cache_len <= 0` that opens the first one comes from mamba's `_decline_retention(is_finished=False)`, which returns 0. That refusal is measured, not inferred -- boot 2g, in its own words: "mamba retention would truncate a protected prefix: tracked position 4096 under cache_protected_len 8192, caching without a mamba anchor". THE RULE sgl-project#936 SHOULD APPLY: if the tree does not take the prefix, there is nothing left to protect -- cpl falls in the SAME moment to what the tree actually holds (0 when nothing was inserted), and the free then covers everything. At the moment of the refusal, never blanket on the request: a later successful match may legitimately grant a protected prefix again. EXPECTED TO BE REPLACED. sgl-project#930's `truncate_prefix_to` carries the same min-never-assign discipline; when it lands, sgl-project#936 should use it and delete this one. Two helpers for one rule is the shape that lets them drift. sgl-project#935's guard (fa7dd3b) is the falsifier for that work and stays as it is: it MEASURES the orphaned interval rather than repairing it, so if it still fires after sgl-project#936 there is a second source and it names the share. Desk checks only, as befits an unwired helper: py_compile clean, ruff clean. No test, because a function with no caller has no behaviour to pin -- the red-first test belongs with the wiring, and its shape is specified in sgl-project#936 (mamba retention refused + a full cache_protected_len, asserting the interval is neither inserted nor freed today).
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 27, 2026
…ted_len with it `cache_protected_len` is HOW MANY LEADING ROWS OF THIS REQUEST'S KV THE TREE OWNS. `init_next_round_input` sets it equal to `len(prefix_indices)`. The sgl-project#791 admission-uniformity block in `_get_new_batch_prefill_raw` then truncates `prefix_indices` to the PP-agreed `told` -- on PP0 from the guard's clamped candidate, downstream from PP0's decision -- and NEITHER branch touched the protected length. The request was left claiming more tree-owned rows than it holds. I FILED THIS AS "THE SAFE DIRECTION" AND THAT WAS THE WRONG HALF. The surplus IS harmless for `_insert_helper`'s duplicate free -- a larger `dup_start` frees less -- which is exactly why it never surfaced as a double claim. It is the DANGEROUS direction for `cache_finished_req`'s truncate branch (`unified_radix_cache.py:1111-1116`): free_start = max(effective_cache_len, req.cache_protected_len) free(kv_indices[free_start:]) # starts ABOVE the interval ... # the insert covers only up to ecl With `cache_protected_len > effective_cache_len` the rows in `[effective_cache_len, cache_protected_len)` are neither freed nor inserted and belong to nobody afterwards. That interval is sgl-project#935's per-request row leak (36824 rows on the 2i acceptance boot). SCOPE, kept sharp so the two tickets do not blur. The GAP is the root and is sgl-project#935's: it must not be able to leak whatever the value is. This closes one of the two PRODUCERS that make it reachable; the other is the sgl-project#928 refusal re-prefill. Closing a producer does not close the gap, and closing the gap makes the producers harmless -- both are owed, and neither substitutes for the other. Different files, no collision with fix/935-finished-req-gap. ONE HELPER, TWO SITES, because the two sites are siblings of each other and drifted identically -- both sliced `prefix_indices` by hand and both forgot the same field. `Req.truncate_prefix_to(told)` now owns the pair, and the wiring pin asserts neither branch slices by hand again. MIN, NEVER ASSIGN: the helper may only LOWER the claim. A request whose protected length was already below `told` owns exactly that many, and raising it here would invent protection the tree never granted -- which is the dangerous direction for the duplicate free, i.e. the defect this commit is NOT allowed to trade for. Pinned in both directions. Tests, hermetic, CUDA_VISIBLE_DEVICES="": test_truncation_keeps_protected_len_930.py, 6 passed. Combined mem_cache+managers gate: 17 failed / 7483 passed, the 17 NAME-IDENTICAL to f1a3391's -- zero new failures. Genuinely red-first with the mutant the ticket names -- restoring "slice the prefix, never touch cpl" turns 3 red, including the consequence test that computes the abandoned interval from the real `max(ecl, cpl)` arithmetic, while the may-only-lower direction stays green. Restored after.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 9, 2026
…refill
ROOT of the on_idle wall that killed the serving base twice (boot weg2sn5n
2026-09-09T21:19:33Z, all three D ranks, `leaked_mamba_pages={26}`; boot ARM 3
before it, `={16}` after ONE 24k direct prefill).
NOT the sgl-project#935/sgl-project#936 census false positive, and that was checked first: `[full]`
balanced exactly (707585+50943==758528), TREE CENSUS was self-consistent on
every rank (`MAMBA: tracked_evictable=18 recomputed_evictable=18`), and
`_all_component_values_flatten` DFSs every node from the root regardless of
tier. World (b) of the invariant checker's own docstring: the row was orphaned
outside the tree.
THE DEFECT. This boot runs `--mamba-radix-cache-strategy extra_buffer` with
overlap, so a request holds THREE slots (active + two ping-pong). On the
finish path the unified component:
* allocates a FRESH `new_slot` and calls `donate_mamba_ping_pong_slot`;
* that method returns the OLD slot at `donate_idx` -- what the tree takes --
and REPLACES the buffer entry in place, `buf[donate_idx] = new_slot`;
* `cleanup_after_caching_req` then recomputed the SAME index as `keep_idx`
and passed it to `free_mamba_cache`, which freed only `buf[1-keep_idx]`.
`buf[keep_idx]` is `new_slot`, owned by the REQUEST; the tree owns the old
slot. Keeping it handed it to nobody. Exactly one orphaned slot per finished
request whose donation the tree accepted -- which is why only NEW long
prefixes trip it (a request landing on an existing prefix takes
`mamba_exist=True` and never donated), and why a short-prompt fleet ran for
hours before the first Claude-Code-shaped 40-55k prompts killed it in minutes.
THE KEEP SEMANTICS WERE INHERITED AND INVALIDATED BY THE PORT, not lost as in
sgl-project#1051. The sibling this component replaced (`mamba_radix_cache.py:746-761`)
takes the tree's value from the buffer IN PLACE and allocates nothing:
src_active = req.mamba_ping_pong_track_buffer[keep_idx].unsqueeze(-1)
mamba_value = src_active.clone()
There "keep buf[keep_idx]" is exactly right because the tree really owns that
entry. The unified port added the fresh allocation and the in-place
replacement while carrying keep_idx across unchanged.
SECOND LEAK, same branch, opposite direction: when the tree REFUSES the
donation the old slot has already left the buffer, so `free_mamba_cache`
cannot reach it and nothing else owed it back. The int8 branch directly above
handles its twin (`insert_value_unused`) and the plain branch below does too
(sgl-project#929); only this one handled neither. Both halves are fixed here, and the
rule now matches its two siblings instead of adding a second ledger.
INSTRUMENTATION (approved with this posten, no behaviour change):
* `#924D station=alloc_component` / `station=alloc_cow` close the coverage
hole that made the specimen unrootable: `station=alloc` was emitted ONLY
from scheduler.py, so every draw inside this component was invisible and
slot 26 appeared in no station line at all.
* the leak report now names the OWNER, not just the slot: per leaked slot it
prints `slot_used` (the allocator's own ledger), the #1033b provenance entry
(last event + releaser stack) and whether an alloc group is open. Same
complaint #1033b makes about `_refuse_double_free` -- the instrument named
the visible party, not the responsible one.
Tests: test/registered/unit/weg2/test_mamba_pingpong_donation_ownership.py,
8 cases driving the REAL `donate_mamba_ping_pong_slot`, `free_mamba_cache` and
`cleanup_after_caching_req` against a REAL MambaSlotAllocator on CPU, and
asserting the invariant checker's own arithmetic (expected - free - tree)
after every station. RED-first against the unfixed tree: 5 failed / 3 passed
(the 3 are postcondition pins that hold both ways), reproducing the specimen
shape exactly -- one request, one orphan. 8/8 green on the fix. 3 mutants
killed with an unmutated control: pre-fix keep_idx restored (L1), refused-
donation give-back dropped (L2, and L1 still green under it, so the two halves
are independently covered), tree's slot freed as well (swings into sgl-project#1051's
double-owned direction and is caught by the allocator's double-free refusal).
ruff: invariant_checker.py's 2 findings are pre-existing and identical on the
unfixed tree; the other two files are clean.
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
1 participant
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.
Thank you for your contribution, we really appreciate it. The following instructions will help improve your pull request and make it easier to receive feedback. If there are any items you don't understand, don't worry. Just submit the pull request and ask the maintainers for help.
Motivation
Please explain the motivation behind this PR and the goal you aim to achieve with it.
Modification
Briefly describe the changes made in this PR.
Checklist
pre-commit run --all-filesor other linting tools are used to fix potential lint issues.