Skip to content

[None][test] Disable ctx overlap scheduler for deepseek-v4-pro con8 disagg config - #19424

Open
chenfeiz0326 wants to merge 1 commit into
NVIDIA:mainfrom
chenfeiz0326:user/chenfeiz/ctxonly-con8-disable-overlap
Open

chenfeiz0326 wants to merge 1 commit into
NVIDIA:mainfrom
chenfeiz0326:user/chenfeiz/ctxonly-con8-disable-overlap

Conversation

@chenfeiz0326

@chenfeiz0326 chenfeiz0326 commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Description

Sets worker_config.ctx.disable_overlap_scheduler: true for
gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.

#18346 removed the unconditional ctx_config["disable_overlap_scheduler"] = True from
the ctx_only path on the rationale that "the overlap scheduler works fine for a
context-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 the
third arm:

ctx disable_overlap_scheduler #18457 iterations req/s
true (this PR) 22 7.82 / 7.75
false (today, pre-#18457) 34 6.00 / 5.99
false (today, main) applied 33 6.20 / 6.25

+25% over current main for the ctx_only case. #18457 is merged and active in the
false arm above (verified: KVCM V2 gate passes, IndexMapper capacity 3 -> 5), so this
is not something the merged fix already covers.

Why the overlap scheduler cannot win here

perf-sanity drives --max-concurrency 8 at request_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's
pre-router active-list size and how many of those are GENERATION_TO_COMPLETE), the
steady state is:

  • A(N) = F(N) — no real backlog; everything resident is a retiring request
  • R(N) = F(N-1) — deferred teardown: the request is still GENERATION_TO_COMPLETE at
    N+1 and only retires there, so its response leaves one iteration late
  • A(N) = R(N-2) — permit release, new POST, next fetch

so 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:

in-flight = cadence x period
    8     =  4.00   x   2     overlap off   (measured 4.000)
    8     =  2.67   x   3     overlap on    (measured 2.696 / 2.727)
   12     =  4.00   x   3     what overlap-on would need to match

7456-token prompts against max_num_tokens: 8192 allow 1 ctx request per rank per
iteration, 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 dep4 would need 12 concurrent requests and the config specifies 8. No
scheduler 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:

test measured here
aggr_upload-ctx_only-... yes, +25%
disagg_upload-e2e-... no
disagg_upload-gen_only-... no
disagg_upload-e2e-time_breakdown-... no

Only ctx_only was 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_only override because the
override would re-disable overlap for all 24 ctx_only configs including the
high-concurrency ones, where overlap plus #18457 beats overlap-off by ~2%. Six
gb300_nemotron-ultra-v3-fp4_* configs already set ctx disable_overlap_scheduler: true,
so this needs no new grammar.

Follow-ups, not addressed here

  • The threshold is concurrency < 3 x ctx_dp, so five other ctx_only configs are
    predicted affected — the three con1 cases most strongly (a 3/2 period ratio on a
    single-user latency measurement). They are left alone pending measurement.
  • The underlying product issue stays open on nvbug 6778910. The lever is residency, not
    accounting: emitting the response before the following _fetch_new_requests would make
    R(N) = F(N) and drop the period to 2. Overlap-on iterations are already cheaper than
    overlap-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_only build path, which copies worker_config.ctx wholesale
(ctx_config = dict(worker_config.get("ctx", {})), test_perf_sanity.py:3511) into
ServerConfig.disable_overlap_scheduler (:1530) and reports it to the perf DB as
b_disable_overlap_scheduler (:1690), so the shift is attributable in the baseline
history:

ctx.disable_overlap_scheduler = True    # this PR
gen.disable_overlap_scheduler = False   # unchanged

The gen: block of this yaml carries its own disable_overlap_scheduler at line 76 and
is deliberately left at false; only the ctx: occurrence at line 106 is changed.

PR Checklist

  • Single-line config change, verified to land in the ctx: block only
  • Measured, 2 reps per arm, counters bit-identical across reps
  • Perf baselines for the four affected tests to be refreshed after merge

🤖 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% higher ctx_only throughput. 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_only case 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.

…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>
@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fb87baab-fefe-45de-9c0e-c85f8bc73af8

📥 Commits

Reviewing files that changed from the base of the PR and between c5c8393 and 8b0f7d4.

📒 Files selected for processing (1)
  • tests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The context worker configuration changes disable_overlap_scheduler from false to true.

Changes

Scheduler Configuration

Layer / File(s) Summary
Context worker scheduler setting
tests/scripts/perf-sanity/disaggregated/gb300_deepseek-v4-pro-fp4_8k1k_con8_ctx1_dep4_gen4_tep8_eplb0_mtp3_ccb-NIXL.yaml
The context worker now sets disable_overlap_scheduler to true.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other

Suggested reviewers: bowenfu

Merge Risk: ⚪ Minimal · up to 8b0f7

No actionable current-head risk is established; the remaining baseline refreshes are planned validation follow-up.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the configuration change: disabling the context overlap scheduler for the specified DeepSeek disaggregated test configuration. It uses the required ticket and type format.
Description check ✅ Passed The description is complete and relevant. It explains the change, performance rationale, measurements, scope, test coverage, unchanged generation settings, and follow-up considerations. It also includ…
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

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