Refactoring: derive nixlUcxThreadPoolEngine from nixlUcxThreadEngine - v1.4.0 - #2012
Conversation
|
👋 Hi iyastreb! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
/build |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (24)
📝 WalkthroughWalkthroughChangesCI and container build workflows
Release metadata and licensing
UCX shared and dedicated workers
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Jenkins as Jenkins dispatcher
participant WheelCI as Wheel matrix
participant WheelBase as Cached wheel_base
participant Build as build-container.sh
participant ContainerCI as Container verification
Jenkins->>WheelCI: Dispatch wheel build
WheelCI->>WheelBase: Pull CI-managed base image
WheelCI->>Build: Build targeted wheel stage
Jenkins->>ContainerCI: Dispatch build-only verification
ContainerCI->>Build: Build nixl or nixlbench image
Build-->>ContainerCI: Return verification result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.ci/docs/build-wheel-matrix-ci.md (1)
439-456: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTwo docs give opposite instructions for
CI_IMAGE_TAG.build-wheel-matrix-ci.md's new Maintenance section tells contributors to manually bumpCI_IMAGE_TAG, whileci-overview.md's new "CI_IMAGE_TAG management" section (added in the same PR) correctly states it is auto-derived and must never be hand-edited. Per thecidemo-init.shcontract, manually replacing the"CI_MANAGED"placeholder is exactly what triggers the script's hard failure ("CI_MANAGED placeholder missing"), so the Maintenance instruction would actively break CI for anyone who follows it.
.ci/docs/build-wheel-matrix-ci.md#L439-L456: remove/replace the "bumpCI_IMAGE_TAG" and "rebuilding it only requires bumpingCI_IMAGE_TAG" instructions with a note that the tag rebuilds automatically whenevercontrib/Dockerfile.manylinuxchanges (it's incidemo-init.sh'sCI_FILESlist) — no manual edit needed..ci/docs/ci-overview.md#L205-L233: no change needed; keep as the source of truth once the other file is corrected.🤖 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/docs/build-wheel-matrix-ci.md around lines 439 - 456, The maintenance guidance in .ci/docs/build-wheel-matrix-ci.md:439-456 must stop instructing contributors to manually edit CI_IMAGE_TAG; replace both references with guidance that the tag is automatically rebuilt when contrib/Dockerfile.manylinux changes because it is included in cidemo-init.sh’s CI_FILES list, and no manual edit is needed. .ci/docs/ci-overview.md:205-233 requires no direct change and remains the source of truth.
🤖 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 @.ci/docs/build-wheel-matrix-ci.md:
- Around line 129-137: Update the UCX SPCX plugin documentation to state that
--ucx-spcx-plugin-ref defaults to v0.1.0, matching the UCX_SPCX_PLUGIN_REF
default in build-container.sh; keep the existing description of the override
behavior unchanged.
In @.ci/jenkins/lib/build-container-pr-matrix.yaml:
- Around line 39-63: Restructure the job around the Setup docker step and
subsequent contrib/build-container.sh and nixlbench build execution so registry
authentication occurs only in a trusted preparation stage. Pull or otherwise
prepare all required base images before invoking any PR-controlled scripts, then
remove credentials and Podman/Docker authentication state and run the PR build
in an unauthenticated stage; ensure credentials are not exposed through
environment variables, mounts, or auth files during either build path.
In `@contrib/build-container.sh`:
- Around line 359-360: Update the EXIT cleanup setup in the
build-ucx-spcx-plugin and BUILD_INFINIA paths of the container build script so
registering cleanup for INFINIA_LIBS_DIR does not overwrite the existing
SPCX_SRC_DIR handler. Accumulate both directory removals in a single EXIT trap
or otherwise preserve both cleanup actions when both options are enabled.
In `@pyproject.toml`:
- Around line 25-29: Update the pyproject.toml license expression to include
LicenseRef-NvidiaProprietary alongside MIT and Apache-2.0, matching the NVIDIA
modules bundled in the Python wheels. If the license field does not cover wheel
contents, instead explicitly scope or document that exclusion.
In `@src/plugins/ucx/ucx_backend.cpp`:
- Around line 683-685: Update the split-transfer path around
nixlUcxCompositeBackendReqH construction to forward opt_args when selecting the
shared worker, matching the small-batch path. Ensure the worker used for the
composite request is selected consistently with the worker used to prepare the
memory view/rkey.
In `@src/plugins/ucx/ucx_backend.h`:
- Line 300: Rename the private member numSharedWorkers_ to num_shared_workers_
in ucx_backend and update every declaration and reference to use the snake_case
name consistently.
---
Outside diff comments:
In @.ci/docs/build-wheel-matrix-ci.md:
- Around line 439-456: The maintenance guidance in
.ci/docs/build-wheel-matrix-ci.md:439-456 must stop instructing contributors to
manually edit CI_IMAGE_TAG; replace both references with guidance that the tag
is automatically rebuilt when contrib/Dockerfile.manylinux changes because it is
included in cidemo-init.sh’s CI_FILES list, and no manual edit is needed.
.ci/docs/ci-overview.md:205-233 requires no direct change and remains the source
of truth.
🪄 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: 4e89cf4e-3fdd-4bd0-8691-150920e8c8e4
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.ci/cidemo-init.sh.ci/docs/build-wheel-matrix-ci.md.ci/docs/ci-overview.md.ci/jenkins/lib/build-container-pr-matrix.yaml.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/build-wheel-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml.gitignoreATTRIBUTIONS-Python.mdCargo.tomlREADME.mdbenchmark/nixlbench/meson.buildcontrib/Dockerfile.manylinuxcontrib/build-container.shlicenses/NVIDIA-proprietary-LICENSE.txtmeson.buildpyproject.tomlsrc/plugins/ucx/ucx_backend.cppsrc/plugins/ucx/ucx_backend.h
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.ci/docs/build-wheel-matrix-ci.md (1)
439-456: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTwo docs give opposite instructions for
CI_IMAGE_TAG.build-wheel-matrix-ci.md's new Maintenance section tells contributors to manually bumpCI_IMAGE_TAG, whileci-overview.md's new "CI_IMAGE_TAG management" section (added in the same PR) correctly states it is auto-derived and must never be hand-edited. Per thecidemo-init.shcontract, manually replacing the"CI_MANAGED"placeholder is exactly what triggers the script's hard failure ("CI_MANAGED placeholder missing"), so the Maintenance instruction would actively break CI for anyone who follows it.
.ci/docs/build-wheel-matrix-ci.md#L439-L456: remove/replace the "bumpCI_IMAGE_TAG" and "rebuilding it only requires bumpingCI_IMAGE_TAG" instructions with a note that the tag rebuilds automatically whenevercontrib/Dockerfile.manylinuxchanges (it's incidemo-init.sh'sCI_FILESlist) — no manual edit needed..ci/docs/ci-overview.md#L205-L233: no change needed; keep as the source of truth once the other file is corrected.🤖 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/docs/build-wheel-matrix-ci.md around lines 439 - 456, The maintenance guidance in .ci/docs/build-wheel-matrix-ci.md:439-456 must stop instructing contributors to manually edit CI_IMAGE_TAG; replace both references with guidance that the tag is automatically rebuilt when contrib/Dockerfile.manylinux changes because it is included in cidemo-init.sh’s CI_FILES list, and no manual edit is needed. .ci/docs/ci-overview.md:205-233 requires no direct change and remains the source of truth.
🤖 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 @.ci/docs/build-wheel-matrix-ci.md:
- Around line 129-137: Update the UCX SPCX plugin documentation to state that
--ucx-spcx-plugin-ref defaults to v0.1.0, matching the UCX_SPCX_PLUGIN_REF
default in build-container.sh; keep the existing description of the override
behavior unchanged.
In @.ci/jenkins/lib/build-container-pr-matrix.yaml:
- Around line 39-63: Restructure the job around the Setup docker step and
subsequent contrib/build-container.sh and nixlbench build execution so registry
authentication occurs only in a trusted preparation stage. Pull or otherwise
prepare all required base images before invoking any PR-controlled scripts, then
remove credentials and Podman/Docker authentication state and run the PR build
in an unauthenticated stage; ensure credentials are not exposed through
environment variables, mounts, or auth files during either build path.
In `@contrib/build-container.sh`:
- Around line 359-360: Update the EXIT cleanup setup in the
build-ucx-spcx-plugin and BUILD_INFINIA paths of the container build script so
registering cleanup for INFINIA_LIBS_DIR does not overwrite the existing
SPCX_SRC_DIR handler. Accumulate both directory removals in a single EXIT trap
or otherwise preserve both cleanup actions when both options are enabled.
In `@pyproject.toml`:
- Around line 25-29: Update the pyproject.toml license expression to include
LicenseRef-NvidiaProprietary alongside MIT and Apache-2.0, matching the NVIDIA
modules bundled in the Python wheels. If the license field does not cover wheel
contents, instead explicitly scope or document that exclusion.
In `@src/plugins/ucx/ucx_backend.cpp`:
- Around line 683-685: Update the split-transfer path around
nixlUcxCompositeBackendReqH construction to forward opt_args when selecting the
shared worker, matching the small-batch path. Ensure the worker used for the
composite request is selected consistently with the worker used to prepare the
memory view/rkey.
In `@src/plugins/ucx/ucx_backend.h`:
- Line 300: Rename the private member numSharedWorkers_ to num_shared_workers_
in ucx_backend and update every declaration and reference to use the snake_case
name consistently.
---
Outside diff comments:
In @.ci/docs/build-wheel-matrix-ci.md:
- Around line 439-456: The maintenance guidance in
.ci/docs/build-wheel-matrix-ci.md:439-456 must stop instructing contributors to
manually edit CI_IMAGE_TAG; replace both references with guidance that the tag
is automatically rebuilt when contrib/Dockerfile.manylinux changes because it is
included in cidemo-init.sh’s CI_FILES list, and no manual edit is needed.
.ci/docs/ci-overview.md:205-233 requires no direct change and remains the source
of truth.
🪄 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: 4e89cf4e-3fdd-4bd0-8691-150920e8c8e4
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.ci/cidemo-init.sh.ci/docs/build-wheel-matrix-ci.md.ci/docs/ci-overview.md.ci/jenkins/lib/build-container-pr-matrix.yaml.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/build-wheel-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml.gitignoreATTRIBUTIONS-Python.mdCargo.tomlREADME.mdbenchmark/nixlbench/meson.buildcontrib/Dockerfile.manylinuxcontrib/build-container.shlicenses/NVIDIA-proprietary-LICENSE.txtmeson.buildpyproject.tomlsrc/plugins/ucx/ucx_backend.cppsrc/plugins/ucx/ucx_backend.h
🛑 Comments failed to post (4)
.ci/docs/build-wheel-matrix-ci.md (1)
129-137: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale default value: doc says
main, code defaults tov0.1.0.The plugin ref defaults to
mainand is selectable with--ucx-spcx-plugin-ref.contrib/build-container.shactually setsUCX_SPCX_PLUGIN_REF="v0.1.0"as the default. Update the doc to match the real default.🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 129-129: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
🤖 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/docs/build-wheel-matrix-ci.md around lines 129 - 137, Update the UCX SPCX plugin documentation to state that --ucx-spcx-plugin-ref defaults to v0.1.0, matching the UCX_SPCX_PLUGIN_REF default in build-container.sh; keep the existing description of the override behavior unchanged..ci/jenkins/lib/build-container-pr-matrix.yaml (1)
39-63: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not expose registry credentials to PR-controlled build code.
After logging in, this job executes
contrib/build-container.shand the nixlbench build script from the PR checkout. A modified script or Dockerfile can read the injected tokens or Podman auth file and exfiltrate them. Split authenticated image preparation into a trusted stage, then run the PR build without credentials/auth state.Also applies to: 76-96
🤖 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/build-container-pr-matrix.yaml around lines 39 - 63, Restructure the job around the Setup docker step and subsequent contrib/build-container.sh and nixlbench build execution so registry authentication occurs only in a trusted preparation stage. Pull or otherwise prepare all required base images before invoking any PR-controlled scripts, then remove credentials and Podman/Docker authentication state and run the PR build in an unauthenticated stage; ensure credentials are not exposed through environment variables, mounts, or auth files during either build path.contrib/build-container.sh (1)
359-360: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
EXIT trap for
SPCX_SRC_DIRis silently overwritten whenBUILD_INFINIAis also enabled.
trap ... EXITset at line 359 for$SPCX_SRC_DIRgets replaced by the latertrap ... EXITat line 396 for$INFINIA_LIBS_DIRwhen both--build-ucx-spcx-pluginand--build-infiniaare used together — bash only keeps the most recently registered EXIT handler.$SPCX_SRC_DIRis cleaned up again at the start of the next run (line 347), so this isn't a permanent leak, but it silently drops the intended cleanup-on-exit guarantee for one of the two directories.🧹 Proposed fix: accumulate cleanup instead of overwriting the trap
- trap 'rm -rf "$SPCX_SRC_DIR"' EXIT + trap 'rm -rf "$SPCX_SRC_DIR" "${INFINIA_LIBS_DIR:-}"' EXIT mkdir -p "$SPCX_SRC_DIR"rm -rf "$INFINIA_LIBS_DIR" mkdir -p "$INFINIA_LIBS_DIR" - trap 'rm -rf "$INFINIA_LIBS_DIR"' EXIT + trap 'rm -rf "$INFINIA_LIBS_DIR" "${SPCX_SRC_DIR:-}"' EXITAlso applies to: 396-410
🤖 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 `@contrib/build-container.sh` around lines 359 - 360, Update the EXIT cleanup setup in the build-ucx-spcx-plugin and BUILD_INFINIA paths of the container build script so registering cleanup for INFINIA_LIBS_DIR does not overwrite the existing SPCX_SRC_DIR handler. Accumulate both directory removals in a single EXIT trap or otherwise preserve both cleanup actions when both options are enabled.pyproject.toml (1)
25-29: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Files ==\n' git ls-files pyproject.toml README.md licenses/NVIDIA-proprietary-LICENSE.txt || true printf '\n== pyproject.toml (relevant lines) ==\n' nl -ba pyproject.toml | sed -n '1,120p' printf '\n== README.md mentions of wheels/modules/license ==\n' rg -n -i 'wheel|nvidia|license|proprietary|bundl' README.md printf '\n== LICENSE reference file ==\n' nl -ba licenses/NVIDIA-proprietary-LICENSE.txt | sed -n '1,160p'Repository: ai-dynamo/nixl
Length of output: 309
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== project metadata search ==\n' rg -n -i 'license-files|license =|LicenseRef|NVIDIA-proprietary|Apache-2.0|MIT' pyproject.toml README.md .github -g '!**/.git/**'Repository: ai-dynamo/nixl
Length of output: 1872
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('pyproject.toml') for i, line in enumerate(p.read_text().splitlines(), 1): if 1 <= i <= 120: print(f"{i:4d}: {line}") PYRepository: ai-dynamo/nixl
Length of output: 2181
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('README.md') for i, line in enumerate(p.read_text().splitlines(), 1): if 400 <= i <= 430: print(f"{i:4d}: {line}") PYRepository: ai-dynamo/nixl
Length of output: 1004
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path text = Path('pyproject.toml').read_text().splitlines() for i, line in enumerate(text, 1): if i <= 80: print(f"{i:4d}: {line}") PYRepository: ai-dynamo/nixl
Length of output: 2181
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('README.md') for i, line in enumerate(p.read_text().splitlines(), 1): if 410 <= i <= 420: print(f"{i:4d}: {line}") PYRepository: ai-dynamo/nixl
Length of output: 617
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('pyproject.toml') for i, line in enumerate(p.read_text().splitlines(), 1): if 20 <= i <= 32: print(f"{i:4d}: {line}") PYRepository: ai-dynamo/nixl
Length of output: 545
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path for path in ['pyproject.toml', 'README.md', 'LICENSE', 'licenses/NVIDIA-proprietary-LICENSE.txt']: p = Path(path) print(f"\n== {path} ==") if p.exists(): for i, line in enumerate(p.read_text().splitlines(), 1): if path != 'README.md' or i <= 40 or 410 <= i <= 420: print(f"{i:4d}: {line}") else: print("missing") PYRepository: ai-dynamo/nixl
Length of output: 19059
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n -i 'LicenseRef-NvidiaProprietary|license = |license-files|SPDX-License-Identifier|third-party component|bundle NVIDIA modules|wheel' . -g '!**/.git/**'Repository: ai-dynamo/nixl
Length of output: 50370
Include
LicenseRef-NvidiaProprietaryin the wheel license expression.pyproject.tomlstill advertises onlyMIT AND Apache-2.0, whileREADME.mdsays the Python wheels bundle NVIDIA modules underLicenseRef-NvidiaProprietary. If this field covers the wheel, add that SPDX reference; otherwise scope or document the exclusion.🤖 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 `@pyproject.toml` around lines 25 - 29, Update the pyproject.toml license expression to include LicenseRef-NvidiaProprietary alongside MIT and Apache-2.0, matching the NVIDIA modules bundled in the Python wheels. If the license field does not cover wheel contents, instead explicitly scope or document that exclusion.
|
🤖 CI Triage Agent — TL;DR: Two CI jobs failed on PR #2012 (UCX thread-pool engine refactor). The CPP-tests stage hung for ~2h in the Full analysisSummary: Root cause:
Implicated commit: PR #2012 head File: Suggested fix: Investigate the thread-pool engine's progress handling when no progress thread is enabled ( Related: PR #2012 (#2012); prior refactor PR #1906 (commit 209abbe). Also see #1743 ("TEST/GTEST: Run in single process") and #1880 ("ucx: fix infinite loop dropping pending requests on dedicated-thread exit") as related UCX thread/progress-handling context.
|
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage was aborted after a ~47-minute hang (13:13:10 → 14:00:49 of total silence) in the UCCL ASIO-runtime benchmark loop; the fix is to disable/skip the UCCL ASIO tests (lines 95-103 of Full analysisSummary: Jenkins stage 356 "Run Nixlbench tests" (job nixl-ci-gpu #3008) was ABORTED because a Root cause: A hang, not a timeout. The last application output was at Implicated commit: File: Suggested fix: Comment out / gate the UCCL ASIO-runtime loop (lines 95-103) the same way the UCCL ETCD loop was disabled in PR #2000, referencing issue #1999. Additionally, wrap Related: PR #2000 (#2000), issue #1999 (referenced in the script), PR #1724 "UCCL crash Fix + GDB trap" (#1724). |
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage (node mizu02) was killed after a ~42-minute hang (exit 143 is the symptom); a UCCL-backend nixlbench worker deadlocked during engine teardown after the Full analysisSummary: Stage 356 "Run Nixlbench tests" on host mizu02 was ABORTED (SIGTERM, exit 143) because a UCCL nixlbench process hung during teardown. Root cause: After the Implicated commit: unknown (hang is in the UCCL backend engine teardown, not introduced by a specific commit in the touched history; PR #2012's own diff was not implicated by the log). UCCL teardown code last touched in f8e2221 "Enable local xfer for UCCL backend" (Pravein Govindan Kannan) and 63c494e "UCCL: Simplify and Optimize for batch transfers." File: UCCL backend engine shutdown path — Suggested fix: Investigate and fix the UCCL engine-destroy deadlock (the accept/ Related: Issue #1999 "UCCL nixlbench test hangs" (directly matches); PR #1724 "UCCL crash Fix + GDB trap"; PR #1963 "Fix race condition with ASIO runtime on shutdown" (related shutdown race pattern). |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The Full analysisSummary: Stage "Setup Image x86_64/nixl-wheel-base-manylinux_2_28/1" (node 40) failed while building the manylinux wheel-base container image. Root cause: During the image build, the step HTTP 503 from gitlab.gnome.org means GNOME's GitLab (Gitaly backend) was temporarily unavailable. The build was progressing continuously (AWS SDK compiled, gusli built) right up to this network operation — there was no hang; it failed fast on an external dependency. Nothing in the nixl codebase caused this. Implicated commit: None (external outage). The libxml2-from-source step was introduced in the wheel_base/wheel Dockerfile split — File: The manylinux wheel-base Dockerfile under Suggested fix:
Related: none (no matching nixl issue/PR for this 503; #1648 from the search is unrelated). |
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage was killed by Jenkins (exit 143) after hanging for ~36 minutes; the hang is in the UCCL backend's engine/process teardown, matching known issue #1999, and is unrelated to PR #2012 (a UCX-only refactor). Retry/quarantine the UCCL nixlbench case rather than raising the time limit. Full analysisSummary: Jenkins stage "Run Nixlbench tests" (node 356) was ABORTED with exit code 143 (SIGTERM) after a wall-clock kill; the process was hung, not slow. Root cause: After the UCCL Implicated commit: Not the PR under test. The UCCL backend last changed in File: Suggested fix: Treat this as infra/known-flake for PR #2012 — re-run the job; the PR is not the cause. For the real fix, track under issue #1999: add a hard timeout around each Related: Issue #1999 (UCCL nixlbench test hangs); PR #2012 (unrelated UCX refactor under test); UCCL history: #895, #1271, #1428, #1595.
|
|
🤖 CI Triage Agent — TL;DR: The Full analysisSummary: Jenkins aborted stage 299 ("Run Nixlbench tests", ucx-v1.22.x variant) after a UCCL benchmark hung; exit code 143 (SIGTERM) is the symptom of the timeout kill. Root cause: Not a slow test but a hang. Last application output was Implicated commit: 7a244b9 "CI: Skip UCCL nixlbench tests (#2000)" by ovidiusm — incomplete mitigation. File: Suggested fix: Also disable/comment out the ASIO UCCL loop at lines 95–103 (same way lines 124–132 were commented out under the reference to issue #1999) until #1999 is resolved. As a defense-in-depth measure against future hangs, wrap the Related: Issue #1999 (UCCL nixlbench test hangs); PR #2000 (partial skip). Failure surfaced on PR #2012 but is unrelated to its changes. |
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage was killed (exit 143) after hanging ~40 minutes inside a UCCL-backend nixlbench WRITE test — a known bug (issue #1999), and this PR branch is missing the merged fix (#2000) that skips those tests. Rebase PR #2012 onto latest main to pick up the UCCL skip, or exclude the UCCL nixlbench cases. Full analysisSummary: Jenkins stage 356 "Run Nixlbench tests" (ucx-v1.22.x variant) was aborted with exit code 143 after a UCCL nixlbench transfer hung. Root cause: Not a timeout-due-to-slowness. The last application log line is at Implicated commit: The hang is a pre-existing UCCL backend bug (not introduced by PR #2012's UCX-engine refactor). The relevant missing fix is File: Suggested fix: Rebase/merge PR #2012 onto current main so it includes PR #2000, which skips the flaky UCCL nixlbench tests. Do not raise the stage time limit — that would only make the hang take longer to fail. Track the underlying UCCL connection-setup hang via issue #1999. Related: Issue #1999 (UCCL nixlbench test hangs); PR #2000 (CI: Skip UCCL nixlbench tests); PR #1724 (UCCL crash fix + GDB trap); PR #2012 (the build under investigation). |
|
🤖 CI Triage Agent — TL;DR: The "Run Python tests" stage was killed (exit 143) after hanging ~9m52s with zero output at pip's Full analysisSummary: Stage 280 ( Root cause: In stage 280's log, the last application line is Implicated commit: No single code commit broke it; the pinned build requirement File: Suggested fix: Avoid pulling torch into the isolated build env during the wheel build. Either add Related: PR #1869 (Switch CI base image to pytorch + CUDA 13.3); PR #1972 (wheel/build-requires changes). No existing issue tracks this specific hang. |
What?
Backport #1906 to v1.4.0
To unify the code and reuse the same functions
It's pre-requisite for completions API PR
Summary by CodeRabbit
New Features
Documentation
Chores