Repository navigation
ci: pin cuda-bindings for the TensorRT-LLM lane - #2494
Conversation
cuda-bindings 13.4.1, published today, dropped the `reserved` field of cudaIpcMemHandle_t that tensorrt-llm 1.3.0rc24's IPC memory setup reads, so every multi-GPU engine in the lane died at startup with an AttributeError from the moment the release appeared; the lane had been resolving 13.3.1 until then. The install pins 13.3.1 next to tensorrt-llm and adds an import canary so a future drift fails at install time. Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 SummarySummary by CodeRabbit
WalkthroughThe CI installer pins ChangesTensorRT-LLM installation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The TensorRT-LLM CI installer now uses the compatible CUDA bindings release and checks the required IPC field during installation. No concrete current-head merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| # Import canary: fail here (not 20 minutes into the lane) if the pin above | ||
| # no longer matches what tensorrt-llm's IPC path expects. | ||
| python3 -c "from cuda.bindings import runtime; runtime.cudaIpcMemHandle_t().reserved" |
There was a problem hiding this comment.
🟡 Nit: The canary sits above the library-path setup it may depend on. import cuda.bindings.runtime dlopens libcudart.so.13, and the site-packages/nvidia/**/lib dirs only land on LD_LIBRARY_PATH at lines 89–94 — after this line. Today the import resolves via $CUDA_HOME/lib64 from the apt cuda-toolkit-13-0 install (line 45), so it works, but that makes a pin check quietly depend on the system toolkit being present rather than on the wheel that was just installed. If that apt install ever stops shipping the runtime on a runner image, this line turns into a false red that blocks the whole lane instead of reporting a pin mismatch.
Moving it into the === TensorRT-LLM verification === block (line 107, next to the other python3 -c import checks) costs nothing in early-failure terms — it's ~20 lines and a couple of seconds later in the same script, still well before the 20-minute test phase — and it keeps all import checks in one place after the paths are set up.
Also worth wrapping the failure with actionable text, since the raw traceback doesn't say what to do:
python3 -c "from cuda.bindings import runtime; runtime.cudaIpcMemHandle_t().reserved" \
|| { echo "cuda-bindings no longer exposes cudaIpcMemHandle_t.reserved; check the pin in this script against tensorrt_llm/_ipc_utils.py"; exit 1; }
| # cuda-bindings is pinned: 13.4.1 (2026-09-10) dropped the `reserved` field | ||
| # of cudaIpcMemHandle_t that tensorrt-llm's IPC memory setup still reads, so | ||
| # every multi-GPU engine died at startup with an AttributeError the moment | ||
| # the release appeared. 13.3.1 is the last version the lane ran on. |
There was a problem hiding this comment.
🟡 Nit: The pin has no stated removal condition. Once TensorRT-LLM stops reading cudaIpcMemHandle_t.reserved, nothing here will prompt anyone to drop ==13.3.1, and scripts/check_engine_versions.sh only tracks TRTLLM_VERSION — so cuda-bindings will silently stay frozen across future TRTLLM_VERSION bumps.
Other pins in this repo record the exit condition (e.g. scripts/ci_install_e2e_deps.sh:40, "Drop the cap once the stack moves to a protobuf 7 runtime"). Adding one line in the same style would match:
Drop the pin once tensorrt-llm's
_ipc_utils.pystops reading.reserved(upstream issue: …).
Worth noting too: tensorrt-llm requires cuda-python>=13, and cuda-python pins cuda-bindings to its own minor, so this exact pin forces the resolver to backtrack cuda-python off the newest release. It resolves today, but pinning cuda-python==13.3.1 alongside would make the intent explicit and skip the backtracking.
|
The 1-GPU TensorRT-LLM lane is green with the pin and logs |
2 participants
Description
Problem
Since about 01:16 UTC today every
e2e-4gpu-chat (trtllm)lane fails at engine startup withfrom
tensorrt_llm/_ipc_utils.py.cuda-bindings13.4.1 was published at that time and removed the field; the TensorRT-LLM install has no pin on it (it arrives throughtorch==2.11.0+cu130 → cuda-bindings<14,>=13.0.3), so lanes that resolved 13.3.1 this afternoon started resolving 13.4.1 tonight. First seen on #2492's CI; unrelated to that change.Solution
Pin
cuda-bindings==13.3.1next totensorrt-llminscripts/ci_install_trtllm.sh, and add an import canary that touches the field so a future drift fails at install time rather than twenty minutes into the lane.Changes
scripts/ci_install_trtllm.sh: the pin and the canary.Test Plan
bash -non the script.e2e-4gpu-chat (trtllm)lane on this PR is the real test.Checklist
make fmt/cargo +nightly fmt --all