feat: add pr e2e test by zhyncs · Pull Request #822 · sgl-project/sglang

zhyncs · 2024-07-30T08:26:43Z

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

Ensure pre-commit pre-commit run --all-files or other linting tools are used to fix potential lint issues.
Confirm that modifications are covered by complete unit tests. If not, please add more unit tests for correctness.
Modify documentation as needed, such as docstrings or example tutorials.

… with a parser and a real baseline Item 5 of the sgl-project#822 cut said the clamp stays as belt-and-suspenders and its firing rate becomes the regression metric. The previous commit stated that as an intention. This makes it code. `parse_clamp_firings` / `clamp_firing_census` (mem_cache/kv_row_ownership.py) read "KV-BACKING exposure clamp" out of a boot log with its three numbers, and `CLAMP_BASELINE_0823` / `CLAMP_BASELINE_ROWS_0823` carry the measured baseline. THE BASELINE IS 12 FIRINGS, NOT 3. The sgl-project#822 brief said the clamp "caught it three times live". Counted against /spinning/evidence-665-f1/boot_816_core_0823_0608.log the rate is TWELVE -- four per rank, at five distinct second-marks (06:14:21, 06:14:22, 06:18:15, 06:19:43, 06:32:05). Three was the number of cited log POSITIONS. A regression metric seeded from three would have scored a nine-firing boot as an improvement, which is the failure mode the INDIKATOR-GESETZ names: an indicator is a finding only once it is checked that it measures what it claims. AND THE NUMBERS DO NOT DRIFT. Every rank reported identical exposed/committed/ unbacked on all four of its firings: PP0 466994 / 212992 / 254002 PP1 466994 / 124928 / 342066 PP2 466994 / 133120 / 333874 A leak accumulates; this did not. Flat across a whole boot, with the exposed figure rank-INVARIANT while the backing is rank-local, is the signature of a global id space over a per-rank arena -- i.e. structural, which is why single site fixes kept not holding. The suite closes the loop: `test_the_law_reproduces_every_logged_firing` asserts the authority's EXPOSURE verdict equals what the clamp actually saw, for each rank. If the law and the actuator under it disagreed on even one, there would be no way to tell which was wrong. TESTS. Hermetic, CUDA_VISIBLE_DEVICES="", PYTHONPATH pinned to the worktree. test_kv_row_ownership_822.py + test_census_withheld_term_814.py: 39 passed, 6 subtests passed. M8 parser marker typo (counts nothing) -> 2 failed. The parser is also proven in the other direction (test_parser_ignores_everything_else): it finds nothing in lines that merely mention the same subsystem, so the count cannot be inflated by neighbouring log traffic. Eight mutants now, all dead: M1-M4 on the four laws, M5-M7 on the wiring, M8 on the metric's parser.

16g: funding_authority, solve_arming_floor at 218 instead of 512, the retraction fix to the abandon message, and sgl-project#819 provenance. Clean merge. Shares phase_flip_runtime.py with feat/822 and kv_backing_relief.py with the already-merged sgl-project#816; hunk regions checked disjoint before merging (sgl-project#770 at :6120-6972 vs sgl-project#822 at :3741-3798/:8029).

…gl-project#821, sgl-project#823, sgl-project#824, sgl-project#770/sgl-project#812, sgl-project#815; close the section 18 breach Merge-checklist duty for the five wave-2 stages on this branch (fix/801, fix/824, feat/770, feat/822, fix/815). Mechanism text is taken from the commits' own measured evidence, not restated from ticket titles. WHAT WAS ALREADY THERE, checked before writing rather than after. feat/822 catalogued ITSELF mid-train -- 21ff075 carries +49 lines of this file -- so sgl-project#822 already has a complete section 3 entry including the sgl-project#816 clamp firing-rate baseline. No second sgl-project#822 entry was written. sgl-project#801, sgl-project#815, sgl-project#821, sgl-project#823, sgl-project#824 and sgl-project#770 had zero prior mentions; sgl-project#812 was name-checked inside the KV-backing-relief bullet as a consequence of the cap, with no entry of its own. SECTION 12, five new families: Said-once (sgl-project#823) -- a fact that changes during a process, reported once at the moment it first became true, is a stale label afterwards. Both instances live in _update_uniform_pool_budget: the floors' coverage said three times at startup and never again through four cutovers in 55 s, and a divergent TP queue head logged as an event when it is a duration. The mutant that makes the case: disabling the recovery edge left all 14 tests green before the streak was extracted. Blind-instrument (sgl-project#821) -- the scheduler watchdog's activity predicate reads exactly the two values a rank parked in the dict receive freezes, so the instrument best placed to see the wedge is the one structurally unable to. Non-monotone-source (sgl-project#824) -- a monotone guarantee (the protected prefix) derived from a non-monotone tracker (mamba_last_track_seqlen). Landed as a measured assertion, cache_protected_len=16384 against page_aligned_len=8192. Invariant-for-funding (sgl-project#770, sgl-project#812) -- a group-derived per-rank floor asking PP1 for 102.9% of its own cap, and the withdrawal of a floor clamp that bought funding by breaking the levelling invariant (1 failed with, 12 passed without). Stub-drift (sgl-project#815) -- ten helpers drifted behind named commits, and the rule the repair follows: faithfully or not at all. This entry also RECORDS why seven cases in test_pp_flip_slot_hold_631.py are deliberately left red -- the faithful stub is a real rank identity, which drags the whole admission-receive path in with it. That is documentation of a known state, not a claim that it is fixed. Plus a NUMBER-COLLISION note, because the log is ambiguous without it: 9d13bf0 is subject-tagged [sgl-project#824] and names its test file _824.py but is canonically register-sgl-project#823 (floor-scope reporter, strand 16f, arriving on fix/801-admission-recv-liveness); the canonical sgl-project#824 is 658d0c1 on fix/824-chunked-prefill-protected-len. No rebase, so the titles stand and the mapping is written down instead. SECTION 7: sgl-project#801's dead-peer measurement, which REFUTES the premise under the sgl-project#816 survivor analysis -- an exited or SIGKILLed peer makes the admission receive raise after 1.02 s, while alive-never-sends and SIGSTOP both block unbounded, so a rank silent for 119.7 s was not waiting on a corpse. SECTION 2: the funding authority (sgl-project#770/sgl-project#584/sgl-project#819) -- one question, posts declared by name, the three priced losses (L1 the unreachable 2638 MiB, L2 a 3437-row ask against 8192-row granularity returning a silent zero at 65% of the shortfall, L3 claimed=0 against an accounted 107 MiB), and the arming floor solved rather than asserted (512 MiB reserve needs 1523 against a 1229 ceiling = structurally unreachable; 218 is the largest that fits). sgl-project#819's break-even carries its provenance because X and P are env-overridable defaults that are never runtime-measured. SECTION 18 -- THE BREACH, CLOSED. Rule 5 says a PR adding a reusable module adds its entry in the SAME PR. Three modules had landed without one: kv_row_ownership.py (sgl-project#822), funding_authority.py (sgl-project#770), uniform_floor_scope.py (sgl-project#823). prefetch_ballot.py (#791b) had the same gap from an EARLIER merge and is added here labelled as backfilled -- the provenance is stated rather than quietly absorbed, because the checker only tests citations that EXIST, so a missing entry costs nothing and is exactly the failure mode rule 5 cannot catch. Each entry says what is NOT wired, in those words. Both authorities are so far consulted only for the arming floor, the census and the retire/audit pair; the clamp firing-rate metric, format_violations, diagnose_floor_band, uniform_absolute_floor and slack_above_uniform_floor have no production caller at this commit. Recording an unwired export as wired is how a catalog becomes a liability. One trap recorded: funding_authority's docstring at :56 names a solve_funding function that does not exist -- the solve is can_fund at :378. TWO DEFECTS FOUND WHILE WRITING, both fixed here: 1. Section 18 cited `watchdog.py:88`, which resolves to python/sglang/srt/watchdog.py -- a file that does not exist in this tree. The intended target is turnkey/watchdog.py:88 (the retired generation probe), now qualified. This was a live red test, not a cosmetic issue. 2. The sgl-project#822 section 3 entry cited kv_backing_relief.py:504 for exposure_over_backing; :504 is a docstring line and the def is at :547. The entry landed already drifted. Corrected, with the drift noted in place per the header rule that the code wins. TESTS. test/registered/unit/docs/test_building_blocks_catalog_538.py, the mechanical section-18 pointer checker: 91 passed / 1 failed BEFORE (the watchdog.py:88 breakage), 101 passed / 0 failed AFTER. The ten new cases are the ten new citations, and every one resolves. The indicator is proven in both directions without needing an injected mutant: it was red on a real bad pointer of exactly the class it claims to detect, and is green now. codespell on the file: clean.

Window 7's 844-O2 read NEGATIVE from one line: ... cur_slot_reqs=5 resident_reqs=0 resident_slots=[] unaccounted=40960 40960 exceeded the pre-sgl-project#822 maximum and resident_reqs was zero, so the resident-working-set explanation looked excluded and the reading went back into the queue as a possible fifth owner or ownerless load-time holding. Neither exists, and the boot's own audit had already said so at the same instants. The sgl-project#822 fourth owner (_resident_rows) was wired into the ownership AUDIT only -- audit_pool_census received it, and the audit's unowned residue at 03:00:11 / 03:01:04 / 03:01:56 is exactly ONE row ("1 committed row id(s) ... belong to no enumerated owner", 33 such lines, a different single id each census, never accumulating). The census LINE never subtracted that owner: its arithmetic is still `range - free - cached - withheld`. And its `resident_reqs=` field is the running_mbs slot scope -- 0 by construction outside the PP event loop -- which in the TP regime masquerades as the fourth owner's verdict. The line manufactured the false leak; the audit, one log line away, had absorbed 40959 of the 40960 rows into resident:requests. The fix is display-plane and additive (I832 pattern): * `resident_census_terms` derives two fields appended at the END of the line: `resident_rows_n=` (the fourth owner's row count) and `unowned_after_owners=` (the audit's arithmetic, on the line). All existing fields stay byte-identical, `unaccounted=` keeps its historical meaning, existing parsers and specimen greps are untouched. * `None` from the enumeration prints `no-verdict` on both terms -- asserting absorption from an empty set is the sgl-project#822 defect with an extra step (same contract as _resident_rows and audit_pool_census). * The counted-free branch is a count difference over in-space resident ids and may go negative on owner overlap; reported as it falls, the same rule `unaccounted` already follows. * ONE enumeration per census, shared with the audit (resident_rows=_RESIDENT_UNSET sentinel keeps explicit None meaning "no verdict"): window 7 had the line and the audit enumerate separately, bracketing the log I/O of a moving working set. The 1-row-per-census residue is real and named: a single moving id (10281/119211/121481/123646/20534/45290/81568 across the run), rank-uniform per census, never accumulating -- the signature of a one-row enumeration gap around the in-flight allocation, not of a leak. It stays visible in `unowned_after_owners=` and in the audit, and is deliberately not chased here. Tests: test_census_resident_line_849.py -- 12 tests, red (collection error) on the branch base; drives the REAL _pool_census through the 832 harness pattern. Five mutants, each killed by the intended tests: (M1) None-as-empty-set -> both no-verdict tests; (M2) reducing the existing unaccounted field -> byte-identity + absorption tests; (M3) count-instead-of-id difference -> overlap-exactness test; (M4) dropped audit pass-through -> shared-enumeration test; (M5) fallback re-enumerating on explicit None -> fallback-direction test. Full test/registered/unit/mem_cache: 1719 passed / 1658 skipped / 365 subtests; the single failure (test_prefetch_participation_desync_580, gloo 3-rank rendezvous) passes 8/8 in isolation on this tree and was green in both baseline runs -- full-suite flakiness, not a regression. ruff clean on both touched files; codespell: zero new findings (all hits pre-exist on base). Desk-only under window rules: no boot, no GPU. The on-metal confirmation is one grep on the next boot's log: the large `unaccounted=` figures must be accompanied by `resident_rows_n=` of the same magnitude and `unowned_after_owners=` near zero.

