Clean up unit tests - #1020
Merged
Merged
Clean up unit tests#1020
Conversation
merrymercy
enabled auto-merge (squash)
August 10, 2024 22:01
merrymercy
disabled auto-merge
August 10, 2024 22:08
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
Sep 2, 2026
… group STOP, not a void Root (HANDOVER_1153_0902, PROVEN, pre-existing at the pin 228a66d): scheduler.py _pp_refuse_forwarded_schedule answered PPScheduleRefused ('sgl-project#791 FORWARDED SCHEDULE UNEXECUTABLE', PP1 reached 1 of the 2 rids PP0 named, batch_full_break) with a rank-local compensation: _pp_admission_pass_voided = True + emptied decision dicts -> mixin _pp_void_own_batch -> 'sgl-project#631 ROW-DELIVER BATCH NULLED slot=0 pass_voided=True' (boot_855_weg1b2 log 65000-65004) -> PP1 sent no proxy while PP0's slot stayed set -> nothing carried the void upstream (sgl-project#797 return trip: zero call sites since CUT V; sgl-project#1072 deleted the void relay) -> PP0's blocking _do_recv consumed PP2's NEXT output under this slot's label (log 65119) -> one output ahead for the rest of the boot -> the pp_to_tp arm at 21:42:05 turned the debt into an unproducible output -> sgl-project#980 ObjectRecvStalled 60 s / sgl-project#1071 PpChainRecvStalled 90 s. Same form sgl-project#1071 (169f53c) deleted for _pp_void_retracted_pass; this was the second writer of the same flag. F1 ROOT: the compensation is deleted, the detector is kept, the refusal is a group STOP. scheduler.py get_new_batch_prefill's except now does `raise self._pp_forwarded_schedule_stop(refusal) from refusal`; _pp_refuse_forwarded_schedule (flag write, dict emptying, sgl-project#971 re-home) is removed; the new _pp_forwarded_schedule_stop logs the kept 'sgl-project#791 PP-ADMISSION forwarded schedule REFUSED on rank N' line and returns a RuntimeError formatted by pp_admission_congruence .forwarded_schedule_stop_message: 'sgl-project#791 FORWARDED SCHEDULE UNEXECUTABLE STOP rank={rank} slot={slot} told=[{told}] reached=[{reached}] census={census} local={local} limiter={limiter} running_bs={running_bs} parked={parked} r2t_avail={r2t_avail} headroom={headroom} group_limit={group_limit} batch_full_setter={batch_full_setter} batch_full_at_loop_entry={batch_full_at_loop_entry}: {refusal}' Every probe is guarded (n/a on an unreadable value); the reached rids are recorded after the admission loop before any of the three raises; the batch_is_full setter site is recorded at each writer of the pass (no_allocatable_reqs_gate, count_arm, disagg_prefill_r2t_avail, add_one_req_NO_TOKEN). Group stop mechanism (existing, no new collective): the RuntimeError leaves run_event_loop; run_scheduler_process (scheduler.py 'except Exception') logs 'Scheduler hit an exception' and parent_process.send_signal(SIGQUIT) (+ killpg/kill_process_tree under SGLANG_KILLPG_ON_SCHEDULER_EXCEPTION); peers end in the bounded sgl-project#980 / sgl-project#1071 receives or the barlink dead-peer probe. F2 TRIGGER CLASS (PP0 order): on a forwarded schedule the follower's rank-local seat-count veto is not a verdict. pp_admission_congruence .rank_local_count_veto_applies(scheduled_extents) is True on PP0 and on every non-PP boot (unchanged expression) and False on a rank > 0 executing PP0's decision; the sgl-project#823 count arm, its batch_full_break, and the three pre-loop count gates of the same arithmetic (batch_full_or_empty_queue's batch_is_full half, min_free_slots_delay, no_allocatable_reqs) are gated on it. The physical allocator still refuses (NO_TOKEN -> membership refusal -> STOP naming the numbers). F3 SIBLINGS: mixin _pp_void_pass_without_upstream_launch's writer of the flag is provably unreachable (pp_upstream_void_pending returns False on every path; its final statement is `return False`) -- left with the proof in a comment. The row-authority _row_skip_plan exit now nulls a slot through _pp_null_frameless_slot, which applies the same named sgl-project#1020 'VOID REFUSED ON A LAUNCHED SLOT' guard as the void path; the guard is factored into _pp_slot_holds_unconsumed_launch(mb_id, site) and used by both sites. Matched check (error class: a follower still ends a PP0-launched pass silently): grep -rn '_pp_admission_pass_voided = True' python/sglang/srt -> only scheduler_pp_mixin.py (the unreachable sgl-project#801 writer); chain scheduler.py raise PPScheduleRefused (:11633/:11646, :11606) -> except PPScheduleRefused (:9441) -> raise self._pp_forwarded_schedule_stop (:9472) -> RuntimeError (:9623); the flag's only readers are _event_loop_pp_body (:4519 -> _pp_void_own_batch) and the scheduler.py void guard (:8955), neither reachable from the refusal. Tests (hermetic, CUDA_VISIBLE_DEVICES=""): new test_pp_forwarded_refusal_stop_1153.py (T1 STOP form + no void, T2 the count-veto helper + source pin, T3 the sgl-project#1020 guard on the frameless null, + the mixin:9389 unreachability proof): 17 passed after; on the parent ca4c6b7 (git worktree) it is a collection error (new names). test_pp_refused_pass_keeps_continuation_971.py: 12 tests that pinned the old compensation INVERTED with the withdrawal named in each docstring (24 passed after; the inverted 12 are red on the parent: 16 failed / 8 passed / 1 error across both files). Mutants: M1 flag write restored in the STOP builder -> 2 T1 red; M2a helper returns True -> 1 T2 red; M2b count-arm gate dropped in the loop -> 1 T2 red; M3 guard dropped from _pp_null_frameless_slot -> 1 T3 red. Bounded suite (7 unit/managers files + test/registered/scheduler): before 103 failed / 527 passed / 1 skipped / 27 errors; after 103 failed / 544 passed / 1 skipped / 27 errors -- per-file identical except the new file (+17). ruff check clean on all touched files; ruff format --diff hunk count unchanged vs parent (15/14/3, pre-existing) and the two test files formatted. Evidence tier: DESK-PROVEN. Boot-3 acceptance: grep -F 'sgl-project#791 FORWARDED SCHEDULE UNEXECUTABLE STOP' in the same second as 'sgl-project#791 PP-ADMISSION forwarded schedule REFUSED on rank', and zero 'ROW-DELIVER BATCH NULLED ... pass_voided=True' lines.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 3, 2026
…mainders; quiescence is an arm precondition; followers execute every launched pass under the arm or STOP weg1b4 08:07:50Z: PP0 launched fwd_ct=81 slot=1 for rid b64dc1cb (extend 1912), posted its sgl-project#631 PROXY-SEND t6, and ONE LINE LATER armed pp_to_tp on a DRAINED verdict reading "0 tok remaining" -- the sgl-project#1153 starvation shape with no designed STOP. D1 THE DRAINED PREMISE COUNTS EVERYTHING STILL TO PREFILL. Scheduler._pending_prefill_tokens now carries an in-flight microbatch term: for every request the PP ring launched but has not returned, it prices len(origin_input_ids) - extend_range.start (NEVER - end: end is advanced at PREPARE time, so pricing from end reads 0 for exactly the request that is still running). Deduped against _queued_ids so a request cannot be billed twice; the chunked and resident terms skip _inflight_ids for the same reason. _pending_prefill_terms records the breakdown and its producer, and all four pp-exit arms print it: "[sgl-project#1173 pending terms: queue=.. inflight=.. chunked=.. arriving=.. resident=.. total=.. (producer Scheduler._pending_prefill_tokens, include_health=..)]". On the weg1b4 numbers the ring reads 1912 for b64dc1cb where it read 0 before. D2a QUIESCENCE IS A PRECONDITION OF THE ARM, NOT A HOLD AFTER IT. PhaseFlipRuntime.arm() takes launched_passes_fn (build_launched_passes_fn reads the same _pp_launched_pending the sgl-project#1020 void guard reads, so arm and guard cannot disagree about what is outstanding). With any slot outstanding the arm is DEFERRED, not taken-and-held: "sgl-project#1173 ARM DEFERRED: launched passes outstanding slots=[..] fwd_ct=.. direction=.. deferrals=..". The check sits after the rank!=0 early return (PP0-only by construction, no new rank-local verdict, no new sync point) and before the side-effecting _prearm_floor_relief. An unreadable probe never defers. The mbs quiescence hold is KEPT, not deleted: it stays reachable on followers, which take the arm as an order and never run this precondition, and for a pass launched around the order's arrival -- its reason text now says so. D2b FOLLOWERS EXECUTE WHAT PP0 LAUNCHED, ARM OR NOT. _pp_flip_hold_slot released the ring only when every slot was None, so a proxy frame for a launched pass sitting in the typed inbox parked the follower on an order it could not obey while the launcher waited for a frame that never came. The hold now releases while such a frame is stashed (_pp_flip_stashed_frame_forces_advance reads the typed inbox ONLY -- never the "provably in flight" counter half of _pp_row_any_proxy_signal, which a transit frame would false-positive during the armed spin) so the loop walks the ring to the slot the frame names. A frame that never leaves is a group STOP through the launcher, never a park and never a silent spin: RuntimeError("sgl-project#1173 LAUNCHED PASS UNEXECUTED UNDER ARM STOP rank=%d slot=%d fwd_ct=%d rid=%s arm_epoch=%d reason=%s") after 2*ring+2 armed visits. Small item: the phase_policy hold that says the premise was verified on the STORE WITNESS now prints the witness STATE census (pending|hit|bounded|cold|unprobed) via phase_purity.store_witness_census. EVIDENCE new test/registered/unit/managers/test_arm_quiescence_1173.py: 14 passed on this tree, 11 failed / 1 passed on parent f58a71b (scratch worktree, same file copied in) = red-first. Mutants, each applied to this tree and reverted: price the in-flight pass from extend_range.end -> 2 red (D1); neutralise the ARM DEFERRED block -> 1 red (D2a); restore the unconditional park in _pp_flip_hold_slot -> 3 red (D2b). All three died. test_pp_flip_slot_hold_631.py: the drained-ring pin is NARROWED, not withdrawn (its docstring says why) and gains the release companion test_does_not_hold_while_a_launched_passs_frame_is_stashed. Touched test files, fixed vs parent: 54 failed / 152 passed vs 54 failed / 151 passed, comm diff EMPTY both directions. test/registered/unit/managers bounded once each: fixed 247 failed / 4684 passed / 5 errors, parent 246 failed / 4670 passed / 5 errors; comm names exactly one extra failure, test_load_snapshot_backends.py::TestZmqRoundTrip ::test_read_returns_latest, which passes SOLO on BOTH trees = the sgl-project#899 box -timing class, not this diff. The 5 collection errors are the pre-existing sgl-project#1044 import-broken *_631 modules on both sides. ruff on the touched files: 104 errors before, 104 after.
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
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.
Add more documentation and clean up tests