[None][test] Disable ctx overlap scheduler for deepseek-v4-pro con8 disagg config - #19424
Open
chenfeiz0326 wants to merge 1 commit into
Open
chenfeiz0326 wants to merge 1 commit into
chenfeiz0326 wants to merge 1 commit into
Conversation
…isagg config The ctx_only perf-sanity case generated from this config measures 25% higher throughput with the ctx worker's overlap scheduler disabled (7.82/7.75 vs 6.20/6.25 req/s, 22 vs 33 iterations, 2 reps per arm on GB300/oci-aga). --max-concurrency 8 makes this a closed loop, so cadence is set by the permit round-trip. Deferred teardown keeps a request resident as GENERATION_TO_COMPLETE for one extra iteration before its response is sent, making the round trip 3 iterations instead of 2. Sustained cadence is then 8/3 = 2.67 forwards per iteration against a 4/iteration cap, and matching overlap-off would require 12 concurrent requests where the config specifies 8. NVIDIA#18457 is merged and active in the measured overlap-on arm; it removes the phantom admission charge (summed charge 86 -> 3, exactly the overlap-off value) but not the extra iteration of residency, so it recovers 1 iteration of 34. NVIDIA#18346 removed the unconditional ctx_only override on the rationale that the overlap scheduler works fine for a context-only run. That holds at high concurrency; this config is below the concurrency where it can. A per-config yaml value is used rather than restoring the override so the high-concurrency ctx_only configs keep overlap enabled. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: chenfeiz0326 <203214996+chenfeiz0326@users.noreply.github.com>
chenfeiz0326
requested review from
Tabrizian,
dhansen-nvidia,
longlee0622 and
pcastonguay
September 18, 2026 15:38
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe context worker configuration changes ChangesScheduler Configuration
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable current-head risk is established; the remaining baseline refreshes are planned validation follow-up. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Closed
2 tasks
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
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.
Description
Sets
worker_config.ctx.disable_overlap_scheduler: trueforgb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.#18346 removed the unconditional
ctx_config["disable_overlap_scheduler"] = Truefromthe
ctx_onlypath on the rationale that "the overlap scheduler works fine for acontext-only run". That holds at high concurrency, but not for this config. Measured on
oci-aga GB300, 3 arms x 2 reps, base
21dc97fbc8(pre-#18457) with #18457 applied as thethird arm:
disable_overlap_schedulertrue(this PR)false(today, pre-#18457)false(today, main)+25% over current main for the
ctx_onlycase. #18457 is merged and active in thefalsearm above (verified: KVCM V2 gate passes,IndexMapper capacity3 -> 5), so thisis not something the merged fix already covers.
Why the overlap scheduler cannot win here
perf-sanity drives
--max-concurrency 8atrequest_rate=inf, i.e. a closed loop:nothing arrives except as an echo of a completion. With per-iteration/per-rank counters
added to
py_executor.py(forwarded, router in/assigned/scheduled, plus each rank'spre-router active-list size and how many of those are
GENERATION_TO_COMPLETE), thesteady state is:
A(N) = F(N)— no real backlog; everything resident is a retiring requestR(N) = F(N-1)— deferred teardown: the request is stillGENERATION_TO_COMPLETEatN+1and only retires there, so its response leaves one iteration lateA(N) = R(N-2)— permit release, new POST, next fetchso
F(N) = F(N-3): a 3-iteration permit round-trip, against 2 with overlap disabled(confirmed by E2E latency in iteration units, 2.00 vs 2.78). Sustained cadence is then
fixed by Little's Law on the fetch grid:
7456-token prompts against
max_num_tokens: 8192allow 1 ctx request per rank periteration, so the cap is 4/iteration at
dep4; both overlap-on arms average 2.67 of 4,i.e. the scheduler is never the binding constraint — concurrency is. Matching
overlap-off at
dep4would need 12 concurrent requests and the config specifies 8. Noscheduler or balancing change closes that; #18457 removes the phantom admission charge
(measured: summed charge 86 -> 3, exactly the overlap-off value) but not the extra
iteration of residency, which is why it buys 1 iteration of 34.
Scope
This config generates four CI tests, and the ctx worker's config is shared by all of
them, so all four baselines shift once after merge:
aggr_upload-ctx_only-...disagg_upload-e2e-...disagg_upload-gen_only-...disagg_upload-e2e-time_breakdown-...Only
ctx_onlywas measured. The three disagg modes are expected to be neutral-to-better(the same deferred-teardown residency applies to their ctx worker) but that is a
prediction, not a measurement — happy to measure them before merge if reviewers prefer.
A per-config yaml value is used rather than restoring the
ctx_onlyoverride because theoverride would re-disable overlap for all 24
ctx_onlyconfigs including thehigh-concurrency ones, where overlap plus #18457 beats overlap-off by ~2%. Six
gb300_nemotron-ultra-v3-fp4_*configs already set ctxdisable_overlap_scheduler: true,so this needs no new grammar.
Follow-ups, not addressed here
concurrency < 3 x ctx_dp, so five otherctx_onlyconfigs arepredicted affected — the three
con1cases most strongly (a 3/2 period ratio on asingle-user latency measurement). They are left alone pending measurement.
accounting: emitting the response before the following
_fetch_new_requestswould makeR(N) = F(N)and drop the period to 2. Overlap-on iterations are already cheaper thanoverlap-off (457 vs 502 ms), so fixing residency should land this config ~10% above
the overlap-off number this PR restores, rather than merely matching it.
Test Coverage
Config-only change; no product code touched. The emitted value was verified through the
ctx_onlybuild path, which copiesworker_config.ctxwholesale(
ctx_config = dict(worker_config.get("ctx", {})),test_perf_sanity.py:3511) intoServerConfig.disable_overlap_scheduler(:1530) and reports it to the perf DB asb_disable_overlap_scheduler(:1690), so the shift is attributable in the baselinehistory:
The
gen:block of this yaml carries its owndisable_overlap_schedulerat line 76 andis deliberately left at
false; only thectx:occurrence at line 106 is changed.PR Checklist
ctx:block only🤖 Generated with Claude Code
Dev Engineer Review
The GB300 configuration now sets
worker_config.ctx.disable_overlap_scheduler: true. The change affects only the context worker; the generation worker remains unchanged. Reported measurements show approximately 25% higherctx_onlythroughput. Three related disaggregated baselines require refresh after merge.QA Engineer Review
One performance-sanity configuration file changed. No test functions, test IDs, selectors, or waivers changed. The
ctx_onlycase was measured and the emitted configuration value was verified. Three related disaggregated tests require baseline refreshes. Coverage verdict: needs follow-up.Per-File QA Perspective
tests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.yaml: This configuration disables overlap scheduling for the context worker only. QA should verify the emitted value and refresh the related performance baselines after merge.