Skip to content

fix(hicache): avoid per-rank-conditional TP collective in is_load_back_event_done (decode-side hierarchical cache) - #29483

Closed
AMD-yanfeiwang wants to merge 1 commit into
sgl-project:mainfrom
AMD-yanfeiwang:fix/hicache-load-back-event-done-no-collective
Closed

AMD-yanfeiwang wants to merge 1 commit into
sgl-project:mainfrom
AMD-yanfeiwang:fix/hicache-load-back-event-done-no-collective

Conversation

@AMD-yanfeiwang

@AMD-yanfeiwang AMD-yanfeiwang commented Jun 27, 2026 •

Copy link
Copy Markdown
Contributor

Scope

Defensive correctness fix for a latent TP-collective desynchronization in the
decode-side hierarchical cache path. It only affects deployments that run
decode with --enable-hierarchical-cache under TP > 1 (where
decode_hicache_mixin calls is_load_back_event_done). Configs that do not
enable decode-side hierarchical cache never reach this code path, so this is a
no-op for them.

Problem

HiRadixCache.is_load_back_event_done() conditionally calls loading_check()
— which issues a TP all_reduce — only on ranks whose local load-back
finish_event has already fired:

if not finish_event.query():
    return False
self.loading_check()   # TP all_reduce, runs on a per-rank-divergent subset
return True

This helper is invoked per request from the decode load-back path
(disaggregation/decode_hicache_mixin.py), so both the number of calls and the
local finish_event.query() result differ across TP ranks. The conditional
collective therefore desynchronizes the TP all_reduce sequence and can
deadlock under TP > 1 (manifesting as a few requests stuck forever with
errors=0 and idle GPUs during high-concurrency decode-side HiCache load-back).

Fix

Make is_load_back_event_done() a pure per-rank GPU-event poll
(return finish_event.query()) and drop the collective. The load ack-reaping
collective is already issued exactly once per scheduler iteration at a
rank-invariant point (check_hicache_events -> loading_check), so acks are
still reaped in lockstep; this helper only needs to report local readiness for
per-request scheduling. This mirrors the existing rank-invariant handling in
writing_check().

Notes / test status

  • python -m py_compile passes.
  • This is a reasoned correctness hardening: it removes a per-rank-conditional TP
    collective, which is unsafe by construction. It has not been independently
    bisected against a live decode-hierarchical-cache TP>1 deadlock repro (our
    observed high-concurrency warmup hang was a separate router/PD-dispatch issue),
    so review of the lockstep-reaping argument above is appreciated.
  • CI

…P collective)

is_load_back_event_done() conditionally called loading_check() (a TP
all_reduce) only on ranks whose local load-back finish_event had already
fired. It is invoked per-request from the decode load-back path, so the
number of calls and the local finish_event.query() result diverge across
TP ranks; the conditional collective therefore desynchronized the TP
all_reduce sequence and could deadlock under TP > 1 (manifesting as a small
number of requests stuck forever with errors=0 and idle GPUs).

The load ack-reaping collective is already issued once per scheduler
iteration at a rank-invariant point (check_hicache_events -> loading_check),
so this per-request helper only needs to report local GPU-event readiness.
Drop the collective and make it a pure finish_event.query(). This mirrors
the existing rank-invariant handling in writing_check().
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@AMD-yanfeiwang AMD-yanfeiwang changed the title fix(hicache): make is_load_back_event_done a pure per-rank poll (no TP collective) fix(hicache): avoid per-rank-conditional TP collective in is_load_back_event_done (decode-side hierarchical cache) Jun 27, 2026
saatwiknagpal added a commit to modal-projects/sglang that referenced this pull request Aug 27, 2026
…k_event_done (backport of sgl-project#29483)

Rank-divergent all_reduce in the decode load-back path deadlocks TP>1
under high-concurrency HiCache load-back. Confirmed present on this
branch; matches the DFLASH+HiCache saturation-hang signature observed
2026-08-26/27 (4/4 containers, watchdog kill ~10 min into fleet load).
CrimsonDump added a commit to CrimsonDump/sglang that referenced this pull request Aug 31, 2026
…on check

is_load_back_event_done() guarded loading_check() -- which issues a TP
all_reduce -- behind a *local* CUDA event query. Each rank's event completes
at a different moment, so ranks took different branches and the collective
was entered by only some of them, aborting with:

    gloo::EnforceNotMet: op.preamble.length <= op.nbytes. 4 vs 2
    Fatal Python error: Aborted

Reproduced on GLM-5.2 (DSA/MLA) with PD disaggregation, decode-side radix
cache + hierarchical cache, TP=8; it fires as soon as L2 load-back gets
dense. TP=1 avoids it by construction, which is why it had no live repro.

Make the check a pure local poll. The acknowledgement is already collected
by check_hicache_events() -> loading_check(finish_count=...), a path that
runs every scheduling round and is rank-invariant, so nothing is lost.

Upstream PR sgl-project#29483 fixes the same bug, but it patches HiRadixCache, which
PR sgl-project#35081 removed from the default selection chain -- DSA models construct
UnifiedRadixCache instead, so that fix never takes effect for them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gm37APvz9k3D8x9Yr2PQew
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @AMD-yanfeiwang. Closing this because it has had no updates in 91 days.

Reopen it if the work is still relevant.

Some directories moved recently, so an older branch may need retargeting:
sgl-kernel/ -> python/sglang/kernels/aot/, python/sglang/jit_kernel/
-> python/sglang/kernels/jit/, docs/ -> docs/docs/ (.mdx),
bench_serving.py -> benchmark/serving.py, test/srt/ -> test/registered/.

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