Fix accuracy test - #1051
Merged
Merged
Fix accuracy test#1051
Conversation
merrymercy
marked this pull request as ready for review
August 12, 2024 09:25
zhyncs
enabled auto-merge (squash)
August 12, 2024 09:28
zhyncs
approved these changes
Aug 12, 2024
zhyncs
disabled auto-merge
August 12, 2024 09:48
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 31, 2026
…ee and then freed it ROOT OF sgl-project#924, and the six boots that could not find it were looking in the right file for the wrong party. Boot 23 (boot_855_1033c_0840f82601_0831_134138.log, 13:45:39Z, all three ranks, slot 1) is the specimen; the #1033b provenance instrument named `alloc_group_end` FIRST RELEASER and the radix eviction SECOND, and both were innocent. They were merely the first two parties to touch a slot that had had two owners since it was cached. THE DEFECT, at one line. On the standing boot form (`mamba_radix_cache_strategy=no_buffer`, `enable_int8_mamba_checkpoint=False` -- both read off the boot's own ServerArgs) `MambaComponent.prepare_for_caching_req` donates the FINISHED request's own active slot to the tree: `active_value = req.mamba_pool_idx.unsqueeze(-1) .clone()` (:1109) becomes `insert_params.mamba_value` (:1118) and then the node's `component_data[MAMBA].value` (:612/:622). `cleanup_after_caching_req` then called `pool.free_mamba_cache(req)` UNCONDITIONALLY -- so the node held slot X and the allocator was told X was free, in the same call. From there: `alloc_group_begin` legitimately re-draws X, nothing consumes it, `alloc_group_end` returns the remainder, and the node's eviction is the first release that finds the ledger already False. IT IS A PORT OMISSION, NOT A MISSING FEATURE. The sibling implementation this component replaced states the rule verbatim (mamba_radix_cache.py:781-792): free_mamba_cache = True if (extra_buffer or int8) else mamba_exist -- on the plain path the active slot goes back ONLY when the tree refused the donation. The term was lost in the move to the unified component. sgl-project#929 fixed the mirror case (the donation the tree did NOT take, which leaked) and its own comment already spelled out that on this branch "the donation IS the request's own slot"; it drew the conclusion for one half only. UPSTREAM-MINIMAL, asked first and answered by measurement, not by assertion. The alloc_group window is NOT fork-own second bookkeeping: `alloc_group_begin` /`alloc_group_end` came in with upstream PR sgl-project#25000 (Leon Gao, 2026-06-04, "Reduce mamba prefill allocation overhead"), were refactored by upstream sgl-project#27256, and are present on `main`. So the deletion default does not apply to it, and -- more to the point -- this commit does NOT add a guard to that window. It REMOVES a fork-introduced deviation from the sibling's ownership rule. Fewer fork terms, not more. `HybridReqToTokenPool.relinquish_mamba_cache` is the release-free counterpart of `free_mamba_cache`: same postconditions (`mamba_pool_idx` None, sgl-project#991 acquire stamp cleared), no slot returned, and it refuses the extra-buffer configuration by assertion because there the donation is a ping-pong track slot and the active slot must still go back. ROOT BEFORE EFFECT * CLASS: ownership transfer performed on one side only -- the receiver takes the handle while the giver still executes its own release. Same class as sgl-project#929 (the reverse omission) and as the boot-2i regression its fix caused. * SIBLING SWEEP: the other three release verbs on this path are correct -- the int8 branch (:1201) and the extra-buffer branch (:1212) donate a SEPARATE slot, so returning the active one is right; `cache_unfinished_req` allocates a fresh donation and copies (:1167). `mamba_radix_cache` already carries the guard. `UnifiedMambaSlotAllocator` inherits the new method. * FUTURE CHECK: `devtools/check_1051_mamba_ownership_transfer.py` -- hermetic, CPU-only, drives the REAL `cleanup_after_caching_req`. `--legacy` replays the pre-fix decision through the same asserts and FAILS case 1 on the boot-23 specimen, so the check is proven able to fail. #1051b, SIDE FINDING IN THE SAME WINDOW, own class (a LEAK, not a double free), fixed here because it is three lines and the same invariant: `alloc_group_begin` dropped a still-open group's iterator, discarding un-consumed slots without returning them (`slot_used=True`, on no free list -- the `leaked_mamba_pages` shape the on-idle ledger reports minutes later with nothing left to attribute). It now closes the open group first. And `MambaSlotAllocator.clear` did not reset `_alloc_iter`, so a flush could leave an iterator minting ids from the OLD free-list generation into the fresh one -- a duplicate on a free list that is a bare `torch.cat` and cannot represent uniqueness. `UnifiedMambaSlotAllocator.clear` already did this; both `alloc_group_begin` implementations now match. EVIDENCE TIER: DESK-PROVEN. Matched check green on the fixed arm, red on the legacy arm at exactly case 1. Metal proof is boot 24.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 31, 2026
…mamba slots blind Twenty lines above the loop this fixes, the same function reads the allocator's enumerated free set and frees only rows that are NOT already on it, under a comment that states the rule in full: "NEVER FREES BLIND. A row that is already on the free list must not be freed twice: that is silent corruption, the one outcome worse than the leak." Every word applies to a mamba slot. None of it was implemented on the mamba side. MEASURED, boot 25 (boot_855_1054diag_0840f82601_0831_143640.log): nine MambaSlotDoubleFree events, all three ranks, and the #1033b provenance put BOTH releasers inside this one function -- the eviction pass at :1924 frees a node's `cd.value` and nulls it, then this loop walks the tree again and returns the same slot a second time. Neither releaser was at fault. The asymmetry was. THE AUTHORITY IS THE ALLOCATOR'S OWN LEDGER. `slot_used` is the sgl-project#924 bool ledger and answers "is this slot already free?" in O(1) -- the same question `read_free_rows` answers for the KV half, with a cheaper instrument and the same refusal to guess. AND IT ASKS THE ALLOCATOR THE FREE ACTUALLY REACHES (sgl-project#941 class, the lesson the KV half spells out one screen up): `_free_mamba_value` routes to `int8_ckpt_pool` when one exists, so differencing against the SLOT allocator's ledger would check membership in one pool while freeing into another -- the guard passing while performing the exact double free it exists to prevent. With a checkpoint pool in play there is no enumerable ledger here, so this REFUSES and says so, rather than freeing blind. Same for an allocator that exposes no ledger at all. NO SILENT `continue` ON THIS PATH ANY MORE. The version replaced here swallowed every per-slot failure without a word. That is the C1 swallow instance in miniature and it is what let boot 25's nine raises die unheard while the ledger corrupted underneath: the boot survived, which is why nobody saw it. Failures are now counted (`mamba_failed`), the first one's type AND message land in `reason`, and a loud ERROR names them. The flip still proceeds -- a reclaim may not abort a seam -- but it can no longer proceed QUIETLY. Skipped already-free slots are counted separately (`mamba_already_free`) and warned, because a non-zero there is not noise: it means two references to one slot survived into the same drop. ROOT BEFORE EFFECT * CLASS: one half of a paired guard implemented, the other left blind -- the same shape as sgl-project#1051 (ownership transfer honoured on one side only) and as sgl-project#929 before it. Third instance in this family on this path. * SIBLING SWEEP: the KV half already differences and already refuses on a non-enumerable free set; the host/mamba host pool is freed through `_mamba_pool_host` under the component's own eviction, not here. * FUTURE CHECK: `devtools/check_1055_mamba_reclaim_differencing.py`, hermetic, no CUDA. It calls the REAL `reclaim_rows_for_drop` UNBOUND against a stand-in `self`, so the shipped body is what runs: an already-free slot is not returned twice, owned slots still are, the skip is counted, the int8 and no-ledger paths refuse with a stated reason, and the swallow is gone. 7/7. It caught one defect in ITSELF while being written -- a source-slice window too short to reach the code it asserted, i.e. a truncated read scored as absence, the same family as the `| head` trap; the window is now bounded by the function's own return. EVIDENCE TIER: DESK-PROVEN. The metal proof is boot 26's MambaSlotDoubleFree column, which is unpassable without this.
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.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 9, 2026
…ck for all three exits ROOT, measured on boot weg2sn5s @ fd92440 under a FLEET-SHAPE load (shared ~16k anchored prefix + growing tails), all three D ranks: 22:27:51 #924D station=alloc_cow rid=664901103e55 mamba_slot=[7] site=finalize_match_result 22:27:51 WEG2 X-GATE rid=664901103e55 uncached=17236 X=8742 verdict=W31 22:27:51 W50 Weg2TpPrefillExceeded rid=664901103e55 uncached=17236 X=8742 on_idle [mamba] total=30, available=4, evictable=25, leaked_mamba_pages={7} mamba_leak_owners=[slot=7 slot_used=True last_event=ALLOC@seq6158 releaser=none-recorded] A prefix match drew a COW resume slot SPECULATIVELY (`MambaComponent.finalize_match_result`, stamping `mamba_slot_acquired_this_admission`). The X gate then refused the request by name and `_weg2_answer_x_refusals` returned it to the front WITHOUT the slot. That request has exactly ONE station line for its whole life: it never reaches `alloc` or `cache_finished_req`, so the refusal exit is the ONLY owner of that give-back. Zero `sgl-project#991` lines in the entire log confirms none ran. WHY IT READ AS INTERMITTENT, and why four clean 41k prompts proved nothing: BOTH conditions must hold -- `uncached > X` (long prompts only) AND a prefix match carrying mamba state (so the COW acquire happens at all). Every earlier repro put a unique nonce at the FRONT of the prompt to defeat prefix caching, which satisfies the first condition and structurally excludes the second. The fleet's real shape -- one shared system+tools prefix, growing tails -- leaked within two passes. THE CLASS: THREE admission-refusal exits, the give-back open-coded in two and absent from the third. * `scheduler.py` admission revert (sgl-project#991) -- FORK-OWN * `schedule_policy.py` PPScheduleRefused (sgl-project#791) -- FORK-OWN * `scheduler.py` `_weg2_answer_x_refusals` (sgl-project#1290/law 4, W31/W50) -- FORK-OWN All three are fork-own Weg-2 admission plumbing; the COW acquire they must compensate is upstream (`e0b692600f`, PR sgl-project#27118). That is the upstream-minimal statistic again: the defect sits in fork-own compensation around an upstream mechanism, not in the mechanism. FIX: lift the two open-coded bodies into ONE `release_admission_acquired_mamba_slot` (`mem_cache/common.py`) and call it from all three exits with identical guards (`mamba_slot_acquired_this_admission` and not `req.session`) -- a third copy would have been the second-bookkeeping answer. It also clears the match's other per-admission carry-overs (`mamba_cow_src_index`, `mamba_needs_clear`, `mamba_loadback_anchor_adopted`) so a refused request cannot carry a resume anchor into its next admission, and emits `#924D station=give_back site=<exit>` so the exit is visible in the trail. The wider partition on that exit needs nothing further: the request never allocated KV rows or a req_pool row (no `station=alloc`), the match's `inc_lock_ref` pairs are closed in-block at `mamba_component.py:488/:499`, and the exit already calls `release_aborted_request` / `terminate_prefetch`. Tests: `test_mamba_admission_giveback_q0.py`, 9 cases asserting the FULL partition after every station -- `expected == free U tree U live` AND all three pairwise intersections empty, i.e. the orphan direction AND the aliasing direction that the earlier suite could not see. RED-first against fd92440: 5/5 red including the specimen simulated on the real allocator (slot orphaned after the W50 exit); green on the fix. 3 mutants killed with an unmutated control: acquired-guard removed (frees batch-owned slots, sgl-project#1051's direction), session-guard removed, give-back made a no-op (the pre-fix exit). ruff: 127 findings across the three touched files, identical on the baseline tree.
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
2 participants
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.
No description provided.