Conversation
Collaborator
Author
|
Thanks to @yichuan520030910320 for catching this bug! |
Merged
3 tasks
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
… -- sweep the four places, not one field window-947 armed the relocated actuator at a site the census PROVED runs (ring:pre_plan 12483) and it still never acted: `sgl-project#946 PREMISE RECOMPUTE` 0 while `UNRESOLVABLE` fired on exactly the frozen rid. Three explanations survived that boot and it could not separate them, so each got ONE counter rather than a fourth guess. THE DESK SEPARATED IT, no metal needed, because it is a property of two call sites rather than of runtime state: * the MARK is written over `can_run_list` (scheduler.py, after `build_pp_admission_decision`); * the ACT read ONE field, `self.chunked_req`. A request marked anywhere else was marked for ever and inspected never. Proven rather than argued, in `test_THE_SEPARATION_a_marked_request_that_is_not_the_ chunked_req`: handed `chunked_req` the actuator returns "none"; handed the request the mark is actually on, the SAME actuator returns "recompute". Same code, different argument. Candidate (b) confirmed; (a) and (c) are instrumented but did not need to be the answer. THE FIX: `pp_apply_dead_premise_anywhere` sweeps `pp_request_locations` -- the four-place enumeration built for sgl-project#946's candidate set, now its SECOND consumer, which is exactly why it was factored out instead of inlined. A fifth place is now one edit for both consumers rather than a fifth ticket. THE THREE COUNTERS SHIP TOO (`pp_premise_probe`), because the next surprise in this family will not be this one: mark_hit / mark_miss (a) was the escalated rid in `can_run_list`? act_rid_hit / act_rid_mismatch (b) is the inspected request the marked one, and `where=` names which of the four places it lives in gen_mismatch (c) counted WITH BOTH generation values, because "they differ" without the numbers cannot tell a cutover from an unreadable stamp ABSENT != ZERO, the sgl-project#947 rule: a kind that never fired does not appear at all, so a null reading is admissible evidence rather than a shrug. Rate-limited (`SGLANG_948_PROBE_EVERY`, default 500); first sample per kind kept verbatim, not one per occurrence. CAN-FAIL ARM PER COUNTER, since three indicators nobody had shown could move would be this family's own failure mode repeated: each constructed case makes exactly its own counter non-zero and leaves the other two ABSENT, and `gen_mismatch` is additionally driven through the REAL actuator with a stamp from a superseded generation rather than by calling the probe by hand. Kein-Doppel-Prefill is unchanged: the sweep still prefers the re-fetch and KEEPS the prefix; `told=0` remains the bounded last resort that names its discard. TESTS, per file, each module alone in a fresh process, against the 70bbd98 baseline: 791-congruence 11/11, 630 5/5, 791-wiring 3/3, 798 12/12, 797 31/31, 944-unit 30, 944-gloo 6, 946 28 (16 + 7 separation/can-fail + 5 sweep). 126 passed, 0 failed. ruff (F401,F821,UP037) and ruff format clean.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 28, 2026
window-958-boot died 25 s into the chunked acceptance load at scheduler.py:7010, `AttributeError: 'NoneType' object has no attribute 'end'`, one line after `sgl-project#946 PREMISE RECOMPUTE`. THE ATTRIBUTION IN THE WINDOW CLOSEOUT IS REFUTED, BY TWO INDEPENDENT INSTRUMENTS. It put the null on the sgl-project#906 seam-refusal branch (scheduler.py:8916). * LOG. The full-phrase counter `[sgl-project#906] SEAM CHUNK REFUSED rid=` is 0 in BOTH boots, and `_note_seam_chunk_refused` logs its first three occurrences unconditionally (scheduler.py:5434-5446), so the zero is a measurement and not a rate limit. Other bracketed INFO tags from the same process are present in the same file, so the sink is not the explanation. * COVERAGE, which does not depend on any logging decision. The boot rode `SGLANG_949_COVERAGE=1`; boot 1's three rank databases (evidence-665-f1/trace949_0828/.coverage.24735{12,13,14}) all report scheduler.py:8916 **not executed**, while :8911 (the truncation), :8918 (the adder), :7010 (the reader) and scheduler_pp_mixin.py:2159/:2161 (the ring actuator and the call below it) are all executed on all three ranks. A fix at that branch alone would not have touched this crash. The junction took the ELSE branch every time and re-derived, exactly as sgl-project#946 argued. THE PRODUCER IS IN THE TRACEBACK, two statements above the reader. scheduler_pp_mixin.py:2159 calls `pp_apply_dead_premise_anywhere` -- sgl-project#948's relocated actuator, armed for that boot by `SGLANG_946_ACT_AT_RING=1` in the window's own recipe -- whose terminator runs `truncate_prefix_to(0)`; :2161 then calls `get_next_batch_to_run`. Nothing re-derives in between. sgl-project#946 had justified the truncation by its NEIGHBOURHOOD ("`add_chunked_req` below derives everything from `len(req.prefix_indices)` and only THEN calls `set_extend_range`"); sgl-project#948 moved the act to a site that RUNS, for a measured reason recorded at scheduler_pp_mixin.py:2100-2109 (the old site was entered ~6 times while 9471 passes voided), and the legality argument did not travel with it. FIX AT THE WRITER, ONE PLACE. `Req.truncate_prefix_to` leaves `Range(told, told)` -- zero rows at the prefix that now exists -- instead of `None`. This satisfies `_executed_extent`'s invariant `extend_range.start == len(prefix_indices)` by construction at the only place that can break it, and closes four producers in one cut instead of one branch per boot: the ring actuator (:2159), the seam refusal (scheduler.py:8916), `add_chunked_req`'s hybrid-SWA zero-budget return (schedule_policy.py:1396, which unlike the sgl-project#679 park at :1434-1436 returns without `set_extend_range`), and the sgl-project#791 clamp sites on a `NO_TOKEN` break. None of the four can now receive a null geometry, because none is produced. ONE EDIT WAS WRITTEN AND WITHDRAWN, recorded in the code rather than dropped. Making that hybrid-SWA branch write the park geometry -- so the two park branches say the same thing -- broke `test_prefill_adder.py::test_add_chunked_req_hybrid_swa_defers_when_swa_ below_page`, which pins "returned unchanged" via `set_extend_range.assert_not_called()`. With the writer fixed the branch is no longer a producer, so the edit would have been consistency rather than a fix, and it is not free: it would overwrite the PREVIOUS chunk's range on any path reaching this branch before that chunk is stashed. In production the stash runs earlier in the same pass, so the write would be value-neutral -- but that is an argument, not a measurement, and hybrid SWA is not a configuration this fork boots. Reverted, and the divergence between the two park branches is named at the site as open. sgl-project#958's ARGUMENT IS HONOURED, NOT REVERSED. Its "NONE, NOT A RECOMPUTED RANGE" paragraph refuses `Range(told, old_end)` because keeping the old end would INVENT a pass. `Range(told, told)` invents nothing, and it is not a new state: `_park_chunked_prefill_chunk` writes `Range(start, start)`, the sgl-project#679 park writes it, and `_executed_extent` declares zero-length ranges first-class. The offer still moves -- now WITHOUT the adder: `_executed_extent` reads (0, 0), so PP0 offers told=0, the value `reconcile_pp_admission_decision` admits unconditionally. `reset_for_retract`'s `None` is deliberately untouched: two disposal sites key off that sentinel (scheduler_pp_mixin.py:6061-6075, :7222-7236) and flipping it would have silenced them. The refused-geometry exit stays reachable from that producer and is now pinned by its own test. THE COMMIT'S OWN SAFETY NET WAS DOWNSTREAM OF THE CRASH. `PPScheduleRefused` / `require_executed_geometry` fired 0 times on metal while the unguarded dereference killed the process, because it iterates `can_run_list` and a resident continuation the adder did not add is never in it. It is not made reachable here; it is made unnecessary, and the structural reason is asserted rather than argued. SIBLING, same class, fixed here so this change does not widen it: `_park_chunked_prefill_chunk` handed back the `inflight_middle_chunks` increment whenever it got past its `end is None` gate rather than only when a chunk was actually prepared. Already reachable before this change via the sgl-project#679 park's `Range(prefix, prefix)`. The predicate is now the same `end > start` the KV release beside it already used -- one expression, not two. #962a: THE SEAM PROBE COULD NOT PROVE ITS HOOK RAN. The reachability probe `cutover_participants.py` registers for `latched_batch_flags` was emitted only `if any(_stale.values())`, so "ran and found nothing" and "never ran" were byte-identical -- the sgl-project#719 shape the registry's own docstring forbids. It is now unconditional and reports `reached=`, because W37-C already showed a bare zero is not enough (it logged `checked=0` eighteen times and was still blind). sgl-project#962 ITSELF IS REFUTED, no code change warranted. `batch_is_full` does not survive the tp_to_pp cutover: the hook is unconditional in `_cutover` with no early return before the completion log; it provably ran (`cutover complete: active stack` 6 and `[sgl-project#690] CUTOVER SUB-STEPS` 6 in both boots); it cleared nothing (`#861c cleared latched batch flag(s)` 0/0); PP0 admits 8 times (boot 1) / 4 times (boot 2) after the cutover before the first latched decline; and boot 1 alternates DECLINE/ADMIT eight times in one second while having MORE latched declines (5 vs 3) and NO livelock. #962b registered, not fixed: #888b's `parked_carrier_relief` re-derivation is on the post-flip path (scheduler.py:8587) but inert, because its gate reads `_parked_decode_verdict`, whose only writer (`_note_parked_carriers`, called at scheduler.py:7675) sits behind `not running_batch.is_empty()` and is unreachable at running=0 -- the state the relief exists for. Measured 0/0 against 8 latched declines. Needs its own danger-direction analysis. TESTS. `test_truncation_geometry_961.py`, 15 tests, RED FIRST at the pin (8 failed / 7 passed before the fix). The `:7010` reader is driven through the REAL `Scheduler.get_next_batch_to_run` on an uninitialised instance carrying the five attributes that line needs, so it reproduces the production AttributeError on the production line rather than on a copy of it; `_Req` borrows the real `Req.truncate_prefix_to`. Five CANFAIL mutants pin each reader to the invariant and pass before AND after. Two further readers are driven for real (`_compute_chunked_req_next_prompt_token`, `pp_chunked_local_match`) plus the real producer (`build_pp_admission_decision`). `test_offer_delivery_958.py`: its EXIT_3 test required the refusal this fix makes unproducible; corrected to assert the moved offer, and split so EXIT 3 stays pinned against the `reset_for_retract` producer that still reaches it. `test_latched_batch_flags_861c.py`: 3 tests for the #962a receipt, including a can-fail that a blind seam is not reported as an all-clear. DESK GATE, /spinning/htsglang-gpu/.venv, CVD="". BEFORE (frozen at the pin 78d030e): serial 895 passed / 2 failed, wide 3701, narrow 202. AFTER: see NOTE below. Failure set unchanged: the two pre-existing test_collective_family_siblings_610.py failures, untouched. sgl-project#954 (test_prefetch_progress_symmetry_580.py) is outside gate scope, as before. ruff: no finding on any of the 273 changed lines (all 64 pre-existing); new test file clean and ruff-formatted. codespell: new file clean; the one hit in phase_flip_draft_bootstrap.py:558 is pre-existing. No boot was run. /spinning/gpu-arb/TICKET_961_WINDOW.md carries the boot acceptance and is drivable from that file alone.
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.
This PR fixes the bug in which the server repeats
get_new_prefill_batchforever but gets no one out.This happens when:
max_new_tokensis set toNone.max_req_input_lenis decided bymax_total_tokensreturn_logprobisTrueThen:
The
max_new_tokensis set tomax_total_tokens-len(input_ids)- 1. The current implementation has unalignedlast_nodeandprefix_indices, when we calculate the available space for a new prefill batch, the radix cache lock status gets wrong and never accepts new requests.