Minor bugfix on benchmark serving - #1005
Merged
Merged
Conversation
Contributor
Author
|
@zhyncs Do you mind taking a look? I think you probably have the most context regarding this script. Thank you! |
Contributor
|
Thanks! |
zhyncs
approved these changes
Aug 9, 2024
timethink
pushed a commit
to timethink/sglang
that referenced
this pull request
Mar 9, 2025
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
…l-project#801 channel-asymmetry family) Boot 59 gave the first complete three-rank stack of this death class, and it names its own cause. PP2 sat in torch.distributed.isend under send_tensor_dict -- the last rank's output wrap, which addresses ranks[(rank_in_group + 1) % world_size] and so wraps 2 -> 0. That pair had never carried anything, a torch p2p pair is built lazily, and the build needs BOTH ends. PP0 was already parked in _pp_commit_admission_send_work with PP1 beside it, so nobody joined. Closed three-arc cycle, with the ranks at two different lines of ONE loop body: PP2 at :4454, PP0/PP1 at :4514. The first use of a pair must not also be the use that has to succeed while the peers are somewhere else. So the ring walks its neighbours once, over the same groups the real send uses -- device_group for device tensors, cpu_group for the metadata half -- before any loop is entered. Every rank sends to its successor and receives from its predecessor in one shot, which is the ring the loop drives, so no rank waits on a peer that is not simultaneously present. One byte per direction per group, idempotent. Wired at dispatch_event_loop because that is the single entry to every loop, flip and non-flip alike, and once per process. UNGATED, deliberately: the cycle reproduces with enable_phase_flip off, which this window measured directly (sgl-project#990 confirmed on this tree by boot 58 dying flip-free on the same collective). Gating it would leave plain PP holding the defect. Not barlink either -- barlink has no send/recv (sgl-project#732); this is the ordinary send_tensor_dict wrap on the default group. Evidence: desk, executed. py_compile on both files, and a path-identity check against send_tensor_dict: same device_group/cpu_group, same (rank_in_group + 1) % world_size arithmetic, same self.ranks[] mapping, and neither side passes tag=, so both sit in the default tag space. The warmup emits PP-P2P-WARMUP with its pairs and duration per rank, so its absence is as visible as its failure. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
Boot 60 never printed PP-P2P-WARMUP, and the reason was not that the path was unreachable: py-spy caught PP2 inside it -- isend at distributed_c10d.py:2552, warmup_p2p_pairs at parallel_state.py:2458, called from dispatch_event_loop -- with both peers simultaneously in the same call. The line sits after the waits, so a warmup that blocks prints nothing. The cause is my own ordering. `isend` on this path does not return before a matching receive is posted, so a ring in which every rank sends before any rank receives closes on itself: three ranks in isend, nobody in irecv. Posting the receive first costs nothing, cannot block, and leaves a buffer waiting for the predecessor before this rank asks anything of its successor. This also explains boot 60 looking healthier than 54-59 -- zero ADMISSION- WEDGE, zero death markers. It never entered the serving loop at all; it hung in front of it. Absence of the wedge was absence of serving, not survival, and the three "Abort request" lines were the hand probes timing out. sgl-project#1005's premise is therefore still untested, not falsified: the first use of a pair must not be the use that has to succeed while the peers are elsewhere. Boot 60 could not test it because the warmup never completed a pair. Evidence: desk, executed. py_compile; the irecv is asserted to precede the isend in the emitted source. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Aug 29, 2026
Two symmetric orderings, two deadlocks, both caught with py-spy on the same rank: isend first put all three ranks in isend (boot 60, PP2 at parallel_state.py:2458); irecv first put all three in irecv (boot 64, PP2 at :2467). The names say non-blocking, the stacks say otherwise -- on this group the call does not return before it is matched. So ANY ring warmup in which every rank issues the same operation first closes on itself, and a third guess at the order would be the third deadlock. Sequencing removes the symmetry rather than guessing. At step k exactly one rank sends and exactly one receives; every operation is matched the moment it is issued, and no rank waits on a peer that is not already in the matching call. world_size steps with a barrier between them so nobody runs ahead into the next pair. The premise of sgl-project#1005 is still the one under test and still untested: the first use of a pair must not be the use that has to succeed while the peers are elsewhere. Boots 60 and 64 could not test it -- the warmup never completed a pair, and boot 64 is also the run that showed the abort was never the point (see the ledger fix below). Recorded with it, because it unblocked the boot without deleting anything: boot_863_noflip.sh priced 65.4G of flip weight images into a boot whose launch line carries no --enable-phase-flip, and refused itself at 78G available (78 - 65.4 - 10 - 16 = -13.4). The posten is now derived from the launch line the script actually runs, so it cannot drift from it; real headroom is 52G. Boots 46-48 passed that same wrong posten only because the host then had 102G free. No hicache-ratio change, so the config stays comparable with them, and the 58G store nobody needed to delete is still on disk. Evidence: desk, executed. py_compile; the warmup contains no isend/irecv any more and does contain the per-step loop and the barrier. Belegstufe: DESK-BEWIESEN.
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 10, 2026
…product call sites Steps B2 and B3 on top of seat 3's 02b3c99, consuming its API rather than re-deriving any of it (section 10.8: "one reader, or they drift"). THE GEOMETRY HAS ONE OWNER. `leg_geometry(terms)` reads slot width and depth out of `xchg_bounce.BounceTerms`, where `buffer_bytes = widest_layer_bytes * depth` already makes the per-slot width the WIDEST layer -- so a unit is assembled COMPLETE in one slot, which is AMENDMENT 2 and the user law's "vollstaendig zusammengesetzt", and bands == units becomes an identity the run line prints rather than an aspiration. `run_bounce_leg` derives no size and refuses a call that gives it neither a term nor an explicit pair. TWO MOMENTS OF ONE COVERAGE GRADE, and neither can cover the other. `bounce_terms` grades a NUMBER -- `widest_layer_bytes`, from the launch-time checkpoint census (B1b) -- while `refuse_if_plan_exceeds_slot` grades the PLAN the leg actually holds. A census that under-read the widest layer passes the first and must not pass the second, and the run-time refusal carries `under_coverage_refusal`'s own W71 text so the log never holds two spellings of one refusal. THE RUN LINE IS NOT THE ARM LINE. `xchg_bounce.arm_line` owns `WEG2-XCHG-BOUNCE` and reports the sizing; the leg emits `WEG2-XCHG-BOUNCE-LEG` and reports what it moved. One prefix carrying both would make an arm figure and a run figure indistinguishable to any parser, which is the instrument confusion sgl-project#1005 exists to end. PRODUCT CALL SITES: `_weg2_xchg_bounce_leg` and `_weg2_xchg_agreed_leg` on SchedulerWeightUpdaterManager, deliberately thin, holding no state (a slots=True dataclass, for the fifth time in that file) and existing to BE the call site the smoke drives -- sgl-project#1329 was three boots spent on a slice whose mixin methods had no executing test while its module functions all did. Unlike the shadow's hooks beside them they RAISE: an observer that took a flip down over its bookkeeping would be wrong, an authority that swallowed would serve undefined weights. THE ROW MAP IS NOT RE-DERIVED. Every offset the loop uses is the descriptor's own src_off/dst_off/spitch/dpitch as build_plan emitted it, which is where device_block_offsets already knows in_proj_qkvz has FOUR device sub-blocks at 0/g_k/2*g_k/2*g_k+g_v (qwen3_5.py:543) and not the checkpoint's byte offsets -- the defect spec section 2.4 paid for twice. MEASURED WHILE TESTING: Weg2XchgPlanDisagree exists TWICE, as two unrelated RuntimeError subclasses both documented as W68 (weight_exchange.py:201 and weight_exchange_region.py:260). The batcher raises the region's; the test asserts that identity, with a note. Unifying them is not this slice's change. Smoke: 16/16 green, hermetic, CVD="", no CUDA, driven through the two product methods; red on f03e405 as a collection error (module absent).
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 11, 2026
…product call sites Steps B2 and B3 on top of seat 3's 02b3c99, consuming its API rather than re-deriving any of it (section 10.8: "one reader, or they drift"). THE GEOMETRY HAS ONE OWNER. `leg_geometry(terms)` reads slot width and depth out of `xchg_bounce.BounceTerms`, where `buffer_bytes = widest_layer_bytes * depth` already makes the per-slot width the WIDEST layer -- so a unit is assembled COMPLETE in one slot, which is AMENDMENT 2 and the user law's "vollstaendig zusammengesetzt", and bands == units becomes an identity the run line prints rather than an aspiration. `run_bounce_leg` derives no size and refuses a call that gives it neither a term nor an explicit pair. TWO MOMENTS OF ONE COVERAGE GRADE, and neither can cover the other. `bounce_terms` grades a NUMBER -- `widest_layer_bytes`, from the launch-time checkpoint census (B1b) -- while `refuse_if_plan_exceeds_slot` grades the PLAN the leg actually holds. A census that under-read the widest layer passes the first and must not pass the second, and the run-time refusal carries `under_coverage_refusal`'s own W71 text so the log never holds two spellings of one refusal. THE RUN LINE IS NOT THE ARM LINE. `xchg_bounce.arm_line` owns `WEG2-XCHG-BOUNCE` and reports the sizing; the leg emits `WEG2-XCHG-BOUNCE-LEG` and reports what it moved. One prefix carrying both would make an arm figure and a run figure indistinguishable to any parser, which is the instrument confusion sgl-project#1005 exists to end. PRODUCT CALL SITES: `_weg2_xchg_bounce_leg` and `_weg2_xchg_agreed_leg` on SchedulerWeightUpdaterManager, deliberately thin, holding no state (a slots=True dataclass, for the fifth time in that file) and existing to BE the call site the smoke drives -- sgl-project#1329 was three boots spent on a slice whose mixin methods had no executing test while its module functions all did. Unlike the shadow's hooks beside them they RAISE: an observer that took a flip down over its bookkeeping would be wrong, an authority that swallowed would serve undefined weights. THE ROW MAP IS NOT RE-DERIVED. Every offset the loop uses is the descriptor's own src_off/dst_off/spitch/dpitch as build_plan emitted it, which is where device_block_offsets already knows in_proj_qkvz has FOUR device sub-blocks at 0/g_k/2*g_k/2*g_k+g_v (qwen3_5.py:543) and not the checkpoint's byte offsets -- the defect spec section 2.4 paid for twice. MEASURED WHILE TESTING: Weg2XchgPlanDisagree exists TWICE, as two unrelated RuntimeError subclasses both documented as W68 (weight_exchange.py:201 and weight_exchange_region.py:260). The batcher raises the region's; the test asserts that identity, with a note. Unifying them is not this slice's change. Smoke: 16/16 green, hermetic, CVD="", no CUDA, driven through the two product methods; red on f03e405 as a collection error (module absent).
efschu
pushed a commit
to efschu/htsglang
that referenced
this pull request
Sep 11, 2026
…product call sites Steps B2 and B3 on top of seat 3's 02b3c99, consuming its API rather than re-deriving any of it (section 10.8: "one reader, or they drift"). THE GEOMETRY HAS ONE OWNER. `leg_geometry(terms)` reads slot width and depth out of `xchg_bounce.BounceTerms`, where `buffer_bytes = widest_layer_bytes * depth` already makes the per-slot width the WIDEST layer -- so a unit is assembled COMPLETE in one slot, which is AMENDMENT 2 and the user law's "vollstaendig zusammengesetzt", and bands == units becomes an identity the run line prints rather than an aspiration. `run_bounce_leg` derives no size and refuses a call that gives it neither a term nor an explicit pair. TWO MOMENTS OF ONE COVERAGE GRADE, and neither can cover the other. `bounce_terms` grades a NUMBER -- `widest_layer_bytes`, from the launch-time checkpoint census (B1b) -- while `refuse_if_plan_exceeds_slot` grades the PLAN the leg actually holds. A census that under-read the widest layer passes the first and must not pass the second, and the run-time refusal carries `under_coverage_refusal`'s own W71 text so the log never holds two spellings of one refusal. THE RUN LINE IS NOT THE ARM LINE. `xchg_bounce.arm_line` owns `WEG2-XCHG-BOUNCE` and reports the sizing; the leg emits `WEG2-XCHG-BOUNCE-LEG` and reports what it moved. One prefix carrying both would make an arm figure and a run figure indistinguishable to any parser, which is the instrument confusion sgl-project#1005 exists to end. PRODUCT CALL SITES: `_weg2_xchg_bounce_leg` and `_weg2_xchg_agreed_leg` on SchedulerWeightUpdaterManager, deliberately thin, holding no state (a slots=True dataclass, for the fifth time in that file) and existing to BE the call site the smoke drives -- sgl-project#1329 was three boots spent on a slice whose mixin methods had no executing test while its module functions all did. Unlike the shadow's hooks beside them they RAISE: an observer that took a flip down over its bookkeeping would be wrong, an authority that swallowed would serve undefined weights. THE ROW MAP IS NOT RE-DERIVED. Every offset the loop uses is the descriptor's own src_off/dst_off/spitch/dpitch as build_plan emitted it, which is where device_block_offsets already knows in_proj_qkvz has FOUR device sub-blocks at 0/g_k/2*g_k/2*g_k+g_v (qwen3_5.py:543) and not the checkpoint's byte offsets -- the defect spec section 2.4 paid for twice. MEASURED WHILE TESTING: Weg2XchgPlanDisagree exists TWICE, as two unrelated RuntimeError subclasses both documented as W68 (weight_exchange.py:201 and weight_exchange_region.py:260). The batcher raises the region's; the test asserts that identity, with a note. Unifying them is not this slice's change. Smoke: 16/16 green, hermetic, CVD="", no CUDA, driven through the two product methods; red on f03e405 as a collection error (module absent).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a minor bug in the benchmark serving where it incorrectly records TTFT as part of ITLs for a request. This issue was originally raised here but I figured to fix it here as well :)