[TRTLLM-13409][fix] Count async KV completions as benchmark fill progress by qiaoxj07 · Pull Request #18456 · NVIDIA/TensorRT-LLM

qiaoxj07 · 2026-08-31T15:18:39Z

Dev Engineer Review

Updated benchmark disaggregated fill progress tracking to count synchronous and asynchronous GEN KV completions.
Supports Python V2 completion IDs and C++ request-state-only completion reporting.
Uses the existing model-parallel all-gather for progress propagation.
Limits tracking to active benchmark fill phases and deduplicates completion signals by logical request ID.
No public API, configuration, or non-benchmark execution changes found.
Formatting, linting, syntax, and test-list validation passed.
Full pytest collection remains deferred because local runtime dependencies are missing.

QA Engineer Review

Modified tests/unittest/_torch/executor/test_benchmark_disagg.py:
- Added parameterized coverage for Python completion IDs and C++ request-state completion reporting.
- Added coverage for generation progress, duplicate completion signals, zero-progress watchdog failures, and pipeline-parallel suppression.
- Added coverage for clearing completed transfer IDs when the fill gate opens.
Modified tests/unittest/_torch/executor/test_py_executor.py:
- Updated synchronous transfer tests to use _disagg_gen_transfer_made_progress.
No tests/integration/test_lists/, test-db/, qa/, or waives.txt changes were reported.
The modified test functions are not listed in integration test-list files.
Verdict: sufficient.

Description

#17202 added a no-progress deadline to the benchmark-disaggregated fill gate, but the executor only fed synchronous receive completions into that deadline. With generation-transfer overlap enabled, asynchronous requests can complete steadily while the watchdog sees no progress and eventually terminates a healthy large fill after the default 600-second window.

This change records synchronous and asynchronous GEN KV completions in one per-iteration progress signal. It supports both transceiver contracts on current main: Python V2 completion IDs and the C++ runtime's request-state-only completion reporting. The existing model-parallel all-gather propagates that signal before pacing and stall checks, so this adds no collective and does not change non-benchmark execution or public APIs.

Related: #17202, #18178.

Test Coverage

Added a parameterized CPU regression that covers both Python completion-ID reporting and C++ state-mutation-only reporting.
The regression keeps the fill gate incomplete with an already-expired stall window, then verifies that a real async completion resets the watchdog and avoids the idle polling sleep.
Updated the existing sync completion tests and model-parallel peer-progress coverage for the generalized signal.
pre-commit passed for all changed files, including isort, YAPF, Ruff, Ruff format, codespell, and repository policy hooks.
Python syntax compilation and test-list AST validation passed. Full pytest collection is deferred to CI because the local macOS Python 3.12 tooling environment does not contain the TensorRT-LLM/PyTorch runtime dependencies.

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)
If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.
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.

…ress Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>

coderabbitai · 2026-08-31T15:22:35Z

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: 346f56b1-728d-4068-a930-4adad0588536

📥 Commits

Reviewing files that changed from the base of the PR and between 7631710 and c8b8ccc.

📒 Files selected for processing (2)

tensorrt_llm/_torch/pyexecutor/py_executor.py
tests/unittest/_torch/executor/test_benchmark_disagg.py

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

Walkthrough

Benchmark disaggregated fill tracking now uses generation-transfer state names. The executor records progress from synchronous, Python, and C++ completion signals. The fill gate deduplicates completions and applies the result to retry and stall handling.

Changes

Disaggregated generation transfer

Layer / File(s)	Summary
Progress state and fill gate `tensorrt_llm/_torch/pyexecutor/py_executor.py`	Renames progress state for generation transfers. Aggregates progress across model-parallel ranks. Clears progress and completion IDs when the fill gate opens.
Completion detection `tensorrt_llm/_torch/pyexecutor/py_executor.py`	Records synchronous completions and benchmark-fill progress from Python completion IDs and C++ request-state transitions. It limits scanning to active, non-warmup, non-PP=1 fill phases and deduplicates logical request IDs.
Retry and runtime coverage `tests/unittest/_torch/executor/test_benchmark_disagg.py`, `tests/unittest/_torch/executor/test_py_executor.py`	Covers retry behavior, watchdog handling, duplicate completion signals, pipeline-parallel suppression, gate cleanup, and the renamed progress state.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c8b8c

This localized change extends benchmark fill progress tracking to asynchronous completions without changing public APIs or non-benchmark behavior. It is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

Suggested reviewers: allisonlim-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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 fix and its purpose: counting asynchronous KV completions as benchmark fill progress. It follows the required ticket and type format.
Description check	✅ Passed	The description explains the issue, solution, scope, compatibility, test coverage, and validation results. It also includes the required checklist with the review confirmation marked.

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

qiaoxj07 · 2026-08-31T15:45:44Z

/bot run --disable-fail-fast

qiaoxj07 · 2026-08-31T16:04:29Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-08-31T16:11:30Z

PR_Github #70387 [ run ] triggered by Bot. Commit: 7631710 Link to invocation

tensorrt-cicd · 2026-08-31T19:21:59Z

PR_Github #70387 [ run ] completed with state SUCCESS. Commit: 7631710
/LLM/main/L0_MergeRequest_PR pipeline #57617 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

CI Agent Failure Analysis

Link to invocation

qiaoxj07 · 2026-08-31T19:38:45Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-08-31T19:44:45Z

PR_Github #70432 [ run ] triggered by Bot. Commit: 7631710 Link to invocation

tensorrt-cicd · 2026-08-31T21:57:18Z

PR_Github #70432 [ run ] completed with state SUCCESS. Commit: 7631710
/LLM/main/L0_MergeRequest_PR pipeline #57659 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

CI Agent Failure Analysis

Link to invocation

qiaoxj07 · 2026-08-31T22:02:23Z

/bot run --disable-fail-fast

qiaoxj07 · 2026-08-31T22:20:31Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-08-31T22:27:54Z

PR_Github #70471 [ run ] triggered by Bot. Commit: 7631710 Link to invocation

tensorrt-cicd · 2026-08-31T23:08:35Z

PR_Github #70471 [ run ] completed with state FAILURE. Commit: 7631710
/LLM/main/L0_MergeRequest_PR pipeline #57694 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

CI Agent Failure Analysis

Link to invocation

qiaoxj07 · 2026-09-01T01:33:15Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-01T01:40:42Z

PR_Github #70510 [ run ] triggered by Bot. Commit: 7631710 Link to invocation

tensorrt-cicd · 2026-09-01T02:49:56Z

PR_Github #70510 [ run ] completed with state FAILURE. Commit: 7631710
/LLM/main/L0_MergeRequest_PR pipeline #57728 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

CI Agent Failure Analysis

Link to invocation

qiaoxj07 · 2026-09-01T05:15:09Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-01T05:21:35Z

PR_Github #70588 [ run ] triggered by Bot. Commit: 7631710 Link to invocation

tensorrt-cicd · 2026-09-01T06:06:54Z

PR_Github #70588 [ run ] completed with state SUCCESS. Commit: 7631710
/LLM/main/L0_MergeRequest_PR pipeline #57792 completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, then re-trigger CI with the same bot command (no rebase needed).

⚠️ 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

Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>

qiaoxj07 · 2026-09-01T11:38:32Z

/bot run

tensorrt-cicd · 2026-09-01T11:45:48Z

PR_Github #70684 [ run ] triggered by Bot. Commit: c8b8ccc Link to invocation

tensorrt-cicd · 2026-09-01T13:57:33Z

PR_Github #70684 [ run ] completed with state SUCCESS. Commit: c8b8ccc
/LLM/main/L0_MergeRequest_PR pipeline #57882 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

CI Agent Failure Analysis

Link to invocation

JunyiXu-nv

LGTM! Thanks for the fix!

qiaoxj07 · 2026-09-03T02:22:38Z

/bot run --disable-fail-fast

qiaoxj07 · 2026-09-03T02:23:20Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-03T02:30:21Z

PR_Github #71127 [ run ] triggered by Bot. Commit: c8b8ccc Link to invocation

tensorrt-cicd · 2026-09-03T02:31:17Z

PR_Github #71129 [ run ] triggered by Bot. Commit: c8b8ccc Link to invocation

tensorrt-cicd · 2026-09-03T02:31:20Z

PR_Github #71127 [ run ] completed with state ABORTED. Commit: c8b8ccc

Link to invocation

tensorrt-cicd · 2026-09-03T06:26:09Z

PR_Github #71129 [ run ] completed with state FAILURE. Commit: c8b8ccc
/LLM/main/L0_MergeRequest_PR pipeline #58272 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

CI Agent Failure Analysis

Link to invocation

qiaoxj07 · 2026-09-03T06:33:32Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-03T06:41:18Z

PR_Github #71199 [ run ] triggered by Bot. Commit: c8b8ccc Link to invocation

tensorrt-cicd · 2026-09-03T07:45:04Z

PR_Github #71199 [ run ] completed with state SUCCESS. Commit: c8b8ccc
/LLM/main/L0_MergeRequest_PR pipeline #58334 completed with status: 'SUCCESS'

CI Report

Link to invocation

[TRTLLM-13409][fix] Count async KV completions as benchmark fill prog…

7631710

…ress Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>

qiaoxj07 requested review from a team as code owners August 31, 2026 15:18

qiaoxj07 requested review from Shixiaowei02, allisonlim-nv, cascade812 and pcastonguay August 31, 2026 15:18

github-actions Bot assigned qiaoxj07 Aug 31, 2026

Shixiaowei02 approved these changes Sep 1, 2026

View reviewed changes

Shixiaowei02 added the ci: full pre-merge approved label Sep 1, 2026

Shixiaowei02 reviewed Sep 1, 2026

View reviewed changes

Comment thread tensorrt_llm/_torch/pyexecutor/py_executor.py Outdated

Shixiaowei02 reviewed Sep 1, 2026

View reviewed changes

Comment thread tensorrt_llm/_torch/pyexecutor/py_executor.py Outdated

Shixiaowei02 reviewed Sep 1, 2026

View reviewed changes

Comment thread tests/unittest/_torch/executor/test_benchmark_disagg.py Outdated

[TRTLLM-13409][fix] Address async fill watchdog review

c8b8ccc

Signed-off-by: Xianjie <5410381+qiaoxj07@users.noreply.github.com>

JunyiXu-nv approved these changes Sep 3, 2026

View reviewed changes

qiaoxj07 enabled auto-merge (squash) September 3, 2026 06:14

qiaoxj07 merged commit 6c3235a into NVIDIA:main Sep 3, 2026
11 checks passed

Uh oh!

Conversation

qiaoxj07 commented Aug 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev Engineer Review

QA Engineer Review

Description

Test Coverage

PR Checklist

GitHub Bot Help

Uh oh!

coderabbitai Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

qiaoxj07 commented Aug 31, 2026

Uh oh!

qiaoxj07 commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

qiaoxj07 commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

qiaoxj07 commented Aug 31, 2026

Uh oh!

qiaoxj07 commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

tensorrt-cicd commented Aug 31, 2026

Uh oh!

qiaoxj07 commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

qiaoxj07 commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qiaoxj07 commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

tensorrt-cicd commented Sep 1, 2026

Uh oh!

JunyiXu-nv left a comment

Choose a reason for hiding this comment

Uh oh!

qiaoxj07 commented Sep 3, 2026

Uh oh!

qiaoxj07 commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

Uh oh!

qiaoxj07 commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

Uh oh!

tensorrt-cicd commented Sep 3, 2026

qiaoxj07 commented Aug 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 31, 2026 •

edited

Loading