Skip to content

Fix flush cache - #627

Merged
merrymercy merged 2 commits into
mainfrom
fix-flush-cache
Jul 16, 2024
Merged

merrymercy merged 2 commits into
mainfrom
fix-flush-cache

Conversation

@merrymercy

Copy link
Copy Markdown
Contributor

No description provided.

@merrymercy
merrymercy merged this pull request into main Jul 16, 2024
@merrymercy
merrymercy deleted the fix-flush-cache branch July 16, 2024 02:56
merrymercy added a commit that referenced this pull request Jul 16, 2024
timethink pushed a commit to timethink/sglang that referenced this pull request Mar 9, 2025
efschu added a commit to efschu/htsglang that referenced this pull request Aug 7, 2026
efschu added a commit to efschu/htsglang that referenced this pull request Aug 8, 2026
… decidable, and refute the collective-stream fix

The sgl-project#622/sgl-project#649 family is an ordering hazard, not a transport fault. Five
production hangs on 2026-08-07 park all three ranks on the identical host
line, GPUs at 100 %, never a divergence -- but on three DIFFERENT lines across
the specimens (dcp/owner.py:566 twice, the draft kv_indptr .cpu() twice, a
host-path abort once). What they share is the shape: a blocking host sync in
the out-of-graph metadata-prep phase is ordered behind a barlink collective
from the previous step. CUDA streams are in-order, so a collective that stalls
for D blocks the host thread for D, and a host thread that is blocked cannot
enqueue, cannot service the abort gate, and cannot be the rank that unwedges
its peers. A bounded stall becomes an unbounded cluster hang.

Enumerating the syncs does not close it, and that is now measured rather than
argued. sgl-project#623 removed the .item() at owner.py:548 by threading total_tokens
through all five call sites; the 15:55 specimen wedges at owner.py:566, five
lines later in the same function, on boolean-mask indexing that has no
host-derivable form. The callsite at flashinfer_backend.py:7237 does pass
total_tokens, so :548 was correctly skipped and the wedge relocated. That is
NOTE_622 section 3's prediction observed in production, before and after on
one line pair, the same day.

This commit adds the seam that makes the property decidable, and reports what
it decides.

barlink_stream_policy states the placement of forward work -- stream role plus
the cross-stream ordering edges it implies -- as inspectable data instead of a
scatter of torch.cuda.stream contexts across the attention backends, the graph
runners and the transport. It holds no torch import and allocates nothing, so
the decision is testable on a host with no CUDA device.

The falsifier builds the ordering graph a production step would produce under
a given policy and asks whether any collective is reachable backwards from a
prep sync. It is a property test, not a call-site test: a test built around
any one of the five specimens would pass while the class stayed open.

THE RESULT, WHICH IS NEGATIVE FOR THE PROPOSED FIX

Giving the collectives their own stream does NOT satisfy the property. A
collective whose result the model consumes must be joined back onto the
compute stream, and that join is itself a compute-stream node that the next
step's sync waits for. Forking without also isolating the sync side moves the
kernel and keeps the hazard. This is asserted explicitly
(test_collective_stream_alone_violates_the_property) so the limitation cannot
be forgotten and re-proposed.

The placement that does satisfy it is ISOLATED_PREP: the prep phase on its own
stream, ordered after the host-driven input copies and nothing else. Its
soundness obligation -- that prep inputs are host-written -- is encoded rather
than documented: a DEVICE_INPUT is joined into prep and breaks the property
check, so adding a device-produced prep input fails a test instead of hanging
production.

Worth noting for whoever wires this up: the fork/join pattern at
barlink.py:1028-1067 is the gloo host-staged fallback, which contains
ev.synchronize() and a host dist.all_reduce and therefore cannot be captured.
The in-graph BAR1 path (barlink.py:1007 -> barlink_all_reduce) has no stream
context at all and lands on whatever is current. Today's placement is exactly
LEGACY.

TESTS

test_collective_stream_sync_isolation_622.py: 12 passed, 202 subtests, on
CUDA_VISIBLE_DEVICES=99.

Can-fail proof by mutation, both executed:
  - PREP_SYNC moved back onto the compute stream in ISOLATED_PREP
    -> 107 failed, 11 passed.
  - the join modelled as free (no node on the joined stream)
    -> 8 failed, 9 passed, killing both the collective-stream refutation and
       the consumer-ordering control.

Full test/registered/unit/distributed: 2645 passed, 12 skipped, 0 failed.
Baseline before this commit was 2633 passed / 12 skipped / 0 failed; the 12
new tests are the entire delta. The 18 pre-existing failures attributed to
sgl-project#627 in the briefing did not reproduce on this tree -- the named 615/580/603
files are 82/82 green.

ruff and codespell clean.

NOT VALIDATED ON A GPU. This branch was produced on a host with no CUDA
device. ACTIVE is pinned to LEGACY and the module has zero production
importers, so merging changes no runtime behaviour; flipping the default is a
separate, GPU-evidenced change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant