[None][infra] Upgrade UCX to v1.19.x and NIXL to 0.5.0 #7024
Conversation
Signed-off-by: Batsheva Black <[email protected]>
📝 WalkthroughWalkthroughBumps NIXL to 0.5.0 and changes CUDA handling to prepend libcuda directory to LD_LIBRARY_PATH (saving/restoring original). UCX install is made unconditional and bumped to v1.19.x. TransferRequest constructor loses syncMessage; a new notifySyncMessage API is added and tests updated. Several Jenkins LLM image tags updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as Script Runner
participant install_nixl as install_nixl.sh
participant FS as Filesystem
participant Env as Environment
participant Build as Meson/Ninja
Runner->>install_nixl: execute
install_nixl->>Env: OLD_LD_LIBRARY_PATH = $LD_LIBRARY_PATH
install_nixl->>FS: find /usr/local -name libcuda.so.1
alt libcuda found
install_nixl->>Env: CUDA_DIR = dirname(CUDA_SO_PATH)
install_nixl->>Env: LD_LIBRARY_PATH = CUDA_DIR:${LD_LIBRARY_PATH}
else not found
install_nixl-->>Runner: continue without CUDA path
end
install_nixl->>Build: meson configure && ninja build (NIXL v0.5.0)
install_nixl->>Env: export LD_LIBRARY_PATH (include NIXL libs)
install_nixl->>Env: restore OLD_LD_LIBRARY_PATH
sequenceDiagram
autonumber
participant Runner as Script Runner
participant install_ucx as install_ucx.sh
participant FS as Filesystem
participant Git as Git
participant Build as Autogen/Configure/Make
Runner->>install_ucx: execute
install_ucx->>FS: rm -rf /usr/local/ucx
install_ucx->>Git: clone UCX repo (v1.19.x)
install_ucx->>Build: autogen.sh / configure / make -j / make install
install_ucx->>FS: install to /usr/local/ucx
install_ucx->>Env: prepend UCX lib path to LD_LIBRARY_PATH
sequenceDiagram
autonumber
participant Test as Unit Test
participant Agent as BaseTransferAgent
participant Transfer as TransferRequest
Test->>Agent: notifySyncMessage(recipient, message)
Test->>Transfer: construct TransferRequest(... without syncMessage ...)
Test->>Agent: submit TransferRequest
Agent->>Test: notify on completion / verify memory parity
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
/bot run --stage-list "Build-Docker-Images" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
docker/common/install_ucx.sh (4)
10-26: Quote vars and fix minor ShellCheck; make invocation order.Safer quoting prevents word splitting/globbing; also switch to make -j"$(nproc)" install (equivalent but clearer). This also addresses ShellCheck SC2046.
Apply:
-rm -rf ${UCX_INSTALL_PATH} -git clone --depth 1 -b ${UCX_VERSION} ${UCX_REPO} +rm -rf "${UCX_INSTALL_PATH}" +git clone --depth 1 -b "${UCX_VERSION}" "${UCX_REPO}" cd ucx ./autogen.sh ./contrib/configure-release \ - --prefix=${UCX_INSTALL_PATH} \ + --prefix="${UCX_INSTALL_PATH}" \ --enable-shared \ --disable-static \ --disable-doxygen-doc \ --enable-optimizations \ --enable-cma \ --enable-devel-headers \ - --with-cuda=${CUDA_PATH} \ + --with-cuda="${CUDA_PATH}" \ --with-verbs \ --with-dm \ --enable-mt -make install -j$(nproc) +make -j"$(nproc)" install
29-29: Guard writing to ENV file and make it idempotent.ENV may be unset; appending unconditionally can also duplicate entries across rebuilds.
Apply:
-echo "export LD_LIBRARY_PATH=${UCX_INSTALL_PATH}/lib:\$LD_LIBRARY_PATH" >> "${ENV}" +ENV_FILE="${ENV:-/etc/profile.d/trtllm_env.sh}" +mkdir -p "$(dirname "$ENV_FILE")" +# Avoid duplicate exports; also normalize trailing slash +grep -qsF "${UCX_INSTALL_PATH%/}/lib" "$ENV_FILE" || \ + echo "export LD_LIBRARY_PATH=${UCX_INSTALL_PATH%/}/lib:\$LD_LIBRARY_PATH" >> "$ENV_FILE"Would you confirm whether ENV is guaranteed to be set at build-time in all Docker variants for this repo?
4-4: Remove unused variable.GITHUB_URL is not used.
-GITHUB_URL="https://github.com"
5-5: Consider pinning to a concrete tag for reproducibility.Using a maintenance branch (v1.19.x) is fine, but it can drift over time and cause non-reproducible builds. If reproducibility matters, consider pinning to a specific tag (e.g., v1.19.2) and updating intentionally later.
docker/common/install_nixl.sh (5)
29-31: Quote expansions and avoid leading colon in LD_LIBRARY_PATH.Prevents word-splitting and keeps LD_LIBRARY_PATH tidy.
-CUDA_SO_PATH=$(dirname $CUDA_SO_PATH) - -export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$CUDA_SO_PATH +CUDA_SO_PATH="$(dirname "$CUDA_SO_PATH")" +export LD_LIBRARY_PATH="${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}$CUDA_SO_PATH"
33-37: Quote Meson flag values.Safer, future-proof formatting for paths.
-meson setup builddir \ - -Ducx_path=$UCX_INSTALL_PATH \ +meson setup builddir \ + -Ducx_path="$UCX_INSTALL_PATH" \ -Dcudapath_lib="$CUDA_PATH/lib64" \ -Dcudapath_inc="$CUDA_PATH/include" \ - -Dgds_path="$GDS_PATH" \ + -Dgds_path="$GDS_PATH" \ -Dinstall_headers=true
9-9: Quote LD_LIBRARY_PATH save/restore.Avoids accidental word-splitting and preserves exact content.
-OLD_LD_LIBRARY_PATH=$LD_LIBRARY_PATH +OLD_LD_LIBRARY_PATH="${LD_LIBRARY_PATH}"-export LD_LIBRARY_PATH=$OLD_LD_LIBRARY_PATH +export LD_LIBRARY_PATH="${OLD_LD_LIBRARY_PATH}"Also applies to: 42-42
44-44: Guard ENV file usage and make export idempotent.ENV may be unset; avoid duplicate lines across rebuilds.
-echo "export LD_LIBRARY_PATH=/opt/nvidia/nvda_nixl/lib/${ARCH_NAME}:/opt/nvidia/nvda_nixl/lib64:\$LD_LIBRARY_PATH" >> "${ENV}" +ENV_FILE="${ENV:-/etc/profile.d/trtllm_env.sh}" +mkdir -p "$(dirname "$ENV_FILE")" +grep -qsF "/opt/nvidia/nvda_nixl/lib/${ARCH_NAME}" "$ENV_FILE" || \ + echo "export LD_LIBRARY_PATH=/opt/nvidia/nvda_nixl/lib/${ARCH_NAME}:/opt/nvidia/nvda_nixl/lib64:\$LD_LIBRARY_PATH" >> "$ENV_FILE"
18-18: Optional: reduce image size by disabling pip cache.Use --no-cache-dir for build-time tools to keep layer small.
Consider:
- pip3 install --no-cache-dir meson ninja pybind11
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docker/common/install_nixl.sh(2 hunks)docker/common/install_ucx.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
docker/common/install_ucx.sh
[warning] 26-26: Quote this to prevent word splitting.
(SC2046)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (3)
docker/common/install_ucx.sh (2)
5-5: Version bump looks aligned with PR goal.Upgrading to UCX v1.19.x matches the PR objective.
14-25: Double-check configure flags against UCX 1.19.x.Some legacy flags occasionally change across UCX minors (e.g., --with-dm historically), which would break configure. Given set -e, the Docker build will fail quickly, but you might want to pre-verify.
Would you verify these flags are still valid in v1.19.x on your target base image?
docker/common/install_nixl.sh (1)
7-7: Version bump to 0.5.0 looks good.Matches PR objective and aligns with UCX upgrade.
|
PR_Github #15737 [ run ] triggered by Bot |
Signed-off-by: Batsheva Black <[email protected]>
Signed-off-by: Batsheva Black <[email protected]>
|
/bot run --add-multi-gpu-test --disable-fail-fast |
|
PR_Github #15758 [ run ] triggered by Bot |
|
PR_Github #15737 [ run ] completed with state |
|
PR_Github #15758 [ run ] completed with state |
Signed-off-by: Bo Deng <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
jenkins/current_image_tags.properties (1)
14-17: Image tags updated consistently; matches prior suggestion.All four tags are aligned on TRT 10.11.0.33 and the 202508201630 pre-test suffix across x86_64, SBSA, and Rocky8 variants. This matches the earlier reviewer’s suggested values.
🧹 Nitpick comments (1)
jenkins/current_image_tags.properties (1)
14-17: Optional: plan for post-merge tags or digests for reproducibility.If these are for pre-merge validation, consider switching to post-merge commit-hash-stamped tags (or digests) after merge to avoid tag drift and improve reproducibility in subsequent pipelines.
Can you confirm whether your release process updates these to post-merge images automatically, or should this PR/ a follow-up PR perform that bump?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
jenkins/current_image_tags.properties(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-01T15:14:45.673Z
Learnt from: yibinl-nvidia
PR: NVIDIA/TensorRT-LLM#6506
File: examples/models/core/mixtral/requirements.txt:3-3
Timestamp: 2025-08-01T15:14:45.673Z
Learning: In TensorRT-LLM, examples directory can have different dependency versions than the root requirements.txt file. Version conflicts between root and examples dependencies are acceptable because examples are designed to be standalone and self-contained.
Applied to files:
jenkins/current_image_tags.properties
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
|
/bot run --skip-test --extra-stage "DGX_H100-4_GPUs-PyTorch-Others-1,GB200-4_GPUs-PyTorch-1" |
|
PR_Github #15895 [ run ] triggered by Bot |
|
PR_Github #15895 [ run ] completed with state |
|
/bot skip --comment "https://nv/trt-llm-cicd/job/main/job/L0_MergeRequest_PR/11845/" |
|
PR_Github #15975 [ skip ] triggered by Bot |
|
PR_Github #15975 [ skip ] completed with state |
Signed-off-by: Batsheva Black <[email protected]> Signed-off-by: Bo Deng <[email protected]> Co-authored-by: Bo Deng <[email protected]> Signed-off-by: Yuxin <[email protected]>
Summary by CodeRabbit
Description
Test Coverage
GitHub Bot Help
/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...Provide a user friendly way for developers to interact with a Jenkins server.
Run
/bot [-h|--help]to print this help message.See details below for each supported subcommand.
run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental)]Launch build/test pipelines. All previously running jobs will be killed.
--reuse-test (optional)pipeline-id(OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.--disable-reuse-test(OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.--disable-fail-fast(OPTIONAL) : Disable fail fast on build/tests/infra failures.--skip-test(OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.--stage-list "A10-PyTorch-1, xxx"(OPTIONAL) : Only run the specified test stages. Examples: "A10-PyTorch-1, xxx". Note: Does NOT update GitHub check status.--gpu-type "A30, H100_PCIe"(OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.--test-backend "pytorch, cpp"(OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.--only-multi-gpu-test(OPTIONAL) : Only run the multi-GPU tests. Note: Does NOT update GitHub check status.--disable-multi-gpu-test(OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.--add-multi-gpu-test(OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--detailed-log(OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.--debug(OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip testing for latest commit on pull request.
--comment "Reason for skipping build/test"is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.reuse-pipeline
reuse-pipelineReuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.