Remove inf value for chunked prefill size by hnyls2002 · Pull Request #812 · sgl-project/sglang

hnyls2002 · 2024-07-30T00:57:04Z

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

Some runtime implementation varies according to whether the chunked prefill feature is enabled or not. We do not have a global fixed INF value, using 1<<30 may cause judging mistakes.

Modification

When chunked prefill is disabled, the chunked_prefill_size is set to None.

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.

* fix test_skip_tokenizer_init.py * Update run_suite.py * Update test_skip_tokenizer_init.py * Update pr-test-npu-debug.yml * Update test_skip_tokenizer_init.py * Update test_utils.py

… cutover `recover_kv_backing` is the only thing that lifts `KvRowCap`, and its only two call sites were post-cutover hooks (phase_flip_runtime.py:1818 and :1835). That is fine while flips commit, and a trap the moment one does not -- a trap that closes on itself: 1. a corridor-bounded recovery leaves the ranks unequal (measured on this rig, one boot: 210944 / 124928 / 131072 backed rows); 2. the cap agreement levels the group to the poorest -- correct, since under pure PP an id a peer cannot map aborts all three ranks inside store_kvcache's bounds assert (phase_flip_boot.py:1264-1278) -- so the allocator is capped at 124928 of 465190, 26.8% of the id space; 3. the capped pool fills, so the group's live floor is 100% of what is left; 4. the next tp_to_pp flip is DECLINED by that peer floor (sgl-project#812); 5. no cutover, so no recovery, so the cap is never lifted -- back to 3. Evidence that step 5 is what actually happened, from the log rather than from the code: that boot has 27 POOL CENSUS lines = 3 ranks x 9 events, and NOT ONE is `post-cutover tp_to_pp`. All six return attempts stop at `at-arm`. `recovered to N of M rows` appears 0 times, `recovery deferred` 0 times, `recovery failed` 0 times. The pool sat at 26.8% for the life of the process and a user got an overloaded_error at 21:02:58 against a pool sized 3.7x larger. So sgl-project#812 and sgl-project#813 are consequences of the cap, not its cause. Both docstrings already named this outcome. `recover`: "an admission-capacity loss, which is recoverable ON ANY LATER LEG" -- there was no later leg. `recover_kv_backing`: "a cap that is never lifted turns dynamic residency into a permanently smaller pool, which is the one fix the standing rule forbids." THE FIX IS A LEG, NOT A NEW POLICY. The cap agreement is left exactly as it is: `cap_proposal` is STRICTLY NON-ALLOCATING on purpose and records why -- the first metal boot of a growing agreement hit "cuMemCreate failed: CUDA_ERROR_OUT_OF_MEMORY" on all three ranks, rank 0 driven to 3 MiB free (kv_backing_relief.py:2710-2745, 2026-08-13). That door stays shut. The design already assigns growing to exactly one owner, `recover`; it simply had nowhere to run. This adds the missing call at the abandon exit, where nothing moved and no seam is owed the memory. Safety, and none of it is new: `recover`'s bound is intrinsic to the function (kv_backing_relief.py:2503-2505) -- a fresh per-rank mem_get_info at call time minus this card's own corridor law, deferring without committing a byte when `rows <= was`. A new call site inherits exactly what the two post-cutover sites rely on. The grow stays rank-local and corridor-bounded; the levelling stays collective and non-allocating. Collective safety: the helper enters a collective, so it may only be called where every rank arrives. Its one call site is the group-unanimous abandon exit in `_execute`, downstream of `reduced_fit = self._collective_min(payload)` (phase_flip_runtime.py:7381) -- a MIN reduction bit-identical on every rank, with no `return` and no `raise` anywhere between it and the exit (checked by reading every statement in that window). Without a channel the helper is a no-op, matching `recover_kv_backing` on single-rank shapes. It never raises: an abandon is already the unhappy path and must not become a dead rank. Tests: test/registered/unit/managers/test_recover_on_abandon_814.py, new, 5 cases, hermetic (CUDA_VISIBLE_DEVICES=""). Three mutants, each killed by its own dedicated case: * single-rank guard removed -> test_without_a_channel_it_is_a_no_op * reduce_fn dropped on the delegate -> test_it_delegates_through_the_group_channel * logs on every abandon instead of only a real one -> test_a_zero_row_recovery_logs_nothing (abandons come in floods; 47 in a row were measured) Regression: 730 passed across the flip/kv-backing/seam/census/evict suites. 3 failures in test_evict_rung_floor_invariant_717.py ("'KvBackingRelief' object has no attribute '_buffers'") are PRE-EXISTING -- measured, not claimed: the identical three fail with the identical error on the base commit 587e4c2. ruff clean, black clean. Metal, D1 half of this branch (boot_814_arm2_0822_2146.log): the census now reads `withheld=330770 unaccounted=122` where it read `unaccounted=340384`, and 122 is the known irreducible mamba artefact. This commit is the half that returns the capacity rather than merely describing it; its metal acceptance follows on the next boot.

… floor from its own cap; solve the arming floor; name the break-even provenance Three ordered items on top of the funding authority. A) THE PEER VETO, AT ITS SOURCE (sgl-project#812 / kein-bindender-rang) The live defect is not _floor_rows, it is what happens to its output. In _floor_ppm, kv_backing_relief.py:167: if floor_rows >= current_rows: return _SHRINK_SCALE # 100% == "no shrink" Measured 06:32:05, all three ranks carrying the SAME floor 128549 -- correct, because under PP a request's tokens occupy KV on every stage, so the live set is genuinely replicated -- against caps that are unequal BY DESIGN: PP0 backed 212992 floor 128549 -> 60.4% of its own cap PP1 backed 124928 floor 128549 -> 102.9% <- the defect PP2 backed 133120 floor 128549 -> 96.6% PP1's 102.9% clamps to 100%, the group MAX takes it, explain_kv_target computes target = max(desire, max_floor), and NOBODY shrinks -- vetoing PP0's fully fundable 84443-row plan on the very rank that needed it. The sibling _shrink_ppm documents this exact trap for the ambition side ("on an uneven fleet the smallest pool's 'no change' is the smallest number in the group and silently wins"). The lesson was never applied to the floor side. Fix, local only -- the collective FORM is untouched, no new collective on the admission path (DESIGN_679's warning stands): _floor_rows now derives its result against this rank's own backed rows and clamps DOWN to them. An unbacked row cannot be reserved, so a floor above the cap was never a meaningful quantity. The condition is logged as an UNDER-BACKED RANK -- a defect about that rank's backing, whose answer is to grow it -- and floor_exceeds_local_cap() splits the two cases _floor_ppm used to collapse: floor == cap is a HEALTHY full pool, floor > cap is the defect. They returned the identical value before, which is how one rank's local problem became a group-wide freeze. HONEST SCOPE: the clamp makes the quantity truthful at the point it is computed and names the defect. It does NOT by itself unblock the veto, because a clamped floor still equals the cap and still returns the neutral element under a PROPORTIONAL agreement. Replacing that proportion is a change to what the group agrees on -- enforcement, and metal. Ticket in the COORD. B) DEFECT A: THE ARMING FLOOR IS THE FREE VARIABLE, THE BAND IS NOT band floor 819 + seam entry reserve 512 = 1331 arming floor + arming margin 192 = 1523 band ceiling 1229 <- 294 MiB short The corridor band (819-1229 free per card under load) is a hard user rule: below it is a breach, above it boot acceptance has FAILED. It does not move. The arming floor is DERIVED and the seam entry reserve is an allowance, so the floor is what must give. solve_arming_floor() reports the largest reserve that fits -- 218 MiB against the 512 shipped -- and refuses BY NAME otherwise. It does not auto-correct: cutting the reserve changes what the seam may spend while it runs, which needs metal. Also withdraws a piece of advice that named a forbidden state. The abandon message said the flip "is retried when occupancy drops"; clearing a 1331 MiB watermark means leaving the acceptance band from ABOVE, so no amount of waiting can end it -- and 18f measured exactly that (draining the load did not lift the lock). _arming_floor_advice() now retracts the retry advice whenever the watermark is unreachable. C) sgl-project#819: 7004 IS NOT A SEED, AND THE STALENESS IS UNEVEN 7004 is a literal nowhere; break_even_tokens computes N = C / (1/X - 1/P) and 7004 is what the shipped inputs produce (asserted in the suite). The three inputs do NOT share a provenance, which is the actual finding: C flip cost SELF-CORRECTING -- FlipCostEstimator.observe() is fed real cutover durations, seeded from DEFAULT_FLIP_COST_S=3.2 X TP prefill 1681.0, env-overridable, NEVER measured at runtime P PP prefill 7245.5, same shape One input self-corrects and two cannot. A rig whose prefill ladder differs from the sgl-project#631 mainrig silently solves N against another machine's hardware unless a human sets two env vars, and nothing tells them to. BreakEvenProvenance makes that visible instead of implicit. TEST RESULTS (hermetic, CUDA_VISIBLE_DEVICES="") test/srt/test_funding_authority_770.py 48 passed (was 40) test/srt/test_floor_local_cap_812.py 16 passed (new) test/srt/mutants_770.sh 11/11 KILLED (M9/M10/M11 new) test/srt/mutants_812.sh 4/4 KILLED ruff --select=F401,F821,UP037 3 findings on phase_flip_runtime.py, identical to base (2090, 2581, 7022->7137 shifted by inserted lines) -- ZERO new codespell clean on all new files TWO MUTANTS SURVIVED FIRST CONTACT AND BOTH WERE REAL. MA3 (clamp rounds UP past the cap) survived because the fixture used cap 124928 with page 64 -- exactly divisible, so round-up and round-down agree and the test could not tell them apart. Refixtured on 124900. MA5 (drop the min() guard in _floor_ppm) is an EQUIVALENT mutant, proven not patched: the early return means the expression is only reached with floor < current, where ceil(floor*1e6/current) <= 999999 < _SHRINK_SCALE. Verified numerically; the guard is kept as defence-in-depth and the mutant is recorded in the harness rather than run, because a harness listing an unkillable mutant reports a permanent false gap.

… a correctness invariant for a funding win THE PREVIOUS COMMIT SHIPPED A REGRESSION AND THE TREE CAUGHT IT. _floor_rows clamped the floor DOWN to this rank's backed rows whenever the floor exceeded them. It looked like the ordered fix -- "derive the per-rank floor from the per-rank cap; a floor >100% of its own cap is a defect by construction" -- and the arithmetic is indeed a defect. The clamp is not its remedy. The floor IS `live set + 1 + margin + admission reserve`. Lowering it to a smaller cap does not make an under-backed rank able to shrink; it authorises a cap BELOW rows that are still in use. Measured, not argued: test_residency_cap_flip_levelling_792 ::TheLevellingMustNotCapBelowTheLiveSet ::test_the_recovery_levelling_leaves_every_rank_able_to_pay with the clamp 1 failed without the clamp 12 passed The invariant is older than this ticket and the tree already carried it. My ID-level regression is what surfaced it, and it is exactly the danger direction I had written into my own mutant rationale one commit earlier -- and then implemented anyway. WHAT STAYS. The detection: floor_exceeds_local_cap() splits the two cases _floor_ppm collapsed -- floor == cap is a HEALTHY full pool, floor > cap is an under-backed rank -- and _floor_rows now LOGS that condition by name instead of letting it vanish into a group-wide freeze. The floor itself is untouched. WHAT THIS MEANS FOR THE VETO. It is not fixed here, and the honest statement is that it cannot be fixed locally at all. An under-backed rank genuinely has nothing to give; the repair is on the BACKING side (grow it) or in the agreement shape (stop agreeing a proportion of each rank's own cap). Both are enforcement and both need metal. Ticket with acceptance criteria in the COORD. MUTANT MA1 IS NOW THE CLAMP ITSELF, so the withdrawn version cannot creep back: reintroducing it turns the suite red. Added test_the_floor_never_drops_below_the_live_set so this file cannot drift toward it either. Also ran black over the touched files (the sgl-project#656 formatting ratchet). The ratchet's own two failures are PRE-EXISTING and not mine: the files it flags are phase_flip_boot.py, phase_flip_seam_reserve.py, kv_vmm_backing.py and two test files, none of which this branch touches. TEST RESULTS (hermetic, CUDA_VISIBLE_DEVICES="") test_funding_authority_770.py + test_floor_local_cap_812.py + test_residency_cap_flip_levelling_792.py 76 passed test/srt/mutants_770.sh 11/11 KILLED test/srt/mutants_812.sh 4/4 KILLED (+1 equivalent) ruff --select=F401,F821,UP037 All checks passed codespell clean PRE-EXISTING FAILURES CONFIRMED NOT MINE, each by running the same file against the base module 481c732: test_evict_rung_floor_invariant_717.py 3 failed on base AND on mine (AttributeError '_buffers' -- the sgl-project#717 stubs, unrelated to this branch) test_r1_private_constant_gate_584.py flags VERDICT_BAR_MIB, NEAR_OOM_MIB, BYTES_PER_GB, MIN_STAGING_GB -- all in files this branch never touches test_scheduler_chunked_req_gate.py 3, unrelated test_pp_proxy_cross_epoch_mispair_795.py 1, unrelated

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

Wave 3, stage 4 of the batched-window tree. WINDOW-QUEUE ticket W6 (strand 21a, preflight_pass=Y: 19 passed red-first with 6F/6P before the fix, 8/8 mutants killed, id-regression 75 files byte-identical with 0 new). Two commits, base 3b2bbde -- which is already ON this line (the sgl-project#770/sgl-project#812 floor-clamp withdrawal), so this stage adds no divergence of its own: b546893 [sgl-project#828] The backing dial converges the BACKING, and a post is credited by what it delivered 4235879 [sgl-project#828] Desk pre-flight for the batched window: the released band clears the gate boot_827 refused at THE DEFECT. `runtime_set_backing_tokens` branched grow-vs-shrink on `self.size` -- the EXPOSED id space -- instead of on the committed backing, so a rung that should have released reported `branch=grow` and released 0. A dial that converges the wrong quantity is not a mis-tuned dial; it is a dial attached to the wrong shaft, and the census downstream then credits a post for delivering nothing. This is the same family the catalog records in section 2 under the funding authority: a post is credited BY WHAT IT DELIVERED, not by what it was asked for. sgl-project#770 named the posts; this makes the credit honest. GATE. Battery test/registered/unit/{managers,planner,server_args,mem_cache}, hermetic under CUDA_VISIBLE_DEVICES="", one battery at a time. baseline (tip 4f2072a) 7 failed, 8585 passed, 1852 skipped, 887 s stage 1 (W1+W2) 7 failed, 8638 passed, 1852 skipped, 868 s stage 2 (W3) 7 failed, 8653 passed, 1852 skipped, 922 s stage 3 (W4a/W5/W4b) 7 failed, 8673 passed, 1852 skipped, 915 s stage 3b (sgl-project#791 stub) 0 failed, 8684 passed, 1852 skipped, 900 s this stage 0 failed, 8703 passed, 1852 skipped, 931 s NEW failure ids NONE THIS IS THE FIRST STAGE GATED AGAINST A ZERO BASELINE. Since stage 3b the line's extracted failure list is EMPTY, so "no new failure ids" here is not a comparison against a tolerated set -- it is the whole result. CITATION MAINTENANCE, carried in this commit because this stage is what forced it. W6 adds 37 lines to funding_authority.py and 71 to memory_pool.py, both of which the catalog cites by file:line, and the section-18 checker did NOT notice: it verifies that a cited line EXISTS, not that the cited SYMBOL is on it, so it stayed at 104 passed while solve_arming_floor moved :659 -> :694, diagnose_floor_band :585 -> :620 and slack_above_uniform_floor :813 -> :848, each then pointing at a comment, an `if` and a dataclass field. A sweep of every citation this train touches found the same drift elsewhere and all of it is corrected here, verified line by line against the files: scheduler.py nine citations (stage 3 added 113 lines) :4684->:4795, :4792->:4903, :4814->:4925, :4938->:5049, :4947->:5058, :4995->:5106, :5011->:5122, :7085->:7196, :7462->:7573; phase_flip_runtime.py five, :3838/:3839->:4040/:4041, :3881->:4083, :6275->:6487, :7128->:7353; memory_pool.py :4978 -> :4983 with the bound assert at :5032. Untouched and re-verified as still correct: kv_row_ownership.py, prefetch_ballot.py, uniform_floor_scope.py, tree_congruence.py, invariant_checker.py:1175, mamba_ckpt_utils.py:185, kv_backing_relief.py:162/:547, phase_policy.py:831. One drift found in the sweep is NOT this train's doing and is labelled as such: planner/placement.py:813 -> :838, in a file no stage here touches. It drifted earlier and nothing caught it, which is the point. The gap is now recorded in section 18.8 with the measurement that proves it, and the interim practice it implies is applied rather than merely described: where a file takes inserts from several tickets, the STATEMENT is cited alongside the line (done for sgl-project#821's three cur_batch_for_debug sites). Symbol-resolving the checker is registered as its own task. A note on the checker being live rather than assumed, because it caught ME while I was documenting its blind spot: writing the phrase `watchdog.py:88` into the section-18.8 prose turned the gap note itself into a section-18 citation, and the checker went 104 passed / 1 FAILED on the spot. It scans section 18 for anything of the shape `file.py:N`, prose included. The wording now names the class instead of quoting a path, and the checker is back to 104 passed / 0 failed -- re-run after the edit, not assumed. So the checker is genuinely armed for the class it covers; the gap in 18.8 is about the class it does not. codespell clean. No boot.

remove inf value for chunked prefill size

dcb1116

hnyls2002 merged commit 3520f75 into main Jul 30, 2024

hnyls2002 deleted the update-chunked-prefill-size branch July 30, 2024 01:34

Ying1123 mentioned this pull request Jul 30, 2024

Development Roadmap (2024 Q3) #634

Closed

29 tasks

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

Remove inf value for chunked prefill size (sgl-project#812)

64c70bf

Navigation Menu

Uh oh!

Remove inf value for chunked prefill size - #812

Remove inf value for chunked prefill size#812
hnyls2002 merged 1 commit into
mainfrom
update-chunked-prefill-size

hnyls2002 commented Jul 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hnyls2002 commented Jul 30, 2024

Motivation

Modification

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant