fix: use v0.2.5 for benchmark - #814
Merged
Merged
Conversation
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
litmei
pushed a commit
to litmei/sglang_litmei
that referenced
this pull request
Jun 16, 2026
Add core testcases
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 22, 2026
… cutover
`recover_kv_backing` is the only thing that lifts `KvRowCap`, and its only two
call sites were post-cutover hooks (phase_flip_runtime.py:1818 and :1835). That
is fine while flips commit, and a trap the moment one does not -- a trap that
closes on itself:
1. a corridor-bounded recovery leaves the ranks unequal (measured on this
rig, one boot: 210944 / 124928 / 131072 backed rows);
2. the cap agreement levels the group to the poorest -- correct, since under
pure PP an id a peer cannot map aborts all three ranks inside
store_kvcache's bounds assert (phase_flip_boot.py:1264-1278) -- so the
allocator is capped at 124928 of 465190, 26.8% of the id space;
3. the capped pool fills, so the group's live floor is 100% of what is left;
4. the next tp_to_pp flip is DECLINED by that peer floor (sgl-project#812);
5. no cutover, so no recovery, so the cap is never lifted -- back to 3.
Evidence that step 5 is what actually happened, from the log rather than from
the code: that boot has 27 POOL CENSUS lines = 3 ranks x 9 events, and NOT ONE
is `post-cutover tp_to_pp`. All six return attempts stop at `at-arm`.
`recovered to N of M rows` appears 0 times, `recovery deferred` 0 times,
`recovery failed` 0 times. The pool sat at 26.8% for the life of the process
and a user got an overloaded_error at 21:02:58 against a pool sized 3.7x
larger. So sgl-project#812 and sgl-project#813 are consequences of the cap, not its cause.
Both docstrings already named this outcome. `recover`: "an admission-capacity
loss, which is recoverable ON ANY LATER LEG" -- there was no later leg.
`recover_kv_backing`: "a cap that is never lifted turns dynamic residency into
a permanently smaller pool, which is the one fix the standing rule forbids."
THE FIX IS A LEG, NOT A NEW POLICY. The cap agreement is left exactly as it is:
`cap_proposal` is STRICTLY NON-ALLOCATING on purpose and records why -- the
first metal boot of a growing agreement hit "cuMemCreate failed:
CUDA_ERROR_OUT_OF_MEMORY" on all three ranks, rank 0 driven to 3 MiB free
(kv_backing_relief.py:2710-2745, 2026-08-13). That door stays shut. The design
already assigns growing to exactly one owner, `recover`; it simply had nowhere
to run. This adds the missing call at the abandon exit, where nothing moved and
no seam is owed the memory.
Safety, and none of it is new: `recover`'s bound is intrinsic to the function
(kv_backing_relief.py:2503-2505) -- a fresh per-rank mem_get_info at call time
minus this card's own corridor law, deferring without committing a byte when
`rows <= was`. A new call site inherits exactly what the two post-cutover sites
rely on. The grow stays rank-local and corridor-bounded; the levelling stays
collective and non-allocating.
Collective safety: the helper enters a collective, so it may only be called
where every rank arrives. Its one call site is the group-unanimous abandon exit
in `_execute`, downstream of `reduced_fit = self._collective_min(payload)`
(phase_flip_runtime.py:7381) -- a MIN reduction bit-identical on every rank,
with no `return` and no `raise` anywhere between it and the exit (checked by
reading every statement in that window). Without a channel the helper is a
no-op, matching `recover_kv_backing` on single-rank shapes. It never raises: an
abandon is already the unhappy path and must not become a dead rank.
Tests: test/registered/unit/managers/test_recover_on_abandon_814.py, new,
5 cases, hermetic (CUDA_VISIBLE_DEVICES=""). Three mutants, each killed by its
own dedicated case:
* single-rank guard removed -> test_without_a_channel_it_is_a_no_op
* reduce_fn dropped on the delegate -> test_it_delegates_through_the_group_channel
* logs on every abandon instead of only a real one ->
test_a_zero_row_recovery_logs_nothing (abandons come in floods; 47 in a
row were measured)
Regression: 730 passed across the flip/kv-backing/seam/census/evict suites.
3 failures in test_evict_rung_floor_invariant_717.py
("'KvBackingRelief' object has no attribute '_buffers'") are PRE-EXISTING --
measured, not claimed: the identical three fail with the identical error on the
base commit 587e4c2. ruff clean, black clean.
Metal, D1 half of this branch (boot_814_arm2_0822_2146.log): the census now
reads `withheld=330770 unaccounted=122` where it read `unaccounted=340384`,
and 122 is the known irreducible mamba artefact. This commit is the half that
returns the capacity rather than merely describing it; its metal acceptance
follows on the next boot.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 23, 2026
…t#816, sgl-project#810, sgl-project#806, sgl-project#797, sgl-project#790/sgl-project#777, sgl-project#817/sgl-project#820, sgl-project#818 Merge-checklist duty for the nine stages on this branch. Entries follow each section's own house style, and the mechanism text is taken from the commits' measured evidence rather than restated from the ticket titles. §3 KV backing relief + the allocator cap -- UPDATED IN PLACE rather than given a second bullet, because sgl-project#814 and sgl-project#816 are follow-on defects OF the KvRowCap mechanism that bullet already describes: the census reading the withheld block as a leak (340262 of 465190 ids), the lift being reachable only from a cutover (one boot at 26.8% of its id space for the life of the process), and exposure exceeding the backing (417850 rows over 105413 committed, the device-side assert in masked_set_kv_buffer_kernel). §3 HiCache staging write-through ring (sgl-project#810), new bullet, plus its two companion refusals -- the unbounded-file-tier refusal and the boot preflight ledger entry, the latter being why 22.01 GB of MHATokenToKVPoolHost across three PP ranks previously reached the preflight as nothing. §7 BAR1 deadline + loud abort -- appended the sgl-project#818 peer-liveness half to the existing narrative: the gate could wait forever on a peer that no longer exists, and neither Bar1CollectiveStalled (reset by every resolved read) nor defer_stall_for_building_peer (900 s off a build marker) caught it. §12 Robustness canon -- three new families: contradictory-flag (sgl-project#806), read-back-after-construction (sgl-project#797), denylist-of-reasons (sgl-project#817, sgl-project#820). §18.3 hicache staging sizing (sgl-project#810) -- §18's own rule is that a merge adding a reusable module adds its entry in the SAME merge, and this module had none. Records the removal of fits_pinned_host_budget so it is not reintroduced. §18.6 mamba carry instrument (sgl-project#767, gated by sgl-project#790) and flip break-even N (sgl-project#777). No existing entry was contradicted. Checked before writing: none of sgl-project#814, sgl-project#810, sgl-project#806, sgl-project#772, sgl-project#797, sgl-project#790, sgl-project#777, sgl-project#817, sgl-project#820, sgl-project#818 had a catalog entry, and the one sgl-project#677 line (§19.2, RESTORE-NEVER-REBUILD) describes a different mechanism than the sgl-project#677 layout hold, so it is not stale and was left alone. Gates for the tree this documents (hermetic, CUDA_VISIBLE_DEVICES=""): battery test/registered/unit/{managers,planner,server_args,mem_cache} baseline integ @ 78d27da 44 failed c56d238 (through sgl-project#818) 30 failed, 8452 passed 0 new ids, 14 fixed 1c4eadb (through sgl-project#816) 30 failed, 8461 passed 0 new ids, same set test_barlink_abort_gate_liveness_818.py (outside the battery dirs) 10 passed ruff --select=F401,F821,UP037 and codespell: 0 new findings vs the same file set on 78d27da (16 ruff / 6 codespell exist identically on base) docs-only change; codespell clean on the catalog itself
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 26, 2026
…ated gap and a stale double-owned reading as a leak, killing all three schedulers on idle Five on_idle firings (two boots) showed available+evictable+withheld exceeding total by exactly 22, never a deficit -- the opposite sign from sgl-project#814/sgl-project#902/sgl-project#832/sgl-project#856, and never previously routed through the sgl-project#822 ownership authority. Decomposition: 21 rows from _check_full_pool reading TokenToKVPoolAllocator.available_size() (a raw sum of free_pages+release_pages, allocator/token.py:52-54) instead of the union read_free_rows() already used by the phase-flip census and the sgl-project#822 audit (kv_row_ownership.py:743-843); 1 row from a single row simultaneously claimed by the free list and the radix tree, the sgl-project#822 authority's own EXCLUSIVITY "claimed by multiple" finding, never subtracted before. Fix, both additive, neither a tolerance/epsilon: - _check_full_pool now reads available via read_free_rows() when the allocator can enumerate; composite/watermark allocators keep ps.full_available_size unchanged. - _check_pool_invariant gained a double_owned term, subtracted, sourced from allocator.double_owned_slots (the sgl-project#822 EXCLUSIVITY count). Two defects found in review of the first cut, both fixed here: 1. The double_owned filter in phase_flip_runtime.py::_census_ownership_audit selected violations by substring match on Violation.detail ("more than one owner" in v.detail) -- prose used as control flow, against Violation's own "detail is never load-bearing" contract, the same shape as the line_gate substring defect (sgl-project#908). Fixed by giving Violation a structural kind field, set to EXCLUSIVITY_DOUBLED or EXCLUSIVITY_UNOWNED at its two EXCLUSIVITY construction sites in kv_row_ownership.py, and filtering on kind instead of detail text. 2. double_owned_slots is published only at phase-flip census (seam) events and read by on_idle at unrelated times; a reading taken before authority.retire() drops every claim it was computed from is stale past that point. Fixed by clearing double_owned_slots to None (not 0) at the exact point _retire_row_id_space calls authority.retire(). Checked analytically whether staleness alone could mask a genuine deficit: it cannot, for any non-negative reading, because double_owned is subtracted and can only push total_accounted further below total. It can mask an unrelated, coincidentally-equal-sized surplus; that residual case is disclosed, not eliminated, and bounded by the cutover-clearing fix. Tests (test/registered/unit/mem_cache/test_pool_invariant_double_owned_912.py, 14 tests, 31 subtests): reconstructs all five measured specimens, red-first against unfixed code, green after the fix; separate mutants for the original subtraction, the kind-based filter, and the retire-time clearing, each shown to fail only its own guard when reverted; a parametrized check that no non-negative stale double_owned value can turn a manufactured 100-row deficit into a false pass. Gate, hermetic (CUDA_VISIBLE_DEVICES=""), attribution proven by rerunning identical failing node-id lists with these three files restored to HEAD (git show HEAD:<path>) and diffing failure sets -- both axes' failures are identical with and without this fix, i.e. pre-existing: - mem_cache/: 2 failed, 1951 passed, 1658 skipped, 392 subtests passed (139.10s). Failures: test_acceptance_emitters_758.py RefillTiming x2, RuntimeError: No CUDA GPUs are available. - managers/: 15 failed, 4631 passed, 18 skipped, 563 subtests passed (719.09s). Failures: test_arena_high_water_631.py x7 and test_restore_never_rebuild_677.py x4 (RuntimeError: No CUDA GPUs are available), test_phase_flip_rotation_wiring_809.py x4 (Exception: retry() exceed maximum number of retries) -- all reproduced identically against HEAD-restored files. ruff check and codespell clean on every line touched by this change (both tools also flag pre-existing issues elsewhere in phase_flip_runtime.py and invariant_checker.py, outside this diff's hunks, left untouched as out of scope for sgl-project#912).
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.