Conversation
|
👋 Hi dpressle! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
📝 WalkthroughWalkthroughThe DL Nixlbench Jenkins step no longer forces ChangesGPU test execution
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).ci/jenkins/lib/test-dl-matrix.yamlTraceback (most recent call last): Comment |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The Full analysisSummary: Root cause: UCCL's RDMA layer failed to register the GPU (VRAM) buffer: Implicated commit: unknown — no NIXL source change caused this; the UCCL plugin path is unchanged since PR #1428 (f8e2221, 2026-05-26) / #895 (4749b35). Failure is in the vendored UCCL RDMA layer ( File: Suggested fix: This is an infrastructure/GPUDirect-RDMA failure on Related: UCCL backend history — PR #1428 "Enable local xfer for UCCL backend", PR #895 "Add UCCL backend integration for NIXL". none directly matching this |
|
🤖 CI Triage Agent — TL;DR: The two x86_64 container "Build image" stages failed because Full analysisSummary: Both x86_64 image builds (nixl node 233, nixlbench node 149) aborted during Dockerfile Root cause: Transient network failure reaching Ubuntu package mirrors. Node 233: Implicated commit: None — not a code regression. The failure is environmental (CI runner egress to Ubuntu mirrors was down/blocked, including IPv6 routes being unreachable). File: Dockerfile apt steps — Suggested fix: Re-run the build; this is almost certainly a transient mirror-connectivity blip. To harden against recurrence: (1) add retry/backoff around Related: none |
|
🤖 CI Triage Agent — TL;DR: The Full analysisSummary: Root cause: PR #1981 ("CI: enable GPU tests for nixlbench on DL cluster") turned on the previously-skipped Implicated commit: PR #1981 (enable nixlbench GPU tests); underlying defect in the UCCL backend RDMA epoll-client shutdown path ( File: Suggested fix:
Related: PR #1981 (root trigger); PR #1963 "NIXLBENCH: Fix race condition with ASIO runtime on shutdown" (prior related shutdown-race fix, ASIO path); PR #1964 "CI: Increase cleanup job timeout" (avoid this anti-pattern for a hang). |
For some reason we switched GPU flag off for nixlbench tests on DL cluster. This commit switch it back on to enable GPU tests for nixlbench. Signed-off-by: Daniel Pressler <danielpr@nvidia.com>
When nvidia_peermem is not loaded, UCX sees IB devices but cannot use them with GPU memory, causing all ucxDeviceApi/SingleWriteTest variants to fail with "lane not found". The LogIgnoreGuard already suppresses the warning; checking getIgnoredCount() after backend creation converts the hard failure into a clean GTEST_SKIP on nodes lacking GPUDirect RDMA. Signed-off-by: Daniel Pressler <danielpr@nvidia.com>
ea09411 to
0dfeeeb
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.ci/jenkins/lib/test-dl-matrix.yaml (2)
186-200: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a command-level timeout before re-enabling UCCL GPU tests.
This change restores the UCCL path, but the reported GPU stage hangs during UCCL teardown until the outer Jenkins timeout terminates it. Add a timeout around the Nixlbench/UCCL commands, or temporarily gate this path, so a teardown hang cannot consume the full CI stage.
🤖 Prompt for 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. In @.ci/jenkins/lib/test-dl-matrix.yaml around lines 186 - 200, Add a command-level timeout to the “Run DL Nixlbench tests” step, covering the Nixlbench/UCCL testScript execution so teardown hangs terminate before the outer Jenkins timeout; preserve the existing job configuration and timeout value source where possible.
128-128: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not embed
ucx_versionin the Slurm job name.
ucx_versionmay contain a ref such asrelease/1.22; embedding it injobNamecan produce invalid or unsafe Slurm identifiers. UseBUILD_NUMBER(or a sanitized value) for job/task/container identifiers, while retainingucx_versiononly as the Docker build input.🤖 Prompt for 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. In @.ci/jenkins/lib/test-dl-matrix.yaml at line 128, Update the jobName value in the matrix job configuration to remove ucx_version and use BUILD_NUMBER or another sanitized identifier for Slurm job, task, and container naming. Keep ucx_version referenced only as the Docker build input.Source: Learnings
🤖 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 `@test/gtest/device_api/single_write_test.cu`:
- Around line 202-204: Broaden the setup logic around getIgnoredCount() and
registerMem() so the test skips when GPUDirect RDMA memory registration is
unavailable, not only when backend creation reports a warning. Probe the
registration path or handle registerMem() failure by issuing GTEST_SKIP(), while
preserving normal assertions for other failures.
---
Outside diff comments:
In @.ci/jenkins/lib/test-dl-matrix.yaml:
- Around line 186-200: Add a command-level timeout to the “Run DL Nixlbench
tests” step, covering the Nixlbench/UCCL testScript execution so teardown hangs
terminate before the outer Jenkins timeout; preserve the existing job
configuration and timeout value source where possible.
- Line 128: Update the jobName value in the matrix job configuration to remove
ucx_version and use BUILD_NUMBER or another sanitized identifier for Slurm job,
task, and container naming. Keep ucx_version referenced only as the Docker build
input.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Enterprise
Run ID: e945cca5-d7aa-4cc1-9458-0ca7fcece0ff
📒 Files selected for processing (2)
.ci/jenkins/lib/test-dl-matrix.yamltest/gtest/device_api/single_write_test.cu
| if (lig_->getIgnoredCount() > 0) { | ||
| GTEST_SKIP() << "UCX accelerated IB support not found; GPUDirect RDMA unavailable"; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant test file sections.
git ls-files 'test/gtest/device_api/single_write_test.cu'
echo '---'
nl -ba test/gtest/device_api/single_write_test.cu | sed -n '1,280p'
echo '--- SEARCH ---'
rg -n "getIgnoredCount|LogIgnoreGuard|createBackend|GPUDirect|ibv_reg_mr_iova2|Bad address|accelerated-IB" test/gtest/device_api/single_write_test.cu test/gtest/device_api -SRepository: ai-dynamo/nixl
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map related symbols to understand whether the skip only covers backend creation.
rg -n "class LogIgnoreGuard|struct LogIgnoreGuard|getIgnoredCount|createBackend\(" -S .Repository: ai-dynamo/nixl
Length of output: 10157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the specific failure mode in repo tests/comments/docs.
rg -n "ibv_reg_mr_iova2|Bad address|GPUDirect RDMA unavailable|accelerated IB support not found" -S .Repository: ai-dynamo/nixl
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the test body and the warning guard implementation.
sed -n '150,230p' test/gtest/device_api/single_write_test.cu
echo '---'
sed -n '120,190p' test/gtest/common.h
echo '---'
sed -n '140,185p' test/gtest/common.cppRepository: ai-dynamo/nixl
Length of output: 5708
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the rest of the test to see where GPU memory registration happens relative to the skip.
sed -n '230,420p' test/gtest/device_api/single_write_test.cu
echo '---'
rg -n "register|MemBuffer|cudaMalloc|cudaHostAlloc|queryMem|makeDescList|getBuffer" test/gtest/device_api/single_write_test.cu test/gtest/device_api/utils.cu test/gtest/device_api -SRepository: ai-dynamo/nixl
Length of output: 14417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the UCX createBackend implementation and any log messages about accelerated IB / GDR registration.
rg -n "accelerated IB support was not found|accelerated IB support not found|IB device\\(s\\) were detected|ibv_reg_mr_iova2|Bad address|GPUDirect RDMA|gdr|registration" src test -SRepository: ai-dynamo/nixl
Length of output: 15333
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the UCX helper around the VRAM registration / accelerated IB warning path.
sed -n '560,670p' src/plugins/ucx/ucx_utils.cpp
echo '---'
sed -n '120,210p' test/gtest/hw_warning_test.cppRepository: ai-dynamo/nixl
Length of output: 6193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect whether the device test has any later skip/guard around registration.
sed -n '420,560p' test/gtest/device_api/single_write_test.cuRepository: ai-dynamo/nixl
Length of output: 5748
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm if any other tests use the same warning regex as a skip signal.
rg -n "IB device\\(s\\) were detected, but accelerated IB support was not found|registerMem: registration failed|GPUDirect RDMA unavailable" test src -SRepository: ai-dynamo/nixl
Length of output: 666
Broaden the skip to cover registerMem() failures
This only skips when the backend-creation warning is emitted. The test still reaches registerMem() and can fail later on GPUDirect RDMA registration, so affected nodes will fail instead of skipping. Add a probe for the registration path or skip on registerMem() failure.
🤖 Prompt for 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.
In `@test/gtest/device_api/single_write_test.cu` around lines 202 - 204, Broaden
the setup logic around getIgnoredCount() and registerMem() so the test skips
when GPUDirect RDMA memory registration is unavailable, not only when backend
creation reports a warning. Probe the registration path or handle registerMem()
failure by issuing GTEST_SKIP(), while preserving normal assertions for other
failures.
|
I'll add these changes to #1909. |
3 participants
For some reason we switched GPU flag off for nixlbench tests on DL cluster.
This commit switch it back on to enable GPU tests for nixlbench.
What?
Describe what this PR is doing.
Why?
Justification for the PR. If there is an existing issue/bug, please reference it. For
bug fixes, the 'Why?' and 'What?' can be merged into a single item.
How?
It is optional, but for complex PRs, please provide information about the design,
architecture, approach, etc.
Summary by CodeRabbit