Allow rndv-based PUT/GET protocol without error handling - #1854
Conversation
Signed-off-by: Thomas Vegas <tvegas@nvidia.com>
|
👋 Hi tvegas1! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIn the ChangesUCX RNDV Pipeline Error Handling
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/build |
|
👀 Investigating |
|
🤖 CI Triage Agent — The
This is an environment/driver issue on the specific SLURM node Neither failure is attributable to the PR's code changes. Summary: Two parallel CI branches failed — the aarch64/ucx-v1.21.x GPU test failed on VRAM dmabuf memory registration, and the aarch64/ucx-master build failed on a pip network timeout. Root cause: The primary GPU-test failure is an environment/driver issue, not a code bug: Implicated commit: none — not attributable to File: src/plugins/ucx/ucx_utils.cpp:565 (the Suggested fix:
Related: none |
|
👀 Investigating |
|
🤖 CI Triage Agent — The logs clearly show this is an infrastructure failure, not a code/test failure. Let me analyze the timeline. The key events:
Looking at the timestamps around the allocation:
The This is a cluster resource / connectivity problem, not a defect introduced by the PR. There is no test that ran and failed, no stack trace, no source-level error. The No source file or commit is implicated — the actual nixl build/test never executed. This is not a hang inside application code either; it's Slurm queue starvation / a control-plane connectivity timeout on the Summary: GPU CI (build #2467) failed during the "Allocate Environment" stage — Slurm Root cause: Infrastructure/resource-availability failure, not a code defect. Both parallel branches ( Implicated commit: unknown (not code-related — the failure is in the CI Slurm allocation step, which runs before any code from the PR is exercised) File: unknown (no source file involved; failure is in the Jenkins Suggested fix: This is a transient infra failure — retry the build. If it recurs: (1) check Related: none found (no issue/PR search performed — this is a self-evident infrastructure allocation timeout). |
|
👀 Investigating |
NIXL v1.4.0 appends 'TELEMETRY_DOCA' to all_plugins in meson.build. That single context line was the whole break: patch 01 failed on it, and because patch-check applies the set cumulatively, patch 02's meson.build and meson_options.txt failures were cascade from 01 applying nothing, not independent conflicts. Carry the new entry in both patches' context and append MORI_IO after it. Nothing else needed touching -- the AIS_MT backend, the MORI_IO plugin and both nixlbench patches apply to v1.4.0 unchanged. `make patch-check` is clean for all three components, and the image builds with UCX, MORI_IO, AIS_MT and POSIX all loading and linking. v1.4.0 also moves into the same UCX territory this tree patches: it sets RNDV_PIPELINE_ERROR_HANDLING=y on UCX >= 1.22 (ai-dynamo#1854). That is rndv_errh_ppln_enable, a different knob from the rma_ppln_enable that patches/ucx/01 flips, so both UCX patches are still required. It does mean upstream is now touching rendezvous protocol selection from the NIXL side, which is the first place to look if the intra-node GPU number moves. The release also formalises a UCX v1.22.x dependency (ai-dynamo#1868), which the pin already satisfies. Benchmark numbers are not re-measured here; the storage node was occupied. Signed-off-by: Stephen Bates <sbates@raithlin.com>
What?
Allow RNDV, even if it does not fully support error handling.
Why?
Needed to enable GET and PUT rendezvous-based protocols.
Summary by CodeRabbit