Repository navigation
Conversation
Apply upstream UCX commit 62182080282372e27a5cad45f23b7ac424b7cd94 on the existing 65e342a82dd2609a28c8c2b8c45ed7fe2cb33fa0 base. Advancing the timer wheel before invoking callbacks prevents rearmed timers from firing again in one catch-up sweep and falsely timing out UD peers during NIXL transfers. Upstream: openucx/ucx#12163 Validated the stock/fixed native timer regression and all four Rubin GLM full GSM8K/acceptance-length and one-hour performance workloads with this exact UCS patch over the unchanged model runtime. Signed-off-by: Dongfeng Yu <dongfengy@nvidia.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Pin and apply the timer fix docker/common/install_ucx.sh |
The script pins the timer-fix commit, fetches it, applies the diff from its parent, and removes the temporary patch. The script also adds an Apache 2.0 license header. |
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Suggested reviewers: weiminwang-nv
Merge Risk: 
f6aab
-
The UCX installer now applies the upstream timer-wheel fix on top of the pinned UCX base. The patch-application steps resolve their files correctly, and no merge-blocking issue was found in this script. The image has not yet been rebuilt with the updated installer, which is normal validation still to be done.
Pre-merge checks: 
-
Check name Status Explanation Title check The title follows the required [None][fix] format and clearly identifies the UCX timer-wheel backport that fixes false UD timeouts. Description check The description is complete and directly explains the issue, root cause, backport, scope, validation, limitations, and checklist status. It includes the required Description, Test Coverage, and PR Che… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and directly explains the issue, root cause, backport, scope, validation, limitations, and checklist status. It includes the required Description, Test Coverage, and PR Checklist sections.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Description
NIXL KV transfers can fail with
Endpoint timeout (-80)after a UCX UD endpoint incorrectly times out during timer-wheel catch-up. Backport UCX #12163 so a callback-rearmed timer cannot execute again in the same overdue sweep and produce a false timeout.The installer applies upstream commit
62182080282372e27a5cad45f23b7ac424b7cd94to the existing pinned UCX base65e342a82dd2609a28c8c2b8c45ed7fe2cb33fa0. Scope: one file, 22 insertions, including the required copyright header.Extracted from #21374 as one commit directly on main base
66190c8fd547a1d9c4fa93e3619a1ad56d8cb128. Related PRs: Rubin staircase and initial KV-cache cleanup. The latter fixes a separate startup allocation problem.UCX timeout: failure, investigation, and fix
Observed failure
During GLM Rubin P4 disaggregated serving (one TP4 context instance, three TP8 generation instances, concurrency 28), NIXL reported
Endpoint timeout (-80)while transferring KV data. Tracing the sender's UCX endpoint to its remote UD endpoint showed that the receiver had already incorrectly timed out and deleted that connection several minutes earlier.Selected messages from one P4 diagnostic run with CUDA IPC disabled, in chronological order (2026-10-31 UTC; host/PID prefixes omitted and sender/receiver labels added). The same failure chain also reproduced with the original transport selection:
The impossible
18446744073.71 secvalue is the key clue: the elapsed-time calculation wrapped as an unsigned integer. The later sender-side 30.92-second timeout is a consequence of the lost peer connection.How it was isolated
libucs, without GLM or GPU execution. The old library dispatches a callback-rearmed timer repeatedly in one overdue sweep; the patched library dispatches it once and passes wraparound, cancellation and rescheduling checks.The bug and fix
In the old timer wheel, an overdue sweep invokes callbacks while its cursor still points into earlier time slots. A callback can rearm its timer into a later slot that the same sweep has yet to visit. When UD's keepalive callback runs again, it can subtract a freshly updated send timestamp from the sweep's older timestamp. That unsigned underflow produces the false peer timeout above.
Upstream UCX #12163, merged on October 29, collects expired timers and advances the wheel cursor before invoking callbacks, preserving cancellation/rescheduling semantics.
docker/common/install_ucx.shapplies that exact fix (62182080282372e27a5cad45f23b7ac424b7cd94) to the existing pinned UCX base (65e342a82dd2609a28c8c2b8c45ed7fe2cb33fa0). Validation retained the original eight-thread configuration and transport selection; it did not rely on increasing timeouts or disabling dedicated threads.Why this workload exposed it
Our disaggregated setup creates connections for eight dedicated NIXL UCX workers during metadata exchange. Transfers are split among workers according to their descriptor count, so some connections exist for minutes before their worker first handles a sufficiently large KV transfer. In the captured P4 failure, the receiver had already deleted such a connection before a later KV transfer activated the previously idle sender worker. P4's multiple generation instances and varying KV transfer sizes expose this connection lifecycle.
This is a timing-dependent UCX defect, not a GLM/Rubin-only condition. Idle time or eight threads alone is not sufficient: stock UCX also passed a full P1 run, a P4 run with dedicated threads disabled, and a smaller idle-connection test. The specific trigger is the overdue-sweep/rearm ordering above. Aggregate serving does not exercise this CTX-to-GEN NIXL KV-transfer path, and we have not established that other TRT-LLM users never encounter the defect. The exact endpoint-level failure chain was captured for P4; the earlier P1 failure was not independently attributed to this mechanism.
Validation
Existing validation from the combined implementation in #21374:
libucsand passes with this exact backport. The patched library also passes wraparound, cancellation and rescheduling checks.2a8b1662501f57713de3771d25d7457517e121deplus the exact UCX overlay on immutable image19760, with the patched library identity verified in actual model workers. They also included the separate staircase and startup-cleanup changes; they are not isolated measurements of this split PR.No new jobs were launched for the split. Normal deployment requires rebuilding the image with the updated installer. The existing validation exercises the exact patched overlay, not a rebuilt image. The detailed four-point performance and accuracy results belong to the staircase PR.
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-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin 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.Dev Engineer Review
docker/common/install_ucx.shnow applies UCX commit62182080282372e27a5cad45f23b7ac424b7cd94on top of the pinned UCX base before archiving and building UCX. This backports the timer-wheel fix without changing the base commit. The installer fetches the fix and its parent to generate the patch. Build validation with an image rebuilt from this installer was not reported.QA Engineer Review
No test changes.
Per-File QA Perspective
docker/common/install_ucx.sh: The installer adds the timer-wheel fix to the UCX source before the build. Verify that the pinned base and fetched fix produce a clean patch application and that the resulting UCX build includes the fix.