[https://nvbugs/6487038][fix] Stop single-rank disagg errors from crashing all gen ranks

[Shixiaowei02]

Description

Three separate changes, one per commit so each can be bisected and reverted on its own. They
are in one PR so the disagg CI cost is paid once rather than three times.

1. [fix] Do not crash every ADP rank on a disagg transfer error — py_executor.py

_pad_attention_dp_dummy_request() asserted expected_num_active_requests >= len(active_requests). Requests whose KV transfer errored can linger in active_requests for a
tick before cleanup drains them, so a single-rank disagg error tripped the assert and took down
the generation loop on every attention-DP rank. The overshoot is transient and
self-correcting, and the padding keys on the schedulable count rather than the raw length, so
it now warns once and continues.

2. [fix] Surface the engine event-loop error from start_thread — worker.py

A ManagedThread that already ran cannot be restarted, so start_thread fell through to
thread.start() and raised RuntimeError: threads can only be started once out of submit(),
hiding the engine failure that actually killed the thread. It now reports the stashed
_event_loop_error, wrapped as RequestError(str(err)) from err — submit() calls start()
every time, and re-raising the same object would append a frame to its __traceback__ on each
call. This also matches how base_worker.py reports its other submit-path failures. The
post-shutdown path (stop() set stop_event, no error stashed) returns quietly.

3. [chore] Tune gb300 disagg perf-sanity transfer configs — three YAMLs

A behavior change on three CI perf cases, so calling it out explicitly:

• all three migrate the cache transceiver from CPP to PYTHON;
• the two 8k1k con4096 cases get kv_cache_bounce_size_mb: 2048 and
  TRTLLM_KV_TRANSFER_NUM_THREADS=4;
• all three now set kv_transfer_timeout_ms: 600000. The 8k1k cases previously relied on the
  60 s default while switching transceiver, which is the exact bound NVBug 6487038 reports being
  exceeded; transfer time varies with the environment, so all three are bounded the same way.

The 128k8k case deliberately gets neither the bounce buffer nor the extra transfer threads, and
says so inline, because measurement says they hurt this shape.

Test Coverage

Unit tests, all CPU-only:

• tests/unittest/executor/test_event_loop_error_broadcast.py::TestStartThreadAfterExit (new):
  engine error → RequestError with __cause__ intact and no restart; repeated calls → distinct
  exception objects, guarding the __traceback__ accumulation; _event_loop_error is None →
  quiet return; fresh thread → still starts.
• tests/unittest/_torch/executor/test_py_executor.py: the overshoot test now pins the outcome
  (add_dummy_calls == [], active_requests unchanged) instead of only asserting no raise, and
  test_pad_dummy_added_when_overshoot_has_no_schedulable_requests covers the branch that
  matters — overshoot with zero schedulable requests must still add the pad dummy.

A/B for the 128k8k transfer config, GB300, 3 nodes / 12 GPUs, disagg-e2e, one run per arm:

metric	without bounce+threads	with bounce+threads	delta
total token throughput (tok/s)	13690.73	12603.99	−7.9%
output token throughput (tok/s)	805.34	741.41	−7.9%
benchmark duration (s)	7812.20	8485.79	+8.6%
mean TTFT (ms)	2114892.99	2304223.47	+9.0%
mean E2EL (ms)	2182876.21	2373675.33	+8.7%
total runtime (s)	8821	9513	+7.8%
requests	768/768	768/768	both pass

One sample per arm on a shared cluster, so the exact percentages are soft — but every metric
moves the same way, and the slower arm finishes in 9513 s against the case's 180 min budget,
which is why the tuning is not applied there.

Dev Engineer Review

• Replaced the transient active-request assertion with a one-time warning.
• Updated worker-thread handling to surface event-loop errors as RequestError and avoid invalid restarts.
• Updated three GB300 NIXL configurations to use the Python transceiver.
• Added KV-transfer threads, bounce buffers, and 600000 ms timeouts to the 8k1k configurations.
• No public API changes were identified.
• The admission-control bypass removal is not reflected in the listed file changes and requires verification.

QA Engineer Review

• Added test_pad_dummy_tolerates_active_request_overshoot().
• Added test_pad_dummy_added_when_overshoot_has_no_schedulable_requests().
• Added test_surfaces_engine_error_as_request_error().
• Added test_repeated_calls_do_not_accumulate_traceback().
• Added test_post_shutdown_exit_returns_quietly().
• Added test_fresh_thread_is_started().
• Added test_does_not_start_when_enqueueing_is_disabled().
• No corresponding test-db/ or qa/ entries were provided for these tests.
• CI and manual QA coverage are not confirmed.
• Verdict: needs follow-up.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: 501cc0d2-c1ec-4b53-8331-3b2e529d6d2f

📥 Commits

Reviewing files that changed from the base of the PR and between 5cc76ef and 6f99769.

📒 Files selected for processing (7)

• tensorrt_llm/_torch/pyexecutor/py_executor.py
• tensorrt_llm/executor/worker.py
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con256_ctx1_pp4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yaml
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yaml
• tests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yaml
• tests/unittest/_torch/executor/test_py_executor.py
• tests/unittest/executor/test_event_loop_error_broadcast.py

🚧 Files skipped from review as they are similar to previous changes (7)

• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con256_ctx1_pp4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yaml
• tensorrt_llm/executor/worker.py
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yaml
• tests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yaml
• tensorrt_llm/_torch/pyexecutor/py_executor.py
• tests/unittest/_torch/executor/test_py_executor.py
• tests/unittest/executor/test_event_loop_error_broadcast.py

---

Walkthrough

The change tolerates transient active-request overshoot, handles completed response threads and event-loop errors, adds regression tests, and updates disaggregated NIXL transfer settings.

Changes

Executor safeguards and transfer configuration

Layer / File(s)	Summary
Active-request padding tolerance tensorrt_llm/_torch/pyexecutor/py_executor.py, tests/unittest/_torch/executor/test_py_executor.py	Overshoot logs a warning instead of raising. Tests cover schedulable and non-schedulable request states.
Response-thread lifecycle guards tensorrt_llm/executor/worker.py, tests/unittest/executor/test_event_loop_error_broadcast.py	start_thread avoids invalid restarts and converts captured engine event-loop errors to RequestError. Tests cover repeated calls, shutdown, fresh threads, and disabled request enqueueing.
Disaggregated transfer configuration tests/scripts/perf-sanity/disaggregated/*.yaml	NIXL configurations use Python runtimes and set KV-transfer threads, bounce buffers, timeouts, and related comments.

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

Suggested reviewers: bo-nv, cascade812, chuangz0

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the NVBug, fix type, and primary disaggregated execution failure addressed by the pull request.
Description check	✅ Passed	The description clearly explains the three changes and provides detailed unit-test and performance coverage; only the checklist section is omitted.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[Shixiaowei02]

/bot run --post-merge --stage-list "GB200-16_GPUs-4_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE2-GPU8-GEN1-NODE2-GPU8-Post-Merge, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge"

[tensorrt-cicd]

PR_Github #61811 [ run ] triggered by Bot. Commit: d290731 Link to invocation

[tensorrt-cicd]

PR_Github #61811 Bot args parsing error: CI requested by --post-merge or an approval-gated stage selector (*, *Post-Merge*, or *PerfSanity*) requires the ci: post-merge approved PR label. Ask a member of NVIDIA/trt-llm-ci-approvers to apply it.

Link to invocation

[Shixiaowei02]

/bot run --post-merge --stage-list "GB200-16_GPUs-4_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE2-GPU8-GEN1-NODE2-GPU8-Post-Merge, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge"

[tensorrt-cicd]

PR_Github #61826 [ run ] triggered by Bot. Commit: 9727efc Link to invocation

[tensorrt-cicd]

PR_Github #61826 Bot args parsing error: CI requested by --post-merge or an approval-gated stage selector (*, *Post-Merge*, or *PerfSanity*) requires the ci: post-merge approved PR label. Ask a member of NVIDIA/trt-llm-ci-approvers to apply it.

Link to invocation

[github-actions]

Removed the "ci: post-merge approved" label because @niukuo could not be verified as an active member of NVIDIA/trt-llm-ci-approvers. Ask a member of that team to apply it.

[Shixiaowei02]

/bot run --add-multi-gpu-test --disable-fail-fast --extra-stage "GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-1, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-2, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-3, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-4, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-1, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-2"

[tensorrt-cicd]

PR_Github #63367 [ run ] triggered by Bot. Commit: ad17083 Link to invocation

[tensorrt-cicd]

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

[Shixiaowei02]

/bot run --add-multi-gpu-test --disable-fail-fast --extra-stage "GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-1, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-2, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-3, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-4, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-1, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-2"

[tensorrt-cicd]

PR_Github #63438 [ run ] triggered by Bot. Commit: ca0c515 Link to invocation

[tensorrt-cicd]

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

[brnguyen2]

The two code changes look right and the start_thread rework is a clear improvement — the old path fell through to thread.start() and raised RuntimeError: threads can only be started once out of submit(), hiding the actual engine failure.

Two things before merge:

1. Scope and description. The title and NVBug are about ADP crash containment, but half the diff is perf-sanity config tuning (CPP→PYTHON transceiver migration, bounce buffer, KV transfer threads). If these were bundled to avoid paying the disagg CI cost three times, that's a fair reason to keep them in one PR — but then please (a) split them into logically separate commits (ADP assert fix / start_thread error surfacing / perf-sanity config tuning) so each can be bisected and reverted on its own, and (b) list all three in the PR description. Right now the description reads as a single ADP fix, so the transceiver migration — a behavior change on three CI perf cases — is invisible to anyone scanning the log or the PR.

2. The 8k1k configs still run with the default 60s kv_transfer_timeout_ms while switching to the PYTHON transceiver — see inline. The 128k8k config in this same PR uses 600000.

Also: neither new code path is really covered. The new test only asserts "doesn't raise", and the start_thread change has none — tests/unittest/executor/test_event_loop_error_broadcast.py already has stubs that would make a 10-line test cheap.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Shixiaowei02]

The two code changes look right and the start_thread rework is a clear improvement — the old path fell through to thread.start() and raised RuntimeError: threads can only be started once out of submit(), hiding the actual engine failure.

Two things before merge:

1. Scope and description. The title and NVBug are about ADP crash containment, but half the diff is perf-sanity config tuning (CPP→PYTHON transceiver migration, bounce buffer, KV transfer threads). If these were bundled to avoid paying the disagg CI cost three times, that's a fair reason to keep them in one PR — but then please (a) split them into logically separate commits (ADP assert fix / start_thread error surfacing / perf-sanity config tuning) so each can be bisected and reverted on its own, and (b) list all three in the PR description. Right now the description reads as a single ADP fix, so the transceiver migration — a behavior change on three CI perf cases — is invisible to anyone scanning the log or the PR.
2. The 8k1k configs still run with the default 60s kv_transfer_timeout_ms while switching to the PYTHON transceiver — see inline. The 128k8k config in this same PR uses 600000.

Also: neither new code path is really covered. The new test only asserts "doesn't raise", and the start_thread change has none — tests/unittest/executor/test_event_loop_error_broadcast.py already has stubs that would make a 10-line test cheap.

Thanks, both paths are pinned now.

start_thread — added TestStartThreadAfterExit to tests/unittest/executor/test_event_loop_error_broadcast.py, reusing the existing _EngineStub:

• _event_loop_error set → raises RequestError with __cause__ intact, and thread.start() is not called
• called repeatedly → distinct exception objects each time, which guards the __traceback__ accumulation you flagged
• _event_loop_error is None (the post-shutdown path) → returns quietly, no restart
• fresh thread (ident is None) → still starts exactly once

Overshoot test — replaced the "doesn't raise" assertion with assert stub.add_dummy_calls == [] / assert len(stub.active_requests) == 2 as you suggested, and added test_pad_dummy_added_when_overshoot_has_no_schedulable_requests: two requests at GENERATION_TO_COMPLETE, so _count_schedulable_active_requests() returns 0 while len(active_requests) still overshoots — that's the branch where the pad dummy must still be added, which the original test never reached.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unittest/executor/test_event_loop_error_broadcast.py`:
- Around line 109-114: Add a regression test alongside the existing start_thread
tests that creates the worker via _make_worker(can_enqueue=False), invokes
start_thread(), and asserts thread.start() is not called. Keep the existing
enabled-enqueueing coverage unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2cf0a1d7-ea3f-4177-86c7-86d0de359332

📥 Commits

Reviewing files that changed from the base of the PR and between 253797c and 19999f5.

📒 Files selected for processing (7)

• tensorrt_llm/_torch/pyexecutor/py_executor.py
• tensorrt_llm/executor/worker.py
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con256_ctx1_pp4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yaml
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yaml
• tests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yaml
• tests/unittest/_torch/executor/test_py_executor.py
• tests/unittest/executor/test_event_loop_error_broadcast.py

🚧 Files skipped from review as they are similar to previous changes (6)

• tensorrt_llm/_torch/pyexecutor/py_executor.py
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp1_ccb-NIXL.yaml
• tensorrt_llm/executor/worker.py
• tests/scripts/perf-sanity/disaggregated/gb300_kimi-k25-thinking-fp4_8k1k_con4096_ctx1_dep4_gen1_dep16_eplb0_mtp0_ccb-NIXL.yaml
• tests/unittest/_torch/executor/test_py_executor.py
• tests/scripts/perf-sanity/disaggregated/gb300_deepseek-r1-fp4_128k8k_con256_ctx1_pp4_gen1_dep8_eplb0_mtp1_ccb-NIXL.yaml

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Shixiaowei02]

/bot run --add-multi-gpu-test --disable-fail-fast --extra-stage "GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-1, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-2, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-3, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-4, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-1, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-2, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-1, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-2, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-3"

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Shixiaowei02]

/bot run --add-multi-gpu-test --disable-fail-fast --extra-stage "GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-1, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-2, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-3, GB300-20_GPUs-5_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE4-GPU16-Post-Merge-4, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-1, GB300-12_GPUs-3_Nodes-PyTorch-Disagg-PerfSanity-CTX1-NODE1-GPU4-GEN1-NODE2-GPU8-Post-Merge-2, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-1, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-2, GB300-4_GPUs-PyTorch-PerfSanity-Post-Merge-3"

[tensorrt-cicd]

PR_Github #64454 [ run ] triggered by Bot. Commit: 6f99769 Link to invocation