Skip to content

[Scheduler] Throttle waiting-prefix refresh by forward count, not wall time - #43512

Closed
mmangkad wants to merge 4 commits into
mainfrom
mmangkad/fix-waiting-prefix-refresh-rank-divergence
Closed

mmangkad wants to merge 4 commits into
mainfrom
mmangkad/fix-waiting-prefix-refresh-rank-divergence

Conversation

@mmangkad

@mmangkad mmangkad commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

The waiting-prefix refresh from #41593 throttles on each rank's own time.monotonic() (0.5 s). Ranks on opposite sides of the 0.5 s mark refresh on different calls, so their radix-tree LRU order, and later their evictions, diverge. This caused two failures:

  • test_unified_radix_cache_hicache_pp_kl.py (TP2 + PP2, HiCache, 4-gpu-h100): the rank consensus checker caught PP0 evicting 27 tokens and PP1 48. Reproduced on 8x H200 with a per-rank refresh log: PP0 and PP1 disagreed at calls 93 and 94, and about 16 s later the checker fired with CI's exact tokens and hashes. Forcing PP1 never to refresh also trips the checker; SGLANG_ENABLE_WAITING_PREFIX_REFRESH=0 ran clean 4/4.
  • TestInklingSmallCacheConsistencyNightly (TP8, 8-gpu-b200) hung: it passed in the 10-07 nightly and hung on 10-09, the first nightly with [Scheduler] Refresh waiting requests' cached prefixes under FCFS-like policies with LRU eviction #41593. Reproduced on 8x B300 with a per-rank refresh log: ranks disagreed on 495 of 7561 calls while seeing the same queue length, starting before the test's jitter is on. The last disagreements split the ranks {TP1, TP5} vs the rest, the same split as the hang dump (12-request batch vs 1).

Modifications

  • Throttle on Scheduler.forward_ct, which is identical on every rank: at most one refresh per 32 forwards. forward_ct=None skips the refresh.
  • @rank_consensus on UnifiedRadixCache.touch_prefix, so a divergence is reported at the touch itself.
  • The static refresh checks are computed once in __init__; the decode-mode check stays per call because a PD role switch can change it. Two test files now publish the runtime context.
  • Regression test: the same calls, made back to back and 0.6 s apart, must refresh identically.

Tests

HiCache PP file on 8x H200 with SGLANG_ENABLE_RANK_CONSENSUS_CHECKER=1, no divergence in any run:

Class Run 1 Run 2
L3 file backend (crashed in CI) GSM8K 0.960 / 0.960 GSM8K 0.960 / 0.960
max-running-requests 2 GSM8K 0.980 GSM8K 0.980

Before the fix, 3 plain runs passed (the race is intermittent) and an instrumented run crashed.

Instrumented run with the fix (per-rank refresh log, 6/6 tests passed), compared with PP0 TP0:

Class Rows Refreshes Differing rows, other 3 ranks Forwards between refreshes (min / median / max)
L3 file backend 1546 161 0 / 0 / 0 32 / 47 / 10164
max-running-requests 2 150 81 0 / 0 / 0 34 / 335 / 1037

Inkling nightly class only, 8x B300 (CI: B200), CI env:

Code Runs Result
main 3 All hung (hard watchdog); the rank split differed per run
main + this PR 3 All passed (1578-1585 s), 0 mismatches over 20480 positions

One run with the fix logged every refresh: all 8 ranks agreed on all 9303 calls, with 32 / 32 / 822 forwards between refreshes (min / median / max).

Unit tests pass: test_fcfs_waiting_prefix_refresh.py (3), unit/managers/test_schedule_policy.py (11), manual/test_schedule_policy.py (14), test_prefill_result_polling.py (20), test_prefill_adder.py (49).


CI States

Latest PR Test (Base): 🚫 Run #38060589208
Latest PR Test (Extra): ❌ Run #38060589042
Latest PR Test (AMD ROCm 10): ❌ Run #38060589186

@mmangkad

Copy link
Copy Markdown
Contributor Author

/rerun-test test/registered/radix_cache/unified_radix_tree/test_unified_radix_cache_hicache_pp_kl.py test/registered/e2e/models_large/test_inkling_nvfp4_nightly.py

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

🚀 4-gpu-h100 (1 test): ✅ View workflow run

cd test/ && python3 registered/radix_cache/unified_radix_tree/test_unified_radix_cache_hicache_pp_kl.py

🚀 8-gpu-h200 (1 test): ✅ View workflow run

cd test/ && python3 registered/e2e/models_large/test_inkling_nvfp4_nightly.py

🚀 8-gpu-b200 (1 test): ✅ View workflow run

cd test/ && python3 registered/e2e/models_large/test_inkling_nvfp4_nightly.py

@mmangkad mmangkad added the run-ci CI: run the baseline test suite on this PR label Oct 10, 2026
@mmangkad

Copy link
Copy Markdown
Contributor Author

Closing this since #43479 landed first and fixes the same divergence.

Just leaving this here for the record: the two PRs count differently. #43479 refreshes every 32 calc_priority calls, this one every 32 forwards. When the server is saturated, calls get sparse, so the call-based refresh barely runs. I ran a quick bench_serving comparison (generated-shared-prefix, 4k shared prefix, 128 output, Qwen3-4B on 1x H200, 4 running slots, 64k-token KV pool, avg of 2 seeds):

Requests Refresh Cache hit Duration Mean TTFT
128 off 27.9% 23.7 s 11.6 s
128 every 32 calls (#43479) 30.8% 23.5 s 11.5 s
128 every 32 forwards (this PR) 51.8% 21.7 s 10.8 s
512 off 16.0% 97.4 s 48.6 s
512 every 32 calls (#43479) 17.6% 96.9 s 48.4 s
512 every 32 forwards (this PR) 39.6% 89.9 s 45.1 s

So under load, the call-based version behaves pretty much like refresh off, while counting forwards gets ~21 points more cache hit and ~7% more throughput. It only matters when things are backed up, but it might be worth a small follow-up at some point.

@mmangkad mmangkad closed this Oct 11, 2026
@mmangkad
mmangkad deleted the mmangkad/fix-waiting-prefix-refresh-rank-divergence branch October 11, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR unified-radix-cache

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant