Skip to content

[https://nvbugs/5948539][fix] Fix disagg gen-only benchmark - #12091

Merged
pcastonguay merged 1 commit into
NVIDIA:mainfrom
Tabrizian:user/imant/nvbugs/5948539
Mar 11, 2026
Merged

[https://nvbugs/5948539][fix] Fix disagg gen-only benchmark#12091
pcastonguay merged 1 commit into
NVIDIA:mainfrom
Tabrizian:user/imant/nvbugs/5948539

Conversation

@Tabrizian

@Tabrizian Tabrizian commented Mar 10, 2026

Copy link
Copy Markdown
Member

@coderabbitai summary

Description

The benchmark fill loop in py_executor.py blocked the GEN server until all benchmark_req_queues_size requests were fetched before processing any of them. In disagg gen_only mode, the CTX server did not have enough KV cache to send all requests at once — it needed the GEN server to fetch and process requests to free KV cache. This created a deadlock.

Changes:

  • Replace the full fill loop with a batched version that fetches tp_size requests per iteration (with ADP) or 1 request per iteration (without ADP), allowing the GEN server to process requests incrementally and free KV cache for the CTX server

Test Coverage

  • Reproduced the hang on Lyris with DeepSeek-R1 GEN-DEP32 config (TP32, ISL 8192/OSL 1024, concurrency 1024)
  • Verified the fix resolves the hang — benchmark completed successfully in ~737 seconds
  • Verified even request distribution across ranks (32 requests per rank)

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

@Tabrizian
Tabrizian requested a review from a team as a code owner March 10, 2026 20:44
@Tabrizian
Tabrizian requested a review from joyang-nv March 10, 2026 20:44
@Tabrizian
Tabrizian force-pushed the user/imant/nvbugs/5948539 branch from f62d5c0 to 688ca80 Compare March 10, 2026 20:46
@coderabbitai

coderabbitai Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR modifies benchmark handling in the TensorRT-LLM executor by removing a dedicated fill loop and refactoring dummy request padding logic to defer dummy request augmentation in benchmark disaggregation mode until all real requests are fetched.

Changes

Cohort / File(s) Summary
Executor Benchmark Logic
tensorrt_llm/_torch/pyexecutor/py_executor.py
Removed benchmark-specific fill loop from _prepare_and_schedule_batch. Modified _pad_attention_dp_dummy_request to skip dummy request padding in benchmark disaggregation mode until all real benchmark requests are fetched (based on num_fetch_requests and benchmark_req_queues_size) and while not in warmup mode. Updated comments and log messages to reflect this deferral behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly explains the issue (deadlock in disagg gen_only mode), the root cause (blocking fill loop), and the solution (batched request fetching). All required sections are present with sufficient detail.
Title check ✅ Passed The title directly references the bug fix for disagg gen-only benchmark hang, which is the primary change in the pull request that removes the fill loop and adjusts dummy-request padding logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@Tabrizian
Tabrizian force-pushed the user/imant/nvbugs/5948539 branch from 688ca80 to 1eb317c Compare March 10, 2026 20:55
Comment thread tensorrt_llm/_torch/pyexecutor/py_executor.py
@Tabrizian
Tabrizian force-pushed the user/imant/nvbugs/5948539 branch 2 times, most recently from 055b808 to bb3c118 Compare March 10, 2026 21:28
@Tabrizian
Tabrizian force-pushed the user/imant/nvbugs/5948539 branch from bb3c118 to 459b3e7 Compare March 11, 2026 05:26
@Tabrizian

Copy link
Copy Markdown
Member Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #38537 [ run ] triggered by Bot. Commit: 459b3e7 Link to invocation

…emoving fill loop

Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
@Tabrizian
Tabrizian force-pushed the user/imant/nvbugs/5948539 branch from 459b3e7 to 2c8bb03 Compare March 11, 2026 05:37
@Tabrizian Tabrizian changed the title [https://nvbugs/5948539][fix] Fix disagg gen-only benchmark hang by removing fill loop [https://nvbugs/5948539][fix] Fix disagg gen-only benchmark Mar 11, 2026
@Tabrizian

Copy link
Copy Markdown
Member Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #38540 [ run ] triggered by Bot. Commit: 2c8bb03 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #38540 [ run ] completed with state SUCCESS. Commit: 2c8bb03
/LLM/main/L0_MergeRequest_PR pipeline #29886 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

@pcastonguay

Copy link
Copy Markdown
Collaborator

/bot run --disable-fail-fast

@pcastonguay
pcastonguay enabled auto-merge (squash) March 11, 2026 13:26
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #38590 [ run ] triggered by Bot. Commit: 2c8bb03 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #38590 [ run ] completed with state SUCCESS. Commit: 2c8bb03
/LLM/main/L0_MergeRequest_PR pipeline #29926 completed with status: 'SUCCESS'

CI Report

Link to invocation

@pcastonguay
pcastonguay merged commit 906781b into NVIDIA:main Mar 11, 2026
9 checks passed
limin2021 pushed a commit to limin2021/TensorRT-LLM that referenced this pull request Mar 19, 2026
…2091)

Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
longcheng-nv pushed a commit to longcheng-nv/TensorRT-LLM that referenced this pull request Mar 31, 2026
…2091)

Signed-off-by: Iman Tabrizian <10105175+tabrizian@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 3, 2026
Design doc covering the deadlock fix in benchmark disaggregated serving
mode: problem statement, root cause analysis (structural starvation in
fill loop), comparison with prior PRs (NVIDIA#12091, NVIDIA#12206), non-blocking
gate design, ADP dummy request handling, and test coverage.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 10, 2026
Design doc covering the deadlock fix in benchmark disaggregated serving
mode: problem statement, root cause analysis (structural starvation in
fill loop), comparison with prior PRs (NVIDIA#12091, NVIDIA#12206), non-blocking
gate design, ADP dummy request handling, and test coverage.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 13, 2026
Design doc covering the deadlock fix in benchmark disaggregated serving
mode: problem statement, root cause analysis (structural starvation in
fill loop), comparison with prior PRs (NVIDIA#12091, NVIDIA#12206), non-blocking
gate design, ADP dummy request handling, and test coverage.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 28, 2026
…stuck-state finding

Document the v2.2 follow-up to PR NVIDIA#13347 and a separate latent issue
discovered while validating it locally:

* 07-fill-phase-flow-control.md (new): v2.2 production fix. Reintroduces
  the per-iteration `tp_size` admission cap that PR NVIDIA#12091 had as part
  of the blocking fill loop, and that PR NVIDIA#12208 implicitly removed when
  it eliminated that loop. Without it, on `dep32 × bs256 == con8192`
  the GEN server is OOM-killed during burst admission before any KV
  transfer can drain. Includes safety analysis, test coverage notes,
  and a tunable-cap discussion.

* 08-fill-phase-stuck-state-finding.md (new): a separate latent
  deadlock observed during local 8-GPU validation, where the
  state-based gate stays closed because some active requests remain in
  INIT after CTX has finished prefilling. Explicitly framed as
  independent of v2.1 / v2.2 (with a per-patch coverage table proving
  none of those patches touch the deadlock's mechanism), with repro
  instructions and recommended next steps.

* README.md: lineage table gains a v2.2 row; current-state section now
  enumerates four issues with v2.1/v2.2 attribution; documents table
  and reading order updated; "Relationship between the steps" notes
  v2.1 and v2.2 both fall under Step 1.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 28, 2026
…rst-OOM

The state-based fill gate fixed the "wrong predicate" failure mode in the
wide-EP Kimi-K2-Thinking gen-only test, but uncovered a latent
fill-phase memory-pressure issue: with the gate now correctly waiting
for all `benchmark_req_queues_size` requests to leave INIT, GEN admits
the entire preloaded waiting queue (`tp_size * max_batch_size` requests)
in a single iteration. This spikes peak KV-cache reservations + NIXL
recv-buffer reservations and OOM-kills the GEN server before any KV
transfer can drain, manifesting in CI as `STEP ... DUE to SIGNAL Killed`
on `dep32 x bs256 == con8192`.

PR NVIDIA#12091 originally throttled per-iteration admission to `tp_size` as
part of its blocking fill loop. PR NVIDIA#12208 removed that throttle along
with the loop, replacing it with a non-blocking gate. The throttle was
the only implicit memory-pressure regulator on the GEN side, and its
removal was the latent regression that the state-based gate (commit
f98e2fd) just made visible.

Reintroduce the per-iteration cap as a deterministic predicate inside
`_pop_from_waiting_queue`, scoped to the benchmark fill phase only:

    if (self.is_benchmark_disagg and self._benchmark_fill_phase_active
            and not self.is_warmup):
        max_new_requests = min(max_new_requests, self.dist.tp_size)

The cap is `tp_size` so admission averages one request per rank per
iteration, regardless of cluster size. With the 100ms outer-loop sleep,
filling an 8192-deep queue takes ~25s, which is the same wall-clock
fill time as PR NVIDIA#12091's per-rank rate (independent of dep size).

The cap is only active under three conjunctive conditions:
* `is_benchmark_disagg`: normal serving never has a queue this large
* `_benchmark_fill_phase_active`: cleared once the gate fires, so
  taper-down and steady-state admission are unaffected
* `not is_warmup`: warmup admits synthetic requests; must not slow

Tests: TestBenchmarkFillAdmissionFlowControl (2 parameterized) verifies
both the cap during fill and the lack-of-cap once the fill phase ends.
Total benchmark-disagg unit tests: 45 passing.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
Made-with: Cursor
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 28, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 29, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request Apr 30, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 1, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 2, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 4, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 4, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 6, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
yingguo-trt pushed a commit to yingguo-trt/TensorRT-LLM that referenced this pull request May 7, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 8, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
chienchunhung added a commit to chienchunhung/TensorRT-LLM that referenced this pull request May 13, 2026
During the initial benchmark fill, admit at most tp_size new requests per
executor iteration. This restores the conservative one-request-per-rank
fill rate from PR NVIDIA#12091 without reintroducing the blocking fill loop
removed by PR NVIDIA#12208.

The cap only applies to non-warmup benchmark fill. Once the gate opens,
normal admission resumes.

Tests cover tp_size 1, 4, and 32, plus the lifecycle path that calls the
real waiting-queue admission helper.

Signed-off-by: Chien-Chun Hung <2679986+chienchunhung@users.noreply.github.com>
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.

4 participants