THE CLASS FIX, and the half sgl-project#822 named and did not build. `fe43b09e52` states it as its own open item: `_retire_row_id_space` "does not yet REFUSE such an id at the allocator -- that is enforcement". O-2 recorded audit-only as an owner decision on hot-path blast-radius grounds; the reversal is on record in WINDOW-QUEUE with the scope that answers that objection exactly -- enforcement at THE TWO SEAM EVENTS ONLY, where the id space changes regime, never per allocation. The hot allocation path is untouched. WHY IT WAS NEEDED, from my own window: the authority observed "exposed 470755 > committed 126976" and said so 48 times in ONE boot while the instance livelocked on that exact gap, and the sgl-project#816 clamp fired twice. An instrument that names the root once per arm and binds nobody is not a gate. SEAM ONE -- THE CUTOVER (`_execute`, beside `retire(direction)`). `retire()` is the audit half: it stamps a new epoch and reports. The enforcement call sits deliberately OUTSIDE that try/except, because a swallowed audit must not also swallow the gate -- enforcement would inherit the exact silence it exists to end. SEAM TWO -- THE SHRINK RESTATEMENT (`_shrink_to`, success path). The FAILED shrink path already clamped ("undoing the cap must not hand out ids the failed shrink never backed"); the SUCCESSFUL path did not. So a working shrink was the one way to widen the exposed-vs-committed gap with nothing saying so -- which is W22's state exactly: withheld=343779 constant while free and cached swung 126845/131 -> 3729/106449, exposed 470755 against committed 126976, and every id-space-derived quantity downstream still pricing unbacked ids. The accounting was self-consistent; the law was broken. Both branches now call the same actuator, so they cannot drift. NO NEW ACTUATOR. Both seams call `clamp_exposure_to_backing` (sgl-project#816), whose own contract is why it is safe here: it "only ever LOWERS exposure toward `_current_rows()` -- a MEASURED committed count, never a remembered one -- and it never lowers the BACKING". So it cannot cap below a live set. The direction guard therefore holds BY CONSTRUCTION rather than by care, and an under-backed pool is REPORTED by that actuator rather than papered over. This is the sgl-project#722 direction the retracted floor-clamp remedy would have broken; TheLevellingMustNotCapBelowTheLiveSet and test_floor_local_cap_812 were run explicitly and are green (20 passed). `_enforce_exposure_at_seam` returns rows withdrawn so a caller or test asserts on the ACTION, not on the absence of a symptom, and never raises: a seam is mid-flight, and enforcement that kills a cutover is worse than the exposure it corrects. It refuses; it does not explode. WHAT THIS DOES NOT CLOSE -- recorded in docs/dev/NOTE_851_build_caveats.md so it cannot be miscited. `floor > cap` SURVIVES this fix: with exposed == committed == 126976, max_live can still sit at the cap and the floor is max_live + 1 + margin + reserve = 131073 BY DESIGN, since the admission reserve is deliberately above the high-water mark. F1 converts a permanent SILENT veto into an explicit grow requirement at the seam; F2 is what makes that grow fundable or refuses it at boot. The exposure/veto falsifier therefore stays xfail until F1+F2 are BOTH in -- it is the composite's acceptance, not F1's, and it must not be "flipped" by teaching the reduction to drop an under-backed rank's floor (that takes the rank below its own live set = cudaErrorIllegalAddress; the reasoning is in the note). Also recorded there: F4 is ATTRIBUTION, not payout, and must never be cited as closing sgl-project#813 on its own. Tests: 5 new (W22's numbers corrected at the seam; a sound id space untouched; backing never lowered and exposure never RAISED; a missing rung survivable; a raising rung cannot kill a cutover). 1057 passed / 1 xfailed / 0 failures across the funding, relief, corridor, floor, backing, ownership, arming, spill, exposure and levelling suites. ruff clean. Hermetic, CVD="", no boots.

…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

…nd a gate that can say "checked" Two desk items from W36. 1. THE 22-ROW DOUBLE-COUNT. W36 aborted with the pool checker reading available OVER total by exactly 22: total=468981 available=108565 evictable=1 withheld=360437 -> 469003 The census names it one line earlier: size=468981 free=108544 withheld=360437 available=108566 `withheld + free == size` EXACTLY -- the id space is fully owned -- while `available` is `free + 22`, because `available_size()` is `len(free_pages) + len(release_pages)` and 22 ids had re-entered the RELEASE buffer while `_withheld` still counted them. Two owners, one row. MY W36 WRITE-UP CALLED `_withheld` A RANGE. It is not: it is an explicit id TENSOR, and `_apply` already scans both free lists. So the defect is not range-vs-buffer arithmetic but a re-entry window -- a high id freed between two `_apply` calls lands in `release_pages` while still withheld (the "allocator has no free listener" path warns about exactly that and then proceeds). Corrected here rather than left standing. WHICH OWNER WINS, by reading: the ids are ABOVE THE CAP and `_apply`'s own docstring is "move ids above the cap out of every free list". The withhold is authoritative; the free list is stale. `_settle_free_lists` drops them from the free side at `_publish` -- the single point where the withheld count is published, so the number the checker reads and the lists `available_size()` reads are reconciled in ONE place. Counted and named. PRIOR-ART GATE: `kv_row_ownership.read_free_rows` is the existing one authority for "free rows in the allocator's own shape" and already unions free_pages|release_pages, so the census side was correct and nothing new was built there. THE CHECKER IS NOT SOFTENED. It has now fired correctly in BOTH directions -- one row SHORT in W29, twenty-two OVER in W36 -- and both signs are pinned as tests. Both being right is what proves it measures. 2. RUNG 3 OBSERVABILITY. Every stale-generation gate logged only on REFUSAL, so W36's eight cutovers with zero refusals were byte-identical to eight cutovers whose gates were never reached, and the rung was INCONCLUSIVE -- an ambiguity created by the very lines meant to detect it. The gates now count CHECKS as well as refusals and the SEAM prints "checked N refused M" once per cutover. Emitted from the seam because the seam always runs: an unreachable gate now produces `checked=0` instead of silence. That is the can-fail, and it is a test. TESTS (12): the specimen arithmetic red-first; one-row-one-owner after settling; no-overlap left untouched; the reclaim counted; the checker still aborting on BOTH W29's under-count and W36's over-count and still able to pass a balanced pool; heartbeat resets per epoch; unreachable gate reads checked=0; and pins that the seam emits it and that both gates count checks. Tier-1 scoped gate: 103 passed (touched modules + consumers + this round's specimen tests) and 100 passed across the five kv_backing_relief consumer suites.

H1. W37-A booted my W36 census fix and the abort reproduced BYTE-IDENTICALLY while `ONE-OWNER` logged ZERO times: the invariant I enforced (`withheld ∩ free-lists = 0`) already held. My own fix, falsified by the checker I refused to soften. The log names the real pair arithmetically: census free=108544 (read_free_rows: a set UNION of both lists) alloc available=108566 (available_size(): a SUM of both lists) A union 22 smaller than the sum is 22 ids present in BOTH lists -- and `free + withheld == size` already held exactly. So the violated invariant is `free_pages ∩ release_pages = 0`, an overlap INTERNAL to the allocator's own two free lists, not between the withhold and the free side. PRODUCER, found by checking every insertion site rather than only the first suspect. `release_pages` is written in four places: the `need_sort` free path (token.py:73), `merge_and_sort_free` (base.py:159, which DRAINS it into free_pages), state restore (base.py:197), and `KvRowCap.release()`. The last one cats the whole withheld set into the FIRST of ("free_pages", "release_pages") and then BREAKS -- so any withheld id still present in the OTHER list lands in both. `_apply` compounds it: it scans both lists and its `torch.unique` belt was removed on purpose, so a pre-existing overlap is banked TWICE in `_withheld` and restored twice. TWO ONE-OWNER GUARANTEES, both at points this class already owns: * `release()` clears the withheld ids from BOTH lists before restoring, so the restore cannot double-insert; * `_publish()` drops any remaining `free ∩ release` duplicate, from `release_pages` -- the TRANSIENT owner, since `merge_and_sort_free` exists to fold it into `free_pages`, which is durable. Removing the transient copy cannot lose a row. The W36 withheld-vs-free settle is KEPT: it held here, but it is the same one-owner rule for a different pair and both are cheap at the same point. NOT FIXED INSIDE `available_size()`, and that is pinned by a test. Deduping the SUM would make the census and the allocator agree by construction and blind the one instrument that has now caught three distinct defects -- W29's one-row shortfall, W36's 22-row excess, and W37-A's proof that my repair was wrong. The checker stays untouched. TESTS (8 new, 20 in the file): the specimen arithmetic red-first (union vs sum, delta == the double-inserted ids); overlap removed at publish; available_size() then equals the union; the durable owner keeps the row; a disjoint pair untouched; the drop counted by name; restore does not double-insert; and the pin that available_size() contains no unique/set dedupe. Tier-1 scoped gate: 142 passed across the touched module's five consumer suites plus this round's specimen tests. ruff 0.

…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).

…se that could not fail is gone BOOT 17 AND BOOT 18 DIED OF THIS, at the first genuine idle, rank-uniform: `ValueError: pool memory leak detected! [full]` from `on_idle`. The admission wedge that came with it was a symptom -- 99 ADMITs still happened in the same window -- and the timeout that ended the load driver did not cause the death, it EXPOSED it by producing the first idle the boot ever had. THE CHAIN, MEASURED, five boots and no counter-instance (loadback-genuine / orphan-events / max-unaccounted): 1043b 0/0/0, 1043c 0/0/0, 1046cut 3/18/54626, 1048fix 69/21/43803, 1049n9 30/6/21608. Zero load-backs -> zero orphans -> zero ratchet. The orphan sizes are LITERALLY the load-back prefix lengths (5834, 4618, 350), rank-uniform on PP0/PP1/PP2, and the last census `unaccounted=` before death is identical to the size of the killer's `leaked_full_pages` set (21608 = 21608, contiguous 1..21608). An identity, not a plausibility. So this is not a regression of the load-back. It is sgl-project#938 -- registered open since 2026-08-27, Stage A (instrument) shipped as 226d34e, Stage B never built -- made FATAL because the load-back finally works: whole 4618-token prefixes are locked at the drop where the register's pre-registered fork expected ~283 rows. That fork is hereby answered from metal, 16x larger. WHAT CHANGES * `UnifiedRadixCache.reclaim_rows_for_drop` returns every row the tree still holds, locked ones included, and `drop_prefix_tree_returning_rows` calls it before `reset()`. `evict` refuses locked nodes and `_reset_full` zeroes the protected book without freeing a row, so those rows were leaving with nobody owning them, one drop's worth per flip cycle. * THE sgl-project#938 PREMISE IS NOW CHECKED, NOT ASSUMED. sgl-project#938 refused to free because a lock here is supposedly an in-flight write-through, and freeing under a live reader is a use-after-free in the sgl-project#913 family. Correct -- so the reclaim runs ONLY when `ongoing_write_through` is empty. On boot_855_1049n9 all 13 `sgl-project#792 post-retract writeback fence` lines report `outstanding=0`, including the two drops that orphaned 5834 and 4618 rows: the in-flight explanation held on no drop of that boot. When a copy IS outstanding the reclaim refuses and prints the count -- today's behaviour, kept. * NEVER FREES BLIND. Candidates are differenced against the allocator's own enumerated free set (`read_free_rows`, the union the idle invariant and the sgl-project#822 census already share). A non-enumerable reading refuses outright: a watermark allocator can say HOW MANY rows are free and not WHICH, and freeing against a count would be inventing membership. Double-freeing a row is silent corruption -- the one outcome worse than the leak. * The free is addressed through `FullComponent._free_full`, which resolves the allocator per access (sgl-project#941): a captured binding lands the rows on the other phase's free list as duplicates, the same symptom with a different root. * THE DEFERRAL IS INSTRUMENTED UNCONDITIONALLY. `sgl-project#1050 CUTOVER ROW RECLAIM` prints reclaimed/full_rows/mamba_slots/full_held/mamba_held/already_free and the refusal reason on every drop, including the all-zero reading. A clearer that can silently never run is how the same ratchet comes back one level up in a new pocket. * sgl-project#919's EXCUSE COULD NOT FAIL, AND IS GONE. The audit reported "pp_stack_allocator owns ids [1, 578995) and covers every sampled row" as the explanation for 43803 unowned rows, against a census id space of 578994 -- a containment test true for every sample that could ever be drawn. It excused a real, monotone, ultimately fatal loss. `OwnerCandidate` now carries `discriminating`, set false when the range is not narrower than the census's own, and such a candidate can no longer close the question: the new `SECOND-POOL-SPANS-EVERYTHING` verdict says the block is NOT excused. CHECKS (matched to this edit's failure classes, not a battery) * devtools/check_1050_cutover_reclaim.py, 17 cases, all pass: the double-free guard (an already-free row is not re-freed, and the count is reported), the outstanding-write-through refusal with its number, the non-enumerable refusal, root never harvested, and the no-mamba lane. py_compile is structurally blind to all of these -- `component_data` is a LIST indexed by the ComponentType enum, not a dict, and a `.get` there would have raised on every drop, on metal, mid-flip. * devtools/unaccounted_watchman.py --selftest, 4 cases, all pass, INCLUDING a replay of the real boot-18 log. That case earned its place: the first version keyed on the rank alone, passed its synthetic staircase, and read SILENT on the very boot it was written for, because the census emits four readings per cycle and the PP-layout zeros reset the streak. Keyed on the census family it fires. A watchman validated only against its own fixture is not validated. * ruff clean, py_compile clean on all three modules. NOT YET BOOTED -- desk-proven only.

zhyncs added 2 commits July 30, 2024 18:25

feat: add pr e2e test

695ec1a

update

ba547d7

zhyncs merged commit a30d5d7 into sgl-project:main Jul 30, 2024

zhyncs deleted the runner branch July 30, 2024 08:31

timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025

feat: add pr e2e test (sgl-project#822)

9b14ee7

Navigation Menu

Uh oh!

feat: add pr e2e test - #822

feat: add pr e2e test#822
zhyncs merged 2 commits into
sgl-project:mainfrom
zhyncs:runner

zhyncs commented Jul 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

zhyncs commented Jul 30, 2024

Motivation

Modification

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant