Fix StreamExecutor.fork() losing the current role start index. by max99x · Pull Request #684 · sgl-project/sglang

max99x · 2024-07-21T05:34:31Z

Motivation

If you call fork() while the state is within a role, the forked program states will return the full text for the in-progress role from messages(). Example:

s += sglang.user('Parent ')
s += sglang.assistant_begin()
c = s.fork()[0]
c += 'Child'
c += sglang.assistant_end()
print(c.messages())

Previous wrong output:
[{'role': 'user', 'content': 'Parent '}, {'role': 'assistant', 'content': 'Parent Child'}]

New correct output:
[{'role': 'user', 'content': 'Parent '}, {'role': 'assistant', 'content': 'Child'}]

Modification

Copied cur_role_begin_pos from parent to forked executors when forking.

Checklist

This seems simple enough to not be worth a separate test, but let me know if you want me to add one.

…roject#684)

* Update run_suite.py * Update test_ascend_hybrid_attention_backend.py * Update test_ascend_llm_models_phi_4_multimodal.py * Update test_ascend_llm_models_smollm_1_7b.py * Update test_ascend_llm_models_ling.py * Update test_ascend_llm_models_ling.py * Update test_ascend_llm_models_phi_4_multimodal.py * Update test_ascend_llm_models_smollm_1_7b.py * Update run_suite.py * Update pr-test-npu-debug.yml * Update run_suite.py * Update pr-test-npu-debug.yml * Update test_ascend_weight_loader_disable_mmap.py * Update run_suite.py * Update test_ascend_llm_models_Stablelm_2_1_6b.py * Update run_suite.py * Update test_ascend_model_impl.py * Update run_suite.py * Update test_protocol.py * Update pr-test-npu-debug.yml * Update run_suite.py * Update test_protocol.py * Update run_suite.py * Update test_ascend_lora_backend.py * Update run_suite.py * Update pr-test-npu-debug.yml * Update test_ascend_lora_backend.py * Update test_ascend_lora_backend.py * Update test_serving_chat.py * Update run_suite.py

… unattributed WHAT THE GAP COST, MEASURED. On 2026-08-16 at 02:36:30 an instance died with GPU 0 ... 76.38 MiB is free ... Process 1920108 has 4.29 GiB memory in use Naming that process took hours: log archaeology, then a pid-clock interpolation across two boots' `boot_id` fields (pid 1851351 -> 02:10:44, pid 1924983 -> 02:38:10, 44.7 pids/s, so pid 1920108 lands at 02:36:22). It turned out to be a test harness on the serving card. Every fact needed to answer that in ONE LINE was already computed. `_nvml_view` returns `nvml_processes`, the full pid->bytes map of everyone on the card, on every mark. The recorder simply stops marking: its last boot post is `first_forward`, and the failure was 36 minutes later. NOT A DUPLICATE OF THE sgl-project#605 CORRIDOR SAMPLER, and the difference is durability. That sampler does run during serving, at 100 ms, and does call `_nvml_view`: * it keeps a fixed-size RAM ring, so it dies with the process that crashes -- which is the one process whose state the post-mortem needs; * its `Sample` retains free/self/reserved/allocated and DISCARDS the per-pid map it just read, so it cannot name a foreign holder even while running; * it was not armed on the boot that died. Marks are appended to a FILE and survive the crash. That is not a theoretical advantage: the surviving boot marks are exactly what made the pid clock calibratable after the process was gone. A SEPARATE FILE, DELIBERATELY. The boot ledger's readers pair marks BY POST NAME (`reconcile` asks for the `weights_loaded -> kv_pool_sized` delta, and for `kv_arena_backed_bytes` at `boot_complete`). A boot post is a unique boundary; a serving sample is a time series, and thousands of the latter in that file would turn a table of posts into a log with posts in it. So the series goes to `flight_serving_rank{n}.jsonl` and every existing consumer of `flight_marks_rank{n}.jsonl` is untouched -- 467 mem_ledger tests confirm it. ONE RECORD BUILDER, TWO DESTINATIONS. `mark` grew an internal `_filename` rather than gaining a second copy of the record layout. A duplicated schema is precisely how the field a post-mortem needs ends up present in one file and missing in the other -- which is the shape of the defect this commit exists to close. PACED, ON THE MONOTONIC CLOCK. The call site runs once per scheduler iteration, thousands of times a second; the pacer (default 30 s, `0` disables) is what makes that affordable, and the cost when the recorder is unarmed is one dict lookup. Wall time is unusable here: an NTP step backwards would stall the series and a step forwards would flood it, and the boot that most needs the record is the long-lived one whose clock is being corrected. WHERE IT IS CALLED, AND WHY THERE. Beside `_corridor_trace_tick` in `get_next_batch_to_run`, on that line's existing argument -- every rank reaches it exactly once per round -- so the cadence is replicated and the per-rank files line up round for round, which is what makes them comparable across ranks. Unlike its neighbours it needs no collective and takes no branch: it is write-only and cannot make two ranks disagree about anything. It is NOT on a batch-conditional path, because an idle rank losing its card to a foreign process is exactly the 2026-08-16 case. THE CALL-SITE TEST EARNED ITS PLACE IMMEDIATELY. The first version of the tick referenced `flight_recorder` as if it were a module-level import; it is imported inside `run_scheduler_process`. That is a `NameError` on every iteration, and the bare `except Exception` around it turned the whole instrument into one that silently never runs -- the exact failure mode this task exists to prevent, in the code meant to prevent it. `test_the_tick_marks_with_this_rank` failed and named it. The import now sits OUTSIDE the guard so it fails loudly, only the call is guarded, and the guard logs once at WARNING rather than staying silent. The recorder's own `BOOT_PHASES` comment states the lesson this file follows: "twelve green fixture tests passed while the production carrier lacked the field they all built by hand." TESTS. `test_flight_serving_marks_684`, 17 cases, hermetic (CPU only, no CUDA): - RED FIRST: all 13 API cases failed with `no attribute` before the change. - The acceptance property reproduces 02:36:30 exactly -- this process at 26.65 GiB and a foreign pid at 4.29 GiB on one card with 80 MiB free -- and asserts the foreign holder is named IN THE RECORD ON DISK. - Pacing, monotonic-clock behaviour, `0` disables, a malformed interval falls back rather than silencing the series, an unarmed process writes nothing. - The boot ledger keeps one mark per post while the series accumulates. - An NVML failure still leaves a timestamped record; a write failure and a raising probe never reach the serving loop. - Call site: taken off the REAL Scheduler class, so a rename fails this file; pins `self.ps.tp_rank` (the Scheduler has no `tp_rank` -- a predecessor's assumption that it did raised on every rank), and pins the position beside the corridor tick. Suites: 467 mem_ledger, plus 557 across mem_ledger + the 681/682 guard files.

… cap never lifts VERIFICATION ONLY -- NO FIX IN THIS COMMIT. The clamp is deliberately not built: the task's framing moved twice and the reproduction is what settles which repair is right. THE FRAMING WAS WRONG, AND THE STATIC READ IS WHAT SHOWED IT. sgl-project#684 was opened as extent fragmentation -- "191k rows of slack, zero releasable extents". It is not. `KvBackingRelief` releases by lowering backing "to just above the highest live row" (its own class docstring), a HIGH-WATER-MARK TAIL policy, and the log's `highest live row` field corroborates it: at row 122 a shrink released 1322 MiB, at row 234118 it released nothing worth having. Fragmentation was the wrong frame. WHAT IS ACTUALLY BROKEN, MEASURED AND UNCONFOUNDED. From 02:15:24 to 02:35:26 on 2026-08-16, 59 times, at a steady 3 per minute -- once per rank per flip leg: KV-BACKING recovery to 270646 rows failed: final_num_tokens=270646 must satisfy page_size=1 <= final <= reserved=190596. The cap stays engaged. 59 attempts, 59 failures, the target always ABOVE the pool's reservation (270646/190596, 180428/108912, 179466/136140). That window opens BEFORE any test-harness CUDA activity on the rig, so unlike the free-column readings from 02:29 onward these lines are not confounded by a competing process. WHY IT IS BIGGER THAN THE LINE. Recovery is what LIFTS the backing cap. While it fails the cap stays engaged, the pool stays shrunk, and every later `free_up_to` finds the backing already at or below its target and honestly claims 0 MiB -- reported through a message asserting "this pool cannot pay: the arena has no commit chunk", a mechanism the surrounding code itself knows may not apply. That is how the corridor guard's only escalation rung above `allocator-cache` stayed dead for a whole boot while its diagnostic pointed at the arena. It also explains the shape sgl-project#683 was opened on: with that rung dead, `[allocator-cache]` is the only provider that can ever appear. THE DEFECT IS A MISSING CLAMP. `recover` bounds its target two ways -- by `_rows_at_boot`, and by what the free column affords above the corridor law -- and by nothing else. The pool's reservation is never consulted; `reserved` is not read against a pool anywhere in the module. When the reservation is smaller than the boot row count the target is unsatisfiable BY CONSTRUCTION and every attempt fails identically, forever. `_rows_at_boot` is captured on the first shrink and parked per pool, so it is a number from an earlier state of a pool whose reservation has since moved. HERMETIC, NO GPU, AND THAT WAS THE POINT. The reproduction drives the real `KvBackingRelief` over an injected fake pool that refuses exactly as the production one does, with the production numbers. It needs no card, so it costs no gpu-arb window -- which matters, because a probe on live serving cards is what took the instance down at 02:36:30 in the first place. FOUR CASES, AND TWO OF THEM EXIST TO STOP THIS PINNING A FICTION: - the defect itself: recovery refused, 0 returned, watermark unmoved, every attempt above the reservation; - it fails IDENTICALLY however often it runs, and the target never moves -- the 59-of-59 shape; - CONTROL: the same path recovers normally when the reservation is large enough, so the test cannot be satisfied by a recovery broken for every input; - the bound that IS implemented still holds: an unaffordable grow defers and the pool is never asked. WHEN THE CLAMP LANDS, `test_recovery_is_refused_forever_because_nothing_clamps_it` INVERTS -- recovery returns bytes and the cap lifts. That inversion is the fix's acceptance test, already written. Suites: 120 across the four kv_backing files plus this one.

… what we remembered THE ROOT QUESTION IS ANSWERED, AND MY OWN CANDIDATE WAS WRONG. I had named the flip's "released 1410.0 MiB of weights-arena tail" as the reason the reservation ends up below `_rows_at_boot`. It is refuted twice over: that is the WEIGHTS arena, not the KV pool, and the KV reservation cannot move at all. memory_pool.py:2458 reserved_num_tokens=self.size # at construction kv_vmm_backing.py:979 self._reserved_num_tokens = int(...) # assigned ONCE The reservation is pinned to the pool's size at the moment the arena is built and never assigned again. `size` is NOT immutable -- the sgl-project#330 dial writes it on every step, which sgl-project#662-F4 already noted one layer up. So a grow target derived from a remembered or configured row count can sit above a ceiling that never moves, and `_check_final` refuses it identically, forever. MEASURED, AND UNCONFOUNDED: 59 times between 02:15:24 and 02:35:26 on 2026-08-16, a steady 3 per minute, once per rank per flip leg -- `recovery to 270646 rows failed: ... reserved=190596`, and the same shape on the other two ranks (180428/108912, 179466/136140). That window opens before any test-harness CUDA activity on the rig, so unlike the free-column readings from 02:29 onward it is not confounded. WHY IT IS BIGGER THAN THE LINE. Recovery is what LIFTS the backing cap. 59 refusals meant the cap never lifted, the pool stayed shrunk, and every later `free_up_to` found the backing already at its target and honestly claimed 0 MiB -- which the shrink path then reported as an exhausted ARENA. One unsatisfiable number, and the corridor guard's only rung above `allocator-cache` was dead for the whole boot while its diagnostic pointed somewhere else. That is the shape sgl-project#683 was opened on. THE REPAIR IS THE SAME CORRECTION AS sgl-project#681 AND sgl-project#682: validate against what the ACTUATOR can pay, not against the count that proposed it. sgl-project#681 was a token count against a leaf frontier, sgl-project#682 a guard ceiling against the bound the scheduler actually holds, and this is a grow target against an immutable reservation. So the clamp is deliberately NOT conditional on knowing why the remembered number went stale -- it asks the bound. CLAMP *AND* RE-DERIVE, because the clamp alone would only convert a loud failure into a quiet one: `_rows_at_boot` would still name an impossible level and every later recovery would re-clamp to the same place while believing it had further to go. Correcting it lets the existing "fully recovered" branch fire, which clears the remembered rows AND retires the exhaustion marker -- the latch that kept the rung off. RANK-LOCAL, EXPLICITLY, as the brief asks. A reservation is one card's VA span; under uneven TP the ranks hold different ones -- 190596 / 136140 / 108912 on this boot -- so there is no group quantity here to agree on. `recover` takes no collective, and this commit adds none. The module's collective, the sgl-project#656 C22 cap agreement, is on the SHRINK target and is untouched. The new accessor is also NOT `_reservation_rows` (the allocator's id space, which does feed `exposed_rows` and that agreement); the two are cross-referenced in code so a later reader cannot conflate them. SAFE DESK-SIDE, AND THE JUDGEMENT IS ASKED FOR, SO HERE IT IS. Two properties make this shippable without a GPU window: * the clamp fires ONLY where `rows > ceiling`, which is exactly the path that currently fails 100% of the time. On any boot where recovery works today the branch is inert, so there is no working behaviour for it to change. * it runs AFTER the corridor-affordability bound, so when both bite the target is the smaller of the two and the clamp can only LOWER it. Raising it would commit pages the corridor law had already refused -- the failure that drove rank 1 to 6 MiB free and OOMed inside relief. Pinned by `test_the_clamp_can_only_lower_the_target_never_raise_it`. A pool that exposes no reservation keeps its previous behaviour exactly; 0 is read as "no arena", never as a ceiling of zero, which would be a shrink wearing a grow's name. TESTS, red-first. The acceptance pin committed with the verification -- `test_recovery_is_refused_forever_because_nothing_clamps_it` -- was inverted to the post-fix expectation FIRST and failed, together with the re-derivation pin; both pass after. It keeps its name: it asserted the defect before the clamp and asserts the repair after it, which is what an acceptance pin is for. Seven cases in all, four of which exist so the fix cannot pass by being broken everywhere: the control (a reservation above the boot rows recovers normally), the affordability bound still deferring untouched, the clamp/affordability interaction, and the two backward-compatibility contracts. Sweep: 2433 passed across unit/managers + unit/mem_ledger + the 681/682 files. Four failures in that run are pre-existing and unrelated -- same four, same messages, on the untouched tree (`BudgetHarness` and `_Sched` stubs missing attributes in test_collective_family_siblings_610 and test_first_chunk_dynamic_chunking); neither file references anything this commit touches.

…es the pool forever FOURTH LATCH OF THE NIGHT, and the same cure as the other three: sgl-project#681's eviction count that could not be paid, sgl-project#682's guard ceiling the scheduler never held, sgl-project#684's `_exhausted_at_rows` process-lifetime marker. Each was a number that could only ratchet one way. WHAT IT COST. `corridor_shortfall_bytes` is added straight to the arming floor's load margin -- `(DEFAULT_MARGIN_MIB << 20) + measured` -- and the arming floor is the binding constraint on two of three ranks. On 2026-08-16 the rank-0 record carried 1004 MiB of it while every record written the day before carried 0, and the boot reading it logged NO breach of its own: it was inherited. The event it descends from is almost certainly 02:36:30 on that exact card, where a test harness belonging to this strand held 4.29 GiB and drove free to 76 MiB. A few seconds of intrusion, taxing every subsequent boot. THE OLD SEMANTICS WERE HALF RIGHT, AND THAT HALF IS KEPT. `record_corridor_shortfall` documents itself as "A MONOTONIC MAXIMUM, deliberately -- a shallower breach later does not mean the deeper one cannot recur; the pool must be sized for the worst instant that has ever been seen". Correct WITHIN an observation. Wrong ACROSS boots that never see it again, because "ever" had no end and nothing could retire a number nobody could reproduce. So: monotonic maximum while it is being OBSERVED, geometric decay across boots that observe nothing. A breach that recurs is re-observed and re-raised to its worst on the spot. One that cannot be reproduced is halved by each flip boot that measures its seam without seeing it, and written off to exactly 0 below `SHORTFALL_FORGET_BYTES` so the decay terminates instead of leaving a tail that still moves the floor. 1004 MiB is gone in seven clean boots. "OBSERVED BY THIS PROCESS" IS THE DISCRIMINATOR, and it is a pid rather than a timestamp because both writers live in the same process: the runtime's corridor audit stamps the record mid-run, and `write_seam_reserve` rewrites it at the end of the same boot's flip measurement. Same pid means this boot saw it and the value stands; a different pid means it was inherited, and a boot that completed a seam measurement without its audit firing is evidence against it. Evidence is what retires it. RANK-LOCAL. The record is per (configuration, rank) and the shortfall is one card's own measurement -- 1004 / 0 / 0 on this boot, legitimately different. No collective reads or writes it and this change adds none. TESTS, red-first: 7 cases. The three decay cases failed before and pass after; the four that pin the half worth keeping -- a breach this process observed is preserved, a deeper one still raises, a shallower one does not lower it -- passed from the start, so the fix cannot have been "delete the term". One case drives the full production scenario: a 1004 MiB one-off decaying to zero while the load margin returns to its default, and one that proves a breach observed on every boot is never decayed away.

…e layout that decodes WHAT THE USER SAW, LIVE. The blocked-admission exit broke the wedge but handed back TP windows that did not do their job: * `arming tp_to_pp: pending > N=7004` fired while carriers were still mid-decode -- the log shows tp_to_pp taken with running bs 2-3 -- so a decode bundle was cut in half by a backlog that is ALWAYS above N under purity; * prefill then ran inside the TP layout, so the carriers that survived the short window met a layout busy prefilling instead of finishing them. Five blocked-admission exits in one boot, at 400-500k pending: the exit kept firing because each TP window returned the same unfinished carriers. The exit was doing its job; the window it handed to was not. THE USER'S SEMANTICS ARE EXPLICIT -- prefill until empty, decode the bundle TO COMPLETION, prefill again -- and this makes the TP side match. 1. TP EXIT = DECODE DRAINED. Under drain mode `tp_to_pp` arms only when `running_bs == 0` and there is prefill worth returning for. The backlog stops being an exit condition, because under purity it is permanent: treating "pending > N" as a reason to leave means never finishing anything. The receipt names what was finished -- "decode bundle complete: B reqs decoded in S s -- exit condition: decode drained" -- with B captured at phase entry, since by the time a bundle drains there is nothing left to count. 2. NO PREFILL IN TP. `prefill_suppressed_in_tp` is consulted by `phase_purity.prefill_blocked_here` BEFORE the purity mode, deliberately: the deployed mode is prefill_in_tp (the 2026-08-14 correction that let the measured break-even N decide, which stands for its own workload), and drain mode is a different contract for this one. A window entered to finish a bundle must not admit the work it was entered to escape. CARDS PARTIALLY IDLE DURING TP IS ACCEPTED, and is the user's stated model. The alternative measured worse: a bundle that never finishes costs an extra round trip and returns the same carriers. OFF BY DEFAULT, gated on `SGLANG_PHASE_POLICY_DRAIN_MODE`. Every rule is byte-identical until it is set -- pinned by `test_drain_mode_off_is_byte_identical_to_today`, which asserts that a backlog above N still arms with a live bundle exactly as it does now. THE BACKSTOPS ARE UNTOUCHED. The 180s decode-stall cap and the #677a progress exit still sit underneath, both pinned. Drain mode changes which condition ENDS a healthy window, never what rescues a broken one. TWO SILENT-FAILURE TRAPS CAUGHT WHILE WRITING THIS, both the shape that has cost this chain real boots: * the purity hook reached for `scheduler.phase_policy_config`; the attribute is `phase_policy_cfg`. With a `getattr` default that is a feature which silently never fires -- the sgl-project#684 serving-tick NameError again. Pinned by `test_the_purity_hook_reads_the_real_scheduler_attribute`, which binds against the name the Scheduler actually sets. * a flag no boot can set is a flag that does nothing, so `test_the_env_knob_turns_drain_mode_on` pins the env wiring in both directions and that unset keeps the current behaviour. 16 hermetic cases: the backlog no longer cutting a live bundle and arming the moment it empties; the receipt naming bundle and duration; the FULL CYCLE -- PP drains to carriers, ONE flip, TP decodes all four to empty, ONE flip back, asserting exactly one arm each way and that the TP arm happens only at bs 0; prefill suppression at the policy level and at its call site; and the two backstops still live.

…refused on structure Desk-only classification. Verdict and fix SHAPE only -- no fix built; the boot wrapper is F4-r4's and the operator's ack is required. THE HYPOTHESIS DOES NOT HOLD, and it is refused structurally rather than for want of evidence. The store port cannot collide across boots: route_a_631_prod_boot.sh pins neither --nccl-port nor --dist-init-addr, so server_args.py:18787-18788 applies -- nccl_port = get_free_port() draws a port that is free at that moment, and a predecessor still holding its own store port is not a candidate. A lost race is caught anyway at :18873 by wait_port_available, which polls 30 s and names the holding process. There is also a shape argument that does not depend on this codebase: "Connection closed by peer" is a connection ESTABLISHED and then broken, whereas a stale predecessor holding a port yields EADDRINUSE at bind or a refused connect. The observed error is the wrong shape for the hypothesis. So the answer to "does wait_host_release check the store port" is no -- and it should not need to. Extending it there would encode a refuted hypothesis into the boot wrapper, where the next reader would trust it. AT LEAST TWO ROOTS, not one. The specimens split on evidence: - 18:23:43 is not NEAR an OOM event, it IS one. syslog:1789 records "A process of this unit has been killed by the OOM killer" at 18:23:43.842677. The same signature repeats at 18:39:05 against e66bde7's own measurement at 18:39:07 -- F4-r4 called that boot "killed by an external process exit", which is what an OOM kill of a peer looks like from inside a survivor. A third sits at 18:45:30. - 19:31:45 rank2 is NOT memory. The host ledger reads avail=103 headroom=97 at 19:32:24, 39 seconds later, and there is no OOM line anywhere in the 19:3x window. SENDBYTES IS A TOMBSTONE. In 3 of 3 recorded instances it is the SECOND event: HANDOFF_663:696-698 (peer dies, no traceback -- "exactly what a SIGKILL looks like"; that run "died of host RAM"), HANDOFF_658:256 (rank 0's TCPStore died, survivors then spun on sendBytes), and e66bde7's gloo frame naming a dead peer process. Any verdict that makes the socket primary is fighting the prior. THE REAL HOLE is elsewhere and is the recurring shape. wait_host_release.sh computes S -- the count of surviving schedulers -- on every iteration, PRINTS it in the clear-to-boot line, and never puts it in a condition. The gate is available-RAM and nothing else, so a predecessor whose schedulers are alive but whose allocations are already unmapped passes it. The single-instance guard does not cover the gap either: boot script :250 pgreps sglang.launch_server, the LAUNCHER, while the store and ranks live in the sglang::scheduler children -- an orphaned scheduler set whose launcher has exited passes cleanly. That is the counter-without-a-reachable-actuator family (sgl-project#679/sgl-project#681/sgl-project#684/sgl-project#715): the value that answers the question is discarded one line before the decision. FIX SHAPE (not built): gate on the counter that already exists -- require S -eq 0 alongside A -ge NEED -- which closes the predecessor window for RAM, GPU and the store socket at once, without a bespoke port probe. Two cautions handed to the owner: grep -c "sglang::schedul" reads ps comm, truncated at 15 chars, so the match is one rename from silently counting zero and a miscounting gate that reports "clear" is worse than none; and requiring zero turns a soft wait hard, which is the right direction but must fail with PIDs named rather than timing out anonymously. HONEST GAPS, recorded in section 6 rather than papered over: dmesg is permission-denied here and journalctl -k returns "No entries", so kernel OOM detail is UNAVAILABLE from this session, not absent -- which process the killer took at 18:23:43 is therefore not established, only that it fired. The serving tree runs under setsid outside systemd, so the absence of an sglang unit line is not evidence it survived. Specimen B's root and the third specimen's log remain open; a timestamp tied to a specimen is not the same as its log read, and I have not read the second. Adds "schedul" to .codespellrc: it is the literal 15-char truncation ps comm reports, not a typo.

…- unstrangle the funder My sgl-project#744 fix has a live regression and this is the refinement. Specimen 21:46:32 on 72696b0aec: tp_to_pp refused 35x, IDLE-LOCK with 407,622 tokens pending and 0 resident, the guard reporting "no KV provider is registered" because the armed-only gate refused the rung at BOTH sites and left the provider list empty. Seam staging (PP1: needs 1269 MiB, spendable 883, rung wanted SHRINK to 126506 with slack 45526 = a 1455 MiB deficit that WAS coverable) could never be funded. THE ERROR WAS MINE AND IT WAS A DESIGN ERROR, not a slip. Seam-funding eviction of recomputable prefix rows is REQUESTED BY the flip machinery -- it is what "KV capacity is the funder" means -- so refusing the rung for the duration of a flip protects against the 21:18 crash by disabling the thing the flip is waiting on. A wholesale gate cannot be right when the protected party and the requesting party are the same machinery. THERE WERE TWO STRANGLE POINTS, not one. The armed gate was the obvious half. The second was quieter: _nothing_resident() returning False on a parked extent dropped through to the unknown-refuse branch, so even with the gate removed the rung would still have declined. Both are fixed. THE REFINEMENT. The parked extent already carries exactly the information needed to be selective, which is why this is clean rather than a compromise: rows INSIDE it are what the flip is about to pack and may not be touched; every row ABOVE it is recomputable prefix and is precisely what the funding wants. So the extent PINS THE CEILING instead of closing the rung. _parked_ceiling() returns the highest parked row id, -1 when nothing is parked, and -2 for the one case that still refuses wholesale: an UNKNOWN extent while a flip is armed, where there is no boundary to name. Both call sites take req_max = max(req_max, parked), so the evictor is handed a ceiling that cannot reach into the extent. UNKNOWN, DECIDED AND DOCUMENTED as asked: it refuses ONLY while a flip is armed. Outside a flip there is nothing parked to protect, and closing the rung there is exactly the strangle this commit removes. sgl-project#746 (the exact arm-time snapshot) is what removes this last wholesale case; until then it is one narrow, named condition rather than the whole flip window. TEST RESULTS test_evict_rung_flip_park_744.py refined to the new semantics, 19 tests + 2 subtests green. The two tests that asserted the wholesale gate are REPLACED rather than deleted -- they now assert the funding path delivers -- and the crash protection gets its own explicit test that the evictor's ceiling never reaches into the parked extent. Mutation matrix, 4 mutants, ALL KILLED, covering both directions: N1 exclusion removed (parked ignored) -> 1 failed CRASH protection dies N2 exclusion covers everything (gate back) -> 1 failed FUNDING dies N3 collecting site ignores parked ceiling -> 2 failed N4 unknown-while-armed no longer refuses -> 3 failed N1 and N2 are the (c) pair: neither over- nor under-covering survives. Pinned suites green together, 142 passed + 87 subtests: sgl-project#744, sgl-project#731 + the #731x#744 interaction pin, both sgl-project#717 suites, sgl-project#656, sgl-project#684, sgl-project#662-f4, sgl-project#713 admission intake, sgl-project#739 prefill progress. Ruff and codespell clean. PP2 SECONDARY, checked and NOT claimed as fixed. The 0 MiB shrink against an unmoved driver-free column is an already-instrumented condition (kv_backing_relief.py:1915-1927): the code detects it, names retained handles (SGLANG_FLIP_SEAM_RETAIN_HANDLES) or a missing commit chunk as the cause, keeps the cap on and returns 0. The gate refinement lets the rung PRICE again; it cannot make a retained-handle arena PAY. Necessary, not sufficient -- the retain setting needs verifying at the boot, and this commit does not pretend to close it. No boot.

Fix StreamExecutor.fork() losing the current role start index.

546ae56

merrymercy merged commit 5ad033a into sgl-project:main Jul 21, 2024

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

Fix StreamExecutor.fork() losing the current role start index. (sgl-p…

43e6517

…roject#684)

Navigation Menu

Uh oh!

Fix StreamExecutor.fork() losing the current role start index. - #684

Fix StreamExecutor.fork() losing the current role start index.#684
merrymercy merged 1 commit into
sgl-project:mainfrom
max99x:main

max99x commented Jul 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

max99x commented Jul 21, 2024

Motivation

Modification

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants