Update readme - #731
Merged
Merged
Update readme#731
Conversation
merrymercy
pushed a commit
that referenced
this pull request
Jul 25, 2024
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 17, 2026
…ndle
THE SPECIMEN, 2026-08-17 14:48:22, on a boot whose warmup generation never
reached a first token:
arming tp_to_pp: decode bundle complete: 0 reqs decoded in 183.6 s
(13777 tok prefill waiting) -- exit condition: decode drained
The tp phase was entered with NOTHING decoding, sat 183.6 s (released by the
180 s decode-stall cap), and left claiming completion. Six such cycles in one
boot.
`bundle_at_phase_entry` is `running_bs` AT ENTRY (:2093), so 0 means the bundle
was never resident: there was no work to drain and the visit accomplished
nothing. The sentence was the defect -- a drained bundle and an empty visit
produced the SAME words, differing only by a number that reads as a quantity
rather than as a verdict, so a log scan showed "complete" either way.
LEAVING IS STILL CORRECT and is unchanged: a decode layout with nothing to
decode should yield to prefill. The direction stays TP_TO_PP in both cases;
only the diagnosis differs. This is a wording change in the sense that a wrong
instrument reading is a wording change.
WHY IT MATTERS BEYOND WORDING. The sgl-project#699 admission-wedge detector fired 17 times
on this same boot -- "1 queued, 0 running, and NO first token for 288.0s. Work
is admissible and nothing is serving it" -- and recorded "no phase-policy
corroboration seen". An empty decode phase standing beside a non-empty backlog
IS that corroboration: it is the layout's own evidence that work is queued which
nothing is making runnable. The vacuous message was withholding it, so the two
instruments could not be put together. The new line names ADMISSION as the
suspect and points at sgl-project#731 and at the sgl-project#699 line by number.
Also corrected: the healthy path said "{bundle} reqs decoded", but that number
is the count AT ENTRY, not the number decoded. It now says "{bundle} req bundle
drained", which is what it measures.
TESTS test/registered/unit/managers/test_vacuous_decode_exit_730.py, 6 passed,
built on the specimen's own numbers (13777 tok, 183.6 s). Mutation-proven:
restoring the vacuous branch turns 4 red. Can-fails pin that a REAL drain still
reads as completion, that the two paths never share a verdict word, and that a
running bundle is still refused mid-flight -- a fix that renamed the healthy
path would make every green window look like a defect.
REGRESSION managers 19 failed / 2415 passed against a baseline of 19 / 2409
captured on the reverted tree: ZERO new failures, +6 are these.
NOTE FOR THE TRAIN, unrelated to this fix: the desk train's managers baseline is
19, against 12 on train/0817-exec. The extra seven are
test_kv_arena_handle_retention_631 (7) and test_kv_arena_span_ops_631 (1), with
test_collective_family_siblings_610 at 3 rather than 2 -- pre-existing on this
composition and not touched here.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 17, 2026
…utover
THE CAUSAL CHAIN, measured 2026-08-17 across two boots:
the carry re-homed a request into running_batch/running_mbs[0] and left
waiting_queue untouched
-> the request existed TWICE: resident (invisible to the policy as
runnable) and queued (counted)
-> _pending_prefill_tokens summed the queue and the resident set without
excluding their intersection, billing the same prompt twice:
51,369 -> 102,307 tokens across one cutover, within rounding of 2x
-> the inflated backlog drove the flip policy past its threshold
-> six cutovers, FLIP-CARRY announcing a resident carry while the policy
read `running bs 0`, the sgl-project#699 detector reporting "1 queued, 0 running",
and the warmup generation never served.
THE FLIP CHURN WAS A SYMPTOM. Six cutovers looked like a flip defect and were
not one: the policy responded correctly to a number that was wrong. Nobody
should "fix" the churn separately.
THREE PARTS, because the defect has three surfaces:
1. STATE -- the carry now CONSUMES what it re-homes
(`_consume_carried_from_waiting_queue`). After a cutover the resident set
owns the request and the queue must not also claim it. Both edges pinned: a
request that is ONLY queued stays untouched, and a carry over an
already-consumed queue is a no-op rather than a corruption. Bookkeeping
never raises into a cutover (sgl-project#715 lesson).
2. NUMBER -- `_pending_prefill_tokens` de-duplicates at the INTERSECTION. Not a
blanket "count each rid once anywhere": a request genuinely holding budget in
two places is a real state a future reader may need to see, and a global
dedup would make that class silent the way this one was. Exactly one overlap
is excluded, and only this one. The sgl-project#713(a) resident term did not create the
duplicate state -- the carry did -- but a counter that sums two sets without
excluding their intersection is the second half, and that half is mine.
3. GUARD -- `duplicate_resident_reqs` gains the waiting queue in its universe.
It compared batches against each other and never consulted the queue, so it
reported "no duplicates" meaning "none of the kind I look for". Queue-side
hits carry a `queued:` prefix so the 2026-08-09 resident-vs-resident specimen
and this one stay distinguishable.
Plus the hardening: `_arriving_prefill_tokens` asserted "ARRIVED but not yet on
the queue" and enforced nothing. Its only inflight-bearing call site is
pre-queue so the invariant holds today, but an asserted-never-checked invariant
is exactly how this double count stayed silent, so it is checked now.
TESTS 18 passed. Mutation-proven THREE ways, each caught: removing the carry's
call site, removing the counter's dedup, and removing the guard's queue
universe.
TWO OF THOSE TESTS WERE VACUOUS FIRST, and both are worth recording:
* the carry test called the helper directly, so deleting the call from
`install_resident_set` left everything green -- the helper worked and nothing
used it. It now drives the real entry point.
* the counter test's duplicate had `extend_range=None`, which the sgl-project#713(a) term
skips by design, so the resident side contributed 0 and the intersection was
never exercised. The duplicate is now made countable on purpose.
Both were found by running the mutations rather than by reading the tests.
REGRESSION managers + mem_cache: 19 failed / 3588 passed against a captured
baseline of 19 -- ZERO new failures. Ruff: scheduler.py 94 errors before and
after, all pre-existing; the other two files clean.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 17, 2026
…tem was its interaction with sgl-project#744 PRIOR-ART GATE OUTCOME: sgl-project#731 is not open. The fix is fdcf837, authored by F4-r4 2026-08-17 15:42, and it is ALREADY AN ANCESTOR of this branch. The ledger says so too ("sgl-project#731-Fix war schon gepusht fdcf837"). All three parts of the briefed fix shape plus the extra pin I was asked to add are present: 1. STATE -- _consume_carried_from_waiting_queue, carry consumes the entry 2. NUMBER -- _pending_prefill_tokens de-duplicates at the INTERSECTION (deliberately not a blanket per-rid dedup) 3. GUARD -- duplicate_resident_reqs gains waiting_queue, queue-side hits carry a `queued:` prefix so the 2026-08-09 resident-vs-resident specimen stays distinguishable plus -- _arriving_prefill_tokens' asserted-never-enforced invariant is now actually checked (Term 3, the hardening I was told to pin) Building it again would have duplicated merged work. This is the second time this week the gate paid for itself, and the same lesson as sgl-project#741: the corpus includes the ledger. WHAT WAS GENUINELY OPEN is the item the brief flagged and nobody had checked: after sgl-project#744 a carried request's rows reach the KV rung through the parked extent, so does sgl-project#731's consume change what _flip_pending enumerates? If it did, sgl-project#731 would have silently narrowed sgl-project#744's protection and re-opened the illegal-access window from the other side -- and neither suite would have caught it, because each pins only its own half. VERDICT: it does not, and for a structural reason rather than luck. _live_reqs enumerates running_mbs, running_batch, last_batch and chunked_req. It has never read waiting_queue. The carry ADDS the request to running_mbs[0], which IS enumerated, so the carried request's rows reach the parked extent through the resident side both before and after the consume. Before sgl-project#731 it sat in both sets and the enumeration deduplicated by identity; after sgl-project#731 it sits in one. Either way it contributes its rows exactly once. Pinned rather than left as a paragraph, because the next person to touch either enumeration or the carry needs it to fail if they break it. TEST RESULTS New test_carry_parked_extent_interaction_731_744.py, 5 tests, all green: - the extent is identical before and after the consume, driven through sgl-project#731's OWN helper rather than a hand-built after-state; - a CAN-FAIL guard first: the carried request must actually contribute its 51311 rows, otherwise the comparison would pass while proving nothing; - the converse (a queue-only request contributes nothing, which is WHY the consume is safe); - a structural pin, AST-parsed not grepped, that _live_reqs reads no queue -- it fails where the reason is written down rather than somewhere far away. Mutation proof, both KILLED: I1 _live_reqs also reads waiting_queue -> 2 failed (the forbidden interaction) I2 sticky extent never recorded -> 1 failed (the sgl-project#744 side of the pin) Pinned suites all green together, 125 passed + 8 subtests: test_carry_queue_duplication_731, test_evict_rung_flip_park_744, test_evict_rung_nothing_resident_717, test_evict_rung_floor_invariant_717, test_kv_backing_cap_agreement_656, test_kv_backing_recovery_clamp_684, test_kv_backing_exhaustion_662_f4, test_admission_intake_713. No production code changed in this commit -- a test file and a doc rename -- so the managers baseline is untouched by construction. Ruff and codespell clean. Also renames TICKET_745 -> TICKET_746: my sgl-project#744 follow-up filing collided with register-sgl-project#745 (GDN checkpoints). The file now carries a line naming register sgl-project#746 and stating that nothing in it relates to GDN, so the two cannot be merged by a later reader.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 17, 2026
…- unstrangle the funder My sgl-project#744 fix has a live regression and this is the refinement. Specimen 21:46:32 on 72696b0aec: tp_to_pp refused 35x, IDLE-LOCK with 407,622 tokens pending and 0 resident, the guard reporting "no KV provider is registered" because the armed-only gate refused the rung at BOTH sites and left the provider list empty. Seam staging (PP1: needs 1269 MiB, spendable 883, rung wanted SHRINK to 126506 with slack 45526 = a 1455 MiB deficit that WAS coverable) could never be funded. THE ERROR WAS MINE AND IT WAS A DESIGN ERROR, not a slip. Seam-funding eviction of recomputable prefix rows is REQUESTED BY the flip machinery -- it is what "KV capacity is the funder" means -- so refusing the rung for the duration of a flip protects against the 21:18 crash by disabling the thing the flip is waiting on. A wholesale gate cannot be right when the protected party and the requesting party are the same machinery. THERE WERE TWO STRANGLE POINTS, not one. The armed gate was the obvious half. The second was quieter: _nothing_resident() returning False on a parked extent dropped through to the unknown-refuse branch, so even with the gate removed the rung would still have declined. Both are fixed. THE REFINEMENT. The parked extent already carries exactly the information needed to be selective, which is why this is clean rather than a compromise: rows INSIDE it are what the flip is about to pack and may not be touched; every row ABOVE it is recomputable prefix and is precisely what the funding wants. So the extent PINS THE CEILING instead of closing the rung. _parked_ceiling() returns the highest parked row id, -1 when nothing is parked, and -2 for the one case that still refuses wholesale: an UNKNOWN extent while a flip is armed, where there is no boundary to name. Both call sites take req_max = max(req_max, parked), so the evictor is handed a ceiling that cannot reach into the extent. UNKNOWN, DECIDED AND DOCUMENTED as asked: it refuses ONLY while a flip is armed. Outside a flip there is nothing parked to protect, and closing the rung there is exactly the strangle this commit removes. sgl-project#746 (the exact arm-time snapshot) is what removes this last wholesale case; until then it is one narrow, named condition rather than the whole flip window. TEST RESULTS test_evict_rung_flip_park_744.py refined to the new semantics, 19 tests + 2 subtests green. The two tests that asserted the wholesale gate are REPLACED rather than deleted -- they now assert the funding path delivers -- and the crash protection gets its own explicit test that the evictor's ceiling never reaches into the parked extent. Mutation matrix, 4 mutants, ALL KILLED, covering both directions: N1 exclusion removed (parked ignored) -> 1 failed CRASH protection dies N2 exclusion covers everything (gate back) -> 1 failed FUNDING dies N3 collecting site ignores parked ceiling -> 2 failed N4 unknown-while-armed no longer refuses -> 3 failed N1 and N2 are the (c) pair: neither over- nor under-covering survives. Pinned suites green together, 142 passed + 87 subtests: sgl-project#744, sgl-project#731 + the #731x#744 interaction pin, both sgl-project#717 suites, sgl-project#656, sgl-project#684, sgl-project#662-f4, sgl-project#713 admission intake, sgl-project#739 prefill progress. Ruff and codespell clean. PP2 SECONDARY, checked and NOT claimed as fixed. The 0 MiB shrink against an unmoved driver-free column is an already-instrumented condition (kv_backing_relief.py:1915-1927): the code detects it, names retained handles (SGLANG_FLIP_SEAM_RETAIN_HANDLES) or a missing commit chunk as the cause, keeps the cap on and returns 0. The gate refinement lets the rung PRICE again; it cannot make a retained-handle arena PAY. Necessary, not sufficient -- the retain setting needs verifying at the boot, and this commit does not pretend to close it. No boot.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 24, 2026
…ecedented failure Follow-up to f0ed2e4, which predicted that retraction at the seam would feed the flip policy an inflated pending-prefill figure. Verified rather than left as a prediction, because a design risk nobody checks is just a worry. CONFIRMED. `Scheduler._pending_prefill_tokens` (scheduler.py:10508) computes pending = sum(len(req.origin_input_ids) for req in queued) the FULL prompt, not the uncached extend, and there is no prefix-residency term anywhere in the function. A retracted request therefore contributes its entire context regardless of how much of it the fence just persisted and read-through would serve. The bar it is compared against (N, from X and P) is priced on UNCACHED prefill throughput. The tokens are counted once, at a price that is wrong. AND THE SHAPE IS PRECEDENTED, in a comment inside that same function. sgl-project#731, measured 2026-08-17: a cutover left one request both resident and queued, so one prompt was counted twice -- "51,369 -> 102,307 tokens across one cutover, within rounding of exactly 2x. The inflated backlog drove the flip policy past its threshold -- six cutovers, nothing served." So "an inflated pending figure across a cutover drives the policy into thrash" is a MEASURED failure of this exact code path, from a different cause. That moves caveat 2 out of the speculative column entirely. sgl-project#731's fix does not cover this route. It made the carry consume the queue entry, and it deliberately refused a blanket per-rid dedup on the stated grounds that hiding a genuine double-booking would make the class silent the way this one had been. Retraction re-creates the shape without any double-counting at all -- which is exactly why the existing fix cannot catch it and why this needs pricing by cache residency rather than another dedup. Same class as sgl-project#856(b), one quantity further out: a decision is only as good as the quantity it compares, and here the quantity is honest while its PRICE is not. Still explicitly NOT claimed as mitigation: sgl-project#856(b) raising N (8.50 -> 18.06 s, 18614 -> ~39500) makes this harder to trigger. Mitigating accident, not a fix. No code changes. Documentation only.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 24, 2026
… cannot argue for a flip PREREQUISITE for the seam rebuild, and its own defect. The phase-flip seam will RETRACT resident requests rather than carry them -- the carry is what made sgl-project#825's tree reset crash, and read-through makes it unnecessary. But retraction puts each full prompt back in the waiting queue, and `_pending_prefill_tokens` sums `len(req.origin_input_ids)`. That figure is compared against N = C / (1/X - 1/P), where X and P are UNCACHED prefill throughputs. Retracted tokens are not uncached: their KV was computed and the fence persisted it, so re-prefilling them is a CACHE READ -- and a cache read costs the same in TP as in PP. Equal cost on both sides of an inequality cancels, so those tokens cannot make PP cheaper than TP and have no business in the comparison. Pricing them as cold prefill would hand the policy a huge backlog after every cutover, in BOTH directions. sgl-project#731 measured that outcome from a different cause: "51,369 -> 102,307 tokens across one cutover ... six cutovers, nothing served." Its fix (the carry consumes the queue entry) cannot catch this route, because nothing is double-counted here -- the tokens are counted ONCE, at the wrong price. On W25's numbers the shipped sum prices eight retracted 20k prompts at 160,000 against a live bar of 18,614: 8.6x over, from a cutover alone. THE OPTION THAT WAS CHOSEN, AND THE TWO THAT WERE NOT. * CHOSEN -- stamp the residency at retraction. `reset_for_retract` records the fill boundary (`extend_range.end`, the same notion of "computed" the pending counter already uses for a chunked remainder; a request with output has finished its prefill by construction) BEFORE clearing it. Exact, and free. * REJECTED -- residency lookup at the counter. `prefix_indices`, `num_matched_prefix_tokens` and `extend_range` are ALL cleared by `reset_for_retract`, so after the fact the information does not exist anywhere and only a fresh `match_prefix` walk could recover it -- per queued request, per policy round, under the tree lock, duplicating the matching admission already does. * REJECTED -- a dwell-window exclusion. Coarser in the wrong direction: it also excludes tokens that genuinely need computing, so a request retracted mid-chunked-prefill would have its real remainder deleted from the backlog. The stamp distinguishes them (6000 of 20000 computed -> 14000 still count). DEFAULT PATH BYTE-IDENTICAL. A request that was never retracted carries no stamp and is counted in full, so every pre-sgl-project#856 caller gets the same number. A request retracted for a reason OTHER than the flip seam -- priority preemption, sgl-project#731's own path -- has no fence behind it and also carries no stamp, which the can-fail test pins: zeroing anything merely flagged retracted would delete real backlog and pass every other assertion here. ERROR DIRECTION IS THE SAFE ONE. If the cache is evicted under the credit, the stamp overstates residency and this UNDER-reports pending, making the policy LESS eager to flip -- not more. It cannot cause the thrash it exists to prevent. TESTS (hermetic, CUDA_VISIBLE_DEVICES=""): test_uncached_pending_856.py 12 passed, 3 subtests incl. the shipped-sum reproduction of the sgl-project#731 shape, and a source-order assertion that the stamp precedes the clear (afterwards it is unrecoverable) pending-counter consumers (713/677/689/701/819/854/phase_policy) 156 passed retraction consumers (retract_decode_fcfs, prefill_adder, 798, 791b) 28 passed ruff check + format clean
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 24, 2026
…rse, and the GDN mover then retires by construction W27 killed the boot in `resident_mamba_slots` and the guard was RIGHT. This is the root behind it, and it is not the GDN mover. THE ROOT. `retract_all` frees a request's KV rows, its mamba slot and its tree lock ref -- and the scheduler's batch structures keep REFERENCING the `Req`. `_live_reqs` is the one authority for "who is resident" and reads exactly four places: every `running_mbs` slot, `running_batch`, `last_batch`, and the out-of-batch `chunked_req`. Retraction touched none of them, so every seam consumer after it was handed a live request whose resources were gone. The GDN mover was simply the first to look. FREEING A RESOURCE AND RETIRING THE REFERENCE TO IT ARE DIFFERENT JOBS. Doing only the first leaves a live object that every reader has to special-case, and the next reader added reintroduces the bug. Same shape as sgl-project#731's fix, where the carry had to CONSUME the queue entry rather than leave one request counted in two places. `consume_retracted_from_live_universe` retires the reference out of all four, using `filter_batch(keep_indices=...)` and NOT a raw `.reqs` edit -- a batch carries per-request tensors beside the list and a list edit desynchronises them. Pinned by a test that asserts filter_batch was the mechanism, because the raw edit is the tempting shortcut and it fails silently, later. FIXED AT THE AUTHORITY, AND THE SWEEP SAYS THAT IS ENOUGH. Every seam reader of the live set goes through `_live_reqs`: resident_mamba_slots (gdn_flip_mover.py:617), the KV enumeration (:835), the sgl-project#822 census (:1431), :5457, the output trace (phase_flip_output_trace.py:266), and the release itself (:8129). Grepping the seam modules for direct running_mbs / running_batch / last_batch / chunked_req reads returns comments and docstrings ONLY -- no live code bypasses the authority. THE GDN MOVER NOW RETIRES BY CONSTRUCTION, WHICH IS THE POINT. W27's no-retry refused dropping `GdnFlipMover.move()` because doing that with live linear state trades a loud crash for SILENT linear-state loss. Downstream of this fix the trade is gone: with the live universe consumed and the tree dropped, both halves of `flip_mamba_slots` -- resident slots UNION tree checkpoints -- are empty, so the mover moves nothing. No deletion; the same way the KV mover was retired by emptying its input. THE ORDER IS THE SAFETY PROPERTY, and it is pinned against the REAL guard, not a stand-in: `resident_mamba_slots` no longer refuses AFTER the consume, STILL refuses without it, and a genuinely resident request still yields its slot. So the guard is SATISFIED, never weakened -- if that middle test ever stops raising, the silent-loss trade has been made after all. W27's CONFIRMED RESULTS, kept out of the FAIL headline's shadow: * C6 PASS ON METAL: the sgl-project#825 three-rank crash did NOT occur with the prefix tree dropped under a live request -- 0 `NoneType ... has no attribute 'id'`, 0 `dec_lock_ref`. The retract-before-reset order is vindicated on hardware, not only in its hermetic reproduction. * `choom -n 1000` held on the launcher AND all three rank PIDs before the weights load; no OOM, so W26's failure mode did not recur. TESTS (hermetic, CUDA_VISIBLE_DEVICES=""): test_retracted_leaves_live_universe_856.py 15 passed incl. the W27 specimen reproduced (a freed request still enumerated), all four live-universe routes, the filter_batch mechanism, seam-safety (no targets / no match / a refusing filter_batch / a bare scheduler), and the three real-guard derivation tests. ruff check + format clean on phase_flip_runtime.py Full window record: /spinning/gpu-arb/W27-RESULT.md
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 25, 2026
The missing half of sgl-project#856. `retract_all` returned the list it retracted, `_release_residents_for_cutover` returned it upward, and its caller discarded it. No seam path ever called `_add_request_to_queue`. The seam's own log line promised on every flip of every boot that "the new layout re-admits them and serves the prefix by read-through"; nothing performed the re-admitting. W31 arm 2 measured the cost: 28 distinct rids, each admitted EXACTLY once ever (three ADMIT lines apiece, one per rank), 14 requests prefilled once, 78 requests retracted across 42 cutovers, and ZERO completions -- every client waited out its 600 s timeout. W30 and W31 were both read as a flip "livelock"; they were the flip ping-ponging over an instance whose work it had already dropped on the floor. `Scheduler.readmit_seam_residents`, called from the release site. ORDERING: consume FIRST (inside `_retract_and_consume`), requeue SECOND. A request that is simultaneously live-referenced and queued is double-billed by every consumer that sums the two -- the sgl-project#731 shape. QUEUE POSITION: FRONT, as a block, in original arrival order. These are the oldest work AND the flip's own justification -- the tp-ward arm fires because they are ready to decode. Appending them behind arrivals that landed during the ~6 s flip lets a busy instance starve the bundle it just flipped for. Order is restored from `kv_arrival_seq`, which `_add_request_to_queue` already preserves across a retracted re-queue. ABORT PATH: correct by construction, because the requeue happens AT the release site. There is no window in which the list exists and is owned by nobody -- if the cutover raises after this point the flip abandons, the layout is unchanged, and the requests are already on the SOURCE layout's queue, which is where an abandoned flip should leave them. Deferring this to the end of the cutover would recreate the defect for exactly the abort case. sgl-project#703 FENCE: asserted, not assumed. `_writeback_fence_ms` returns None for "NO FENCE RAN" -- a real state, since the fence is skipped without a canonical store -- and that must never read as "fenced, cost 0 ms". The seam names it when it re-admits unfenced: not a wrong answer, but the silent cliff this no-carry design exists to avoid. ONE MOVER, NOT TWO: it calls `_add_request_to_queue(req, is_retracted=True)` and then moves that block to the front, so it inherits the priority validation, the queued-limit abort, the retract timestamp and -- load-bearing here -- `_prefetch_kvcache`, which is what makes the promised read-through hit. RETRACTED MUST EQUAL READMITTED: the seam compares and logs `RE-ADMISSION MISMATCH` on any difference, so the next boot's first check is arithmetic rather than inference. TESTS (14, red-first; the stand-in binds the REAL method) * all N return, exactly once, queued as retracted; * FRONT of newer work; arrival order restored even though the seam enumerates residents by slot, which is not arrival order; * a request the queue legitimately refuses is not conjured into the block; a client that gave up is not re-admitted; the count reports what LANDED, not what was tried; * the stamp SURVIVES the round trip, checked through the real `seam_readmit_candidates` -- stripping it would put the requests back where strict purity still refuses them, i.e. the W30 livelock again; * rank-uniformity: two ranks given the same requests in different slot-enumeration orders rebuild the identical queue; * abort path and consume-before-requeue pinned by source order. CAN-FAIL, proven by probe: re-applying the original defect (discarding the return value) turns 3 tests red; restoring makes all 14 green. Full managers suite 8 failed / 4054 passed -- the same pre-existing 8; the strand reads 9/4004 -> 8/4016 -> 8/4035 -> 8/4054. ruff clean. Window record: /spinning/gpu-arb/W31-RESULT.md
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 3, 2026
…the stashed-frame budget is keyed to the frame and counts arrivals at its own slot Both reviewer BLOCKERS fixed at their root, not at the symptom. BLOCKER A -- the launched-pass set survived the ring rebuild. `_pp_launched_pending` is ring-scoped bookkeeping (slot numbers of the ring that launched them), but `pp_flip_forget_ring_scoped_slots` -- the function whose own sgl-project#829 docstring claims to own exactly that class -- did not clear it. A retraction rebuilds the ring, `arm()`'s quiescence precondition then reads a slot from the DEAD ring and defers for ever: the TP-sticky shape the reviewer named. Fix is one statement in the designated authority, and it CLEARS rather than intersects with `range(pp_loop_size)` -- the ring is rebuilt EMPTY two statements after the call, so an in-range survivor would be exactly as wrong as an out-of-range one. BLOCKER B -- the stashed-frame budget could stop the group falsely. The counter was neither keyed to a frame nor to an armed window, and counted loop VISITS. Three corrections, each with its own test: (a) keyed to (arm epoch, frame stamp) -- back-to-back frames each get their own budget instead of summing into a false STOP, and a disarm cannot leave a count for the next window. Every early return of `_pp_flip_hold_slot` now forgets the window through `_1173_forget_stashed_frame`; in the TP phase the bound is 4, so a single inherited visit changes the verdict. (b) a visit counts only when the loop ARRIVES AT THE SLOT THE FRAME NAMES. That is what the STOP claims -- "the receive kept getting the chance and still did not take it" -- and it is the only form that is reachable on the TP ring: `pp_loop_size` is 1 there, `mb_id` is 0 on every iteration and never CHANGES, so an mb_id-change detector would have been structurally unfireable exactly where the bound is smallest. Own mutant M6 proves it. (c) unreadable live slot or unreadable frame slot => the budget does not advance at all. An instrument that cannot measure the chance it counts must never fire a group stop. Arm side: the deferral was unbounded and its warning unrated. It is now bounded on FORWARD PROGRESS -- a frozen (outstanding slots, fwd_ct) for longer than `launched_pass_stall_s` (default 120 s) escalates to the same named STOP, ring progress restarts the clock. Bounding on deferral COUNT was rejected: `arm()` is driven from the receive poll, so a count bound fires at a rate set by client traffic. The ARM DEFERRED warning is rate-limited like the sibling sgl-project#1020 guard (<=3 or every 512) and carries `deferrals=` and `streak=` so a suppressed gap stays readable. Non-blocking items taken: N4 (the reported `queue=` term is the same reading the verdict used, not a re-derivation), N5 (the sgl-project#731 argument restated for all three in-flight pairs at the loop), N8 (`pending_prefill_terms` declared rank-DIVERGENT by construction -- weg1b4 measured 5739 on PP0 vs 3827 on PP1 -- reporting only, no cross-rank ballot), N9 (the in-flight billing test now uses a request whose `extend_range.end` is below its total, so removing the resident dedup guard actually changes the number). EVIDENCE red-first, reviewed HEAD c32eac6 (test file copied into a scratch worktree): 16 failed / 8 passed of 24. 13 are blocker-targeted; 3 are collateral (the file now passes `launched_pass_stall_s` to the runtime constructor, which does not exist there) -- named, not hidden. fixed tree: 24 passed. mutants, all against the fixed tree, all DEAD: M1 launched-set clear removed -> 2 failed M2 count every visit, not arrivals -> 1 failed M3 frame identity dropped from the key -> 1 failed M4 arm-side escalation neutralised -> 1 failed M5 early returns no longer forget -> 1 failed M6 mb_id-change detector instead of arrival-> 2 failed M7 stall clock never restarts on progress -> 1 failed M3 and M5 SURVIVED a first round and drove two of the corrections above -- recorded, not quietly re-run. sibling test_pp_flip_slot_hold_631.py: 8 failed / 11 passed on both the reviewed HEAD and the fixed tree (its stand-in now binds `_1173_forget_stashed_frame`; without that it was 15 failed). ruff check parity over python/sglang/srt/managers/ + test/registered/unit/managers/: 466 == 466 errors on both trees (all pre-existing); ruff format: the three production files were already dirty on the parent, both new test additions are formatted. matched check (D1) re-run on the fixed tree: the three tests of TestTheDrainedPremiseCountsLaunchedWork pass -- an empty waiting_queue with chunked remainder 1912 and a launched slot reads pending == 1912, not 0. Its mutant M-D1 (drop the in-flight remainder term at scheduler.py `pending += inflight_tokens`) turns exactly those assertions red: 2 failed / 22 passed. Tree restored and re-verified byte-identical afterwards. bounded suite, test/registered/unit/managers, -n 8, one run each: parent scratch c32eac6: 263 failed / 4678 passed / 18 skipped / 12 errors / 565 subtests passed (568.75 s) fixed tree: 248 failed / 4693 passed / 18 skipped / 12 errors / 565 subtests passed (573.82 s) comm of the sorted FAILED/ERROR node-id sets: 16 names cured (all of them the red-first test_arm_quiescence_1173.py cases -- the count matches the red-first tally exactly), and ONE name appears only on the fixed side: test_pp_proxy_readiness_contract_789.py::PPProxyReadinessContract789 ::test_shipped_gate_raises_named_diagnostic_instead_of_wedging That is NOT a regression and was verified, not assumed: run solo, that test PASSES on the fixed tree while a DIFFERENT member of the same file fails -- and the parent scratch produces the identical solo result (1 failed / 3 passed, same member). The failing member of that module wanders between runs; this diff touches neither the module nor its production file. Known sgl-project#899/sgl-project#895 PP-proxy family (NOT_CROWDING_PROVABLE). DEVIATIONS N2 is implemented here, not deferred as the earlier report said. N3 (drain-and-flip queue axis) stays open by design; N6 (D2b is a rank-local refinement of a rank-local predicate) and N7 (two authorities for one notion: `_inflight_ids` slot occupancy vs `_pp_launched_pending` launch bookkeeping) stand as named residue. N11: the earlier report's red-first arithmetic was wrong and is re-measured above. N12: the follower-side raise is rank-local by construction and its message now says what it counted. No boot. Desk only.
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.
No description provided.