Fix max new tokens by merrymercy · Pull Request #772 · sgl-project/sglang

merrymercy · 2024-07-28T00:18:46Z

Fix the case of max_new_tokens=0 after #766

## Motivation  ## Modifications  ## Accuracy Tests  ## Benchmarking and Profiling  ## Checklist - [ ] Format your code according to the [Format code with pre-commit](https://docs.sglang.ai/developer_guide/contribution_guide.html#format-code-with-pre-commit). - [ ] Add unit tests according to the [Run and add unit tests](https://docs.sglang.ai/developer_guide/contribution_guide.html#run-and-add-unit-tests). - [ ] Update documentation according to [Write documentations](https://docs.sglang.ai/developer_guide/contribution_guide.html#write-documentations). - [ ] Provide accuracy and speed benchmark results according to [Test the accuracy](https://docs.sglang.ai/developer_guide/contribution_guide.html#test-the-accuracy) and [Benchmark the speed](https://docs.sglang.ai/developer_guide/contribution_guide.html#benchmark-the-speed).

THE CHECK WAS IN THE WRONG PLACE, NOT MISSING. write() already refuses at ENQUEUE via the sgl-project#718 device-tier disarm. The copy happens later, and the flip rebinds in between, so a write-back queued before a cutover reaches backup_from_device_all_layer carrying a pointer table into the pool it was built from. Both crash specimens died three seconds AFTER a pp_to_tp cutover completed: 14:08:14 -> 14:08:17 (epoch 27) and 07:12:09 -> 07:12:12 (epoch 3), seven hours apart, same direction, same lag. WHY THE SHAPE GUARD COULD NEVER CATCH IT, and why its silence was misread as innocence -- by me, until the operator pushed back. Under layer_first the host layout EQUALS the device layout, so a stale binding is shape-IDENTICAL to the live one and check_shapes passes by construction. That is exactly what sgl-project#760 recorded: KV-TRANSFER-GUARD armed on all three ranks, zero transfers refused, SIGSEGV anyway. Matching shapes plus a crash puts the fault below the Python seam; a generation stamp is what tells "same shape" from "same pool". MEASURED, ONE VARIABLE. HiCache host+disk under sustained 4-way load with the flip REMOVED (plain TP3, which keeps speculation on -- PP+spec requires the flip, server_args.py:18385) survives five minutes with 0 segfaults and 0 admission wedges, and passes the REP gate outright (1 distinct/12, salted 0/6). The fault needs the cutover. STAMPED BY CONSTRUCTION, VERIFIED AT CONSUME. The stamp lives in CacheOperation.__init__ rather than at one enqueue site: an op built by any other path would otherwise be unstamped, and an unstamped op must be refused, which silently dropped legitimate write-backs and broke the staged-dispatch tests until I moved it. A stale op is dropped loudly and counted; its prefix simply misses later, the same cheap failure the sgl-project#718 disarm already accepts. Also folded in, both consequences of earlier sgl-project#767 work rather than new choices: the anchor-protection test that encoded "no interval means no anchors" is updated to the corrected premise it was measured against, and the SECOND-PASS EVICTION line drops from warning to info -- the second pass is documented as legitimate, and anchor eviction was falsified as the drift cause, so it is accounting rather than an alarm. 4 new tests, red-first (a pre-rebind stamp must be refused). mem_cache and managers suites: 4047 passed, 43 failed -- all 43 the pre-existing sgl-project#772 class (PhasePolicyConfig lacks idle_locked_settle_s after the sgl-project#713 revert), one fewer than the 44 baseline because the 747 premise test is now correct. ruff clean on both touched files (0 at HEAD, 0 now).

…at outlive their phase TWO HOLES, ONE WINDOW. The sgl-project#718 guard read the parallel_state routing global, which is toggled INSIDE the cutover -- one step among many -- so for the whole seam (waves moving KV rows, movers releasing the outgoing backing, the cutover rebuilding topology) the guard named a phase while pool bytes were in motion. And even a perfect predicate cannot recall a copy that is already riding the controller's private CUDA streams: write() and start_writing() run in the same Python instant (start_writing is called synchronously from write, its only caller), so the previous commit's consume-time re-check re-asks the question at the same moment it was first asked. The torn window is the STREAM's asynchrony, not the queue's: a device->host copy enqueued legitimately in PP outlives its Python call by seconds under load, and the seam releases the pool under it. Both crash specimens died exactly there -- 3 s after a pp_to_tp cutover, inside backup_from_device_all_layer, below the Python seam. THE FIX, in the two halves the window has: 1. AUTHORITY. PhaseFlipRuntime registers itself (weakly) as the phase guard's authority. Its _phase field is what the PHASE-FLIP DONE line reports -- truthful by the 3 s crash correlation -- and it alone knows the seam's extent: hicache_seam_active is raised at the no-return point (after the unanimous-abandon verdict, before the first wave) and cleared after the cutover installs the new phase, with a finally in the caller as insurance. During the seam the guard refuses device-tier I/O for EVERY binding. Outside it, the authority's phase wins over the routing global, and a disagreement logs the sgl-project#754-shape instrument line that settles whether that global was ever stale here. No authority registered (no flip runtime built) falls back to the routing global: non-flipping deployments stay byte-identical. 2. QUIESCE. At the same no-return point the runtime drains the controller's write_stream and load_stream while every pointer they hold still names live memory. Finishing those copies is correct (they become durable cache entries) and bounded (PCIe transfer of the backlog; this thread is the only device-tier producer, so nothing refills behind the drain; rank-local, so it cannot wedge the group). Ordered after the sgl-project#703 flip-writeback hook, whose staging copies are the largest legitimate producer of exactly such in-flight work. Tests: test_flip_seam_guard_760.py, 10 tests, red-first (all 10 fail on the parent commit: seam disarm and authority-wins are impossible there, quiesce/_quiesce_hicache do not exist). With the fix: 14/14 green including the sgl-project#760 stamp tests. Full mem_cache+managers sweep: 4057 passed, 43 failed -- the 43 are the pre-existing sgl-project#772 class (PhasePolicyConfig lacks idle_locked_settle_s), zero regressions, +10 passed vs the parent's 4047. ruff clean on the new code (the 13 E402 in cache_controller.py pre-exist on HEAD); codespell clean. Metal gate next: the 2-SIGSEGV repro arm must hold >=2 cutovers under 4-way load with 0 segfaults and logged seam refusals.

…-- the hole the crash went through THE METAL FALSIFIED THE PREVIOUS COMMIT'S SUFFICIENCY IN 40 SECONDS, and the faulthandler stack it produced is the whole finding. seamfix1 (52df0ff, repro arm, 4-way load) registered the authority on all three ranks, quiesced at two seams, completed pp_to_tp epoch 3 -- and segfaulted seconds later in the TP phase, through a stack the guards never see: unified_radix_cache.cache_finished_req -> insert -> _inc_hit_count -> write_backup -> hybrid_cache/hybrid_cache_controller.write -> start_writing -> backup_from_device_all_layer -> transfer_kv_direct This deployment's tree cache is UnifiedRadixCache driving HybridCacheController -- which INHERITS from HiCacheController (so the seam quiesce worked, it is inherited) but OVERRIDES write() and load() without the device_tier_disarmed checks the base methods carry. Every sgl-project#718/sgl-project#760 metal reading of 'zero disarm hits' on this stack was therefore vacuous: the guarded methods never executed; TP-phase inserts enqueued copies against the PP-bound pools unchecked, and one of them walked released backing. The overrides now ask the guard first, before any pool is touched, with the base contract: refuse -> return None -> the caller books a miss (write_backup returns 0, load-back returns False -- both verified None-tolerant). Tests: two red-first additions to test_flip_seam_guard_760.py pin both overrides (an alloc-must-not-run pool proves the guard runs FIRST); both fail on the parent commit, 16/16 green with the fix. Full mem_cache+managers sweep: 4059 passed, 43 failed -- the pre-existing sgl-project#772 class only, zero regressions. ruff and codespell clean. Metal gate re-run next on this commit: >=2 cutovers under 4-way load, 0 SIGSEGV, and the first genuinely non-zero disarm/refusal counters on this stack.

…s at the flip, write state bytes in the computing pool The sgl-project#767 round-2 residual (NOTE_735: 'the fourth cached hit degenerates and stays degenerate') rooted to two wrong-pool defects in the mamba checkpoint path of a phase-flip build, found via two new metal discriminators on boot 735_seamfix2 (39 epochs standing): - salted probes with a UNIQUE first token: 0/8 degenerate, serial; - salted probes sharing one lead token: ~10/12 degenerate, including one answered with a verbatim FOREIGN topic (the load run's river essay) that survived /flush_cache; - concurrent unique-lead probes: 2-3/8 degenerate with prompt-level bleed. So corruption enters exclusively where a prefix hit restores a mamba checkpoint, and the payload is stale pool bytes: 1. resident_mamba_slots() moved only live requests' slots at a cutover while the KV leg unions the radix tree's values. Every tree-held checkpoint kept its slot NUMBER across the flip while its CONTENT was never translated into the new layout's pool; the prefix-hit COW (model_runner._maybe_execute_deferred_mamba_cow_and_clear) then seeded fresh requests from whatever the active pool held at that slot. flip_mamba_slots() now unions the tree's mamba values, and refuses a tree that cannot enumerate them. 2. The radix caches' checkpoint copies (donate-copy in cache_unfinished, int8 store_from_active) ran through the tree's bound pool -- the primary PP stack's, forever -- while a TP-phase request's real state bytes live in the TP stack's pool. The copy therefore duplicated the PP tensor's stale previous occupant into the checkpoint even with no flip during the request's lifetime. State-byte operations now resolve the computing stack's pool (mamba_state_pool.active_mamba_state_pool, installed by build_gdn_flip_mover); slot bookkeeping stays single-authority on the bound pool. Non-flip builds have no resolver and keep the old path bit for bit. Excluded by prior metal falsifiers and this trace: the sgl-project#767 slot-carry branch (0 carry-without-copy logged, force-clear changed nothing), allocator divergence (single authority by construction), fresh-no-match prefill state reads (has_initial_states = extend_prefix_lens > 0), ping-pong track path (no_buffer on this shape), ReplaySSM (refused by gdn_flip_preconditions). Tests: test_gdn_flip_tree_slots_767.py (red-first: ImportError then 5 passed), test_mamba_checkpoint_active_pool_767.py (7 passed). mem_cache+managers suites: 4071 passed, 43 failed -- all the pre-existing sgl-project#772 PhasePolicyConfig class, plus the known env-dependent test_localslot_family_756 retry failure (fails identically on clean HEAD). Metal validation on the rebooted arm follows in the boot note.

… loop it drives 7 failed / 7 passed -> 14 passed, exit 0. Completes the WIP commit. `_event_loop_pp_body` is taken unbound off the mixin here, so every link the body grew since sgl-project#791 had to exist on `_Rank`, and each one only became visible once the one above it was closed -- eleven in all across the two commits. The stub/bind split held throughout and is the part worth keeping: a shipped method is BOUND whenever it can answer without a peer, and STUBBED only where it would block on a real PP wire this fixture has no peer for, or dereference the None the link above returns. Every stub returns the shape the shipped function returns on its own no-op path, with that line quoted at the callsite, so none of them encodes a behaviour the production code does not have. The runtime flags (_pp_pass_voided_incoming, _pp_output_expected_incoming, _pp_upstream_launched_incoming, _pp_admission_pass_voided, _pp_gapped_wire) are set to the values a rank that received nothing carries, not to whatever made the next traceback go away. Standing observation, for whoever owns sgl-project#631: a harness that must re-declare every method its subject grows will break exactly like this again. Eleven links in one release is the signal, not the eleven fixes.

…t#816, sgl-project#810, sgl-project#806, sgl-project#797, sgl-project#790/sgl-project#777, sgl-project#817/sgl-project#820, sgl-project#818 Merge-checklist duty for the nine stages on this branch. Entries follow each section's own house style, and the mechanism text is taken from the commits' measured evidence rather than restated from the ticket titles. §3 KV backing relief + the allocator cap -- UPDATED IN PLACE rather than given a second bullet, because sgl-project#814 and sgl-project#816 are follow-on defects OF the KvRowCap mechanism that bullet already describes: the census reading the withheld block as a leak (340262 of 465190 ids), the lift being reachable only from a cutover (one boot at 26.8% of its id space for the life of the process), and exposure exceeding the backing (417850 rows over 105413 committed, the device-side assert in masked_set_kv_buffer_kernel). §3 HiCache staging write-through ring (sgl-project#810), new bullet, plus its two companion refusals -- the unbounded-file-tier refusal and the boot preflight ledger entry, the latter being why 22.01 GB of MHATokenToKVPoolHost across three PP ranks previously reached the preflight as nothing. §7 BAR1 deadline + loud abort -- appended the sgl-project#818 peer-liveness half to the existing narrative: the gate could wait forever on a peer that no longer exists, and neither Bar1CollectiveStalled (reset by every resolved read) nor defer_stall_for_building_peer (900 s off a build marker) caught it. §12 Robustness canon -- three new families: contradictory-flag (sgl-project#806), read-back-after-construction (sgl-project#797), denylist-of-reasons (sgl-project#817, sgl-project#820). §18.3 hicache staging sizing (sgl-project#810) -- §18's own rule is that a merge adding a reusable module adds its entry in the SAME merge, and this module had none. Records the removal of fits_pinned_host_budget so it is not reintroduced. §18.6 mamba carry instrument (sgl-project#767, gated by sgl-project#790) and flip break-even N (sgl-project#777). No existing entry was contradicted. Checked before writing: none of sgl-project#814, sgl-project#810, sgl-project#806, sgl-project#772, sgl-project#797, sgl-project#790, sgl-project#777, sgl-project#817, sgl-project#820, sgl-project#818 had a catalog entry, and the one sgl-project#677 line (§19.2, RESTORE-NEVER-REBUILD) describes a different mechanism than the sgl-project#677 layout hold, so it is not stale and was left alone. Gates for the tree this documents (hermetic, CUDA_VISIBLE_DEVICES=""): battery test/registered/unit/{managers,planner,server_args,mem_cache} baseline integ @ 78d27da 44 failed c56d238 (through sgl-project#818) 30 failed, 8452 passed 0 new ids, 14 fixed 1c4eadb (through sgl-project#816) 30 failed, 8461 passed 0 new ids, same set test_barlink_abort_gate_liveness_818.py (outside the battery dirs) 10 passed ruff --select=F401,F821,UP037 and codespell: 0 new findings vs the same file set on 78d27da (16 ruff / 6 codespell exist identically on base) docs-only change; codespell clean on the catalog itself

merrymercy added 2 commits July 27, 2024 17:18

fix max new tokens

df3149a

fix max new tokens

c167b40

merrymercy merged commit a036d41 into main Jul 28, 2024

merrymercy deleted the fix-max-new-tokens branch July 28, 2024 00:22

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

Fix max new tokens (sgl-project#772)

6157b0d

Navigation Menu

Uh oh!

Fix max new tokens - #772

Fix max new tokens#772
merrymercy merged 2 commits into
mainfrom
fix-max-new-tokens

merrymercy commented Jul 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

merrymercy commented Jul 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant