Skip to content

CORE: Get local rank from topology if unset - #1245

Open
MaayanGadishNvidia wants to merge 1 commit into
openucx:masterfrom
MaayanGadishNvidia:NIC_bid_auto
Open

MaayanGadishNvidia wants to merge 1 commit into
openucx:masterfrom
MaayanGadishNvidia:NIC_bid_auto

Conversation

@MaayanGadishNvidia

Copy link
Copy Markdown

What

Adding automation for calculate local rank from topo if not provided by user

Why ?

Continue of #1189

@greptile-apps

greptile-apps Bot commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge.

Findings

  1. P2 Trailing whitespace added.
Summary

The PR adds a topo mode for automatically deriving a UCX node-local identifier from an initial context-ID and host-information exchange.

  • Adds parsing and formatting support for the topo configuration value.
  • Builds a temporary topology to derive the node-local rank.
  • Reuses the exchanged prefix when performing the subsequent full context-address exchange.
  • Extends context-creation error cleanup for topology and address-storage allocations.

Reviews (73) · Last reviewed commit: "CORE: Get local rank from topology if un..."

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
Comment thread src/components/topo/ucc_topo.h Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
Comment thread src/components/topo/ucc_topo.h Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c Outdated
Comment thread src/components/tl/ucp/tl_ucp_context.c Outdated
Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c
Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c Outdated
ucp_params.estimated_num_eps = params->estimated_num_eps;
}

#ifdef HAVE_UCX_NODE_LOCAL_ID

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace added.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c Outdated
Comment thread src/core/ucc_context.c

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c
Comment thread src/core/ucc_context.c
Comment thread src/core/ucc_context.c Outdated
@greptile-apps

greptile-apps Bot commented Jan 29, 2026

Copy link
Copy Markdown
Contributor
Additional Comments (1)

src/core/ucc_context.c
Added components/topo/ucc_topo.h include to support automatic local rank computation from topology

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/core/ucc_context.c Outdated
@janjust

janjust commented Feb 12, 2026

Copy link
Copy Markdown
Collaborator

/build

@janjust

janjust commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

/build

Comment thread src/core/ucc_context.c Outdated
addr_len == 0 iff storage == NULL. */
ucc_status_t status;
ucc_rank_t i;
const size_t ctx_addr_prefix_len =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declared and initialized vars go first.

Comment thread src/core/ucc_context.c Outdated
const size_t ctx_addr_prefix_len =
offsetof(ucc_context_addr_header_t, n_components);
ucc_context_addr_header_t *h;
void *scratch;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment of vars

Comment thread src/core/ucc_context.c
@janjust

janjust commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

/build

@janjust
janjust enabled auto-merge (squash) September 16, 2026 12:09
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentBuild & Test · commit 44ac5b45

TL;DR: The build broke with a hard compile error in src/core/ucc_context.c: the new ucc_core_ctx_id_exchange() declares ctx_id_prefix_len but references a non-existent ctx_addr_prefix_len; rename the uses (or the declaration) so they match.

Full analysis

Summary: make -j install of UCC failed while compiling core/libucc_la-ucc_context.lo — undeclared identifier plus -Werror=unused-variable, exit code 2.

Root cause: Commit 44ac5b4 ("CORE: Get local rank from topology if unset", PR #1245) added ucc_core_ctx_id_exchange() which declares the local constant as ctx_id_prefix_len at line 678, but every use inside the function (lines 709, 713, 716, 719, 720, 726) refers to ctx_addr_prefix_len, which is declared nowhere in the file. GCC reports:

  • core/ucc_context.c:709:40: error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'?
  • core/ucc_context.c:678:18: error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable]

Since UCC builds with -Wall -Werror, the unused-variable warning is also fatal. This is a pure rename/typo slip — it is not environment-related (the UCX prerequisite built and installed fine, and configure completed successfully with TL modules: < self ucp >).

Implicated commit: 44ac5b4 — Noam Tsemah, "CORE: Get local rank from topology if unset" (2026-08-19)

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709,713,716,719,720,726 (uses)

Suggested fix: Make the identifier consistent within ucc_core_ctx_id_exchange(). Simplest patch — keep the declaration and rename the six uses:

const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) +
                                 sizeof(ucc_host_info_t);
...
addr_storage->storage = ucc_malloc(
    (addr_storage->size + 1) * ctx_id_prefix_len, "ctx_ids_storage");
...
ucc_error("failed to allocate %zd bytes for ctx_ids storage",
          (addr_storage->size + 1) * ctx_id_prefix_len);
...
addr_storage->addr_len = ctx_id_prefix_len;
scratch = PTR_OFFSET(addr_storage->storage,
                     ctx_id_prefix_len * addr_storage->size);
memset(scratch, 0, ctx_id_prefix_len);
...
status = oob->allgather(scratch, addr_storage->storage,
                        ctx_id_prefix_len, oob->coll_info,
                        &addr_storage->oob_req);

(Alternatively rename the declaration to ctx_addr_prefix_len.) Also worth double-checking that sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t) is the intended stride rather than offsetof(ucc_context_addr_header_t, components), since the code casts the buffer to ucc_context_addr_header_t * and writes ctx_id/host_info — a mismatch there would be a latent buffer-overrun even after the build is fixed. Build locally with ./configure --enable-gtest before re-pushing, as -Werror makes any warning a CI failure.

Related: #1245 (the PR under test — this is a self-inflicted failure on its own HEAD commit, not a pre-existing master breakage)

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLinter · commit 44ac5b45

TL;DR: The Linter job's make step fails to compile src/core/ucc_context.c because the new ucc_core_ctx_id_exchange() function declares ctx_id_prefix_len but references ctx_addr_prefix_len in six places — an incomplete rename; make the identifier consistent.

Full analysis

Summary: GitHub Actions "Linter" run 35077588882 failed in the bear ... make -j build step with 6 clang-17 errors: use of undeclared identifier 'ctx_addr_prefix_len'; did you mean 'ctx_id_prefix_len'?, aborting core/libucc_la-ucc_context.lo (make Error 1 → job exit code 2).

Root cause: Pure compile-time bug, not environment-related. In ucc_core_ctx_id_exchange() the local constant is declared at line 678 as const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t);, but all subsequent uses (lines 709, 713, 716, 719, 720, 726) spell it ctx_addr_prefix_len, which is never declared. This looks like a variable rename that was only applied to the declaration. With -Wall -Werror and clang-17 this is a hard error, so the build (and thus the linter/compile-commands generation) never completes. Note the UCX prerequisite built fine — the failure is entirely in the ucc tree at the PR merge commit ef15b35 (PR head 44ac5b45).

Implicated commit: [REDACTED:Hex High Entropy String] — Noam Tsemah, "CORE: Get local rank from topology if unset" (most recent commit touching src/core/ucc_context.c)

File: src/core/ucc_context.c:678 (declaration) with broken references at :709, :713, :716, :719, :720, :726

Suggested fix: Pick one name and use it everywhere. Simplest is to rename the declaration to match the uses:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

or alternatively sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' over that function so all six references use the declared ctx_id_prefix_len. Also update the stale error message text if you keep "ctx_ids" wording, then rebuild locally with CC=clang-17 ./configure --enable-assert && make -j before pushing, since -Werror makes any such typo a build-breaker.

Related: PR #1245 (branch NIC_bid_auto) is the change under test; no pre-existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id bd0a505c-fb2e-4880-91a5-d7ba94427ce0 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentASAN · commit 44ac5b45

TL;DR: The ASAN job failed at compile time, not in the sanitizer run: src/core/ucc_context.c declares ctx_id_prefix_len but still uses the old name ctx_addr_prefix_len in 6 places, and -Werror/clang turns that into a hard error. Rename the 6 remaining uses (or the declaration) so they match.

Full analysis

Summary: make -j install failed in src/core/libucc_la-ucc_context.lo with "use of undeclared identifier 'ctx_addr_prefix_len'; did you mean 'ctx_id_prefix_len'?" (6 errors), aborting the ASAN build before any test ran.

Root cause: In ucc_core_ctx_id_exchange() the prefix-length local was renamed to ctx_id_prefix_len (declared at line 678), but the six references inside the function body were left as the pre-rename name ctx_addr_prefix_len:

  • line 709 — ucc_malloc((addr_storage->size + 1) * ctx_addr_prefix_len, ...)
  • line 713 — inside the ucc_error(...) allocation-failure message
  • line 716 — addr_storage->addr_len = ctx_addr_prefix_len;
  • line 719 — PTR_OFFSET(..., ctx_addr_prefix_len * addr_storage->size)
  • line 720 — memset(scratch, 0, ctx_addr_prefix_len)
  • line 726 — oob->allgather(..., ctx_addr_prefix_len, ...)

No such identifier exists in scope, so clang-17 errors out. This is a pure build break independent of ASAN — every compiler job on this branch will fail identically. Note the log also shows the UCC configure summary is missing CUDA/IB/ROCm (all "not found"), which is expected for this runner and unrelated.

Implicated commit: [REDACTED:Hex High Entropy String] — Noam Tsemah, "CORE: Get local rank from topology if unset" (the run's head commit, and the most recent commit touching this file)

File: src/core/ucc_context.c:678 (declaration) with unresolved uses at 709, 713, 716, 719, 720, 726

Suggested fix: Complete the rename — replace all six occurrences of ctx_addr_prefix_len with ctx_id_prefix_len in ucc_core_ctx_id_exchange():

addr_storage->storage = ucc_malloc(
    (addr_storage->size + 1) * ctx_id_prefix_len, "ctx_ids_storage");
...
ucc_error("failed to allocate %zd bytes for ctx_ids storage",
          (addr_storage->size + 1) * ctx_id_prefix_len);
...
addr_storage->addr_len = ctx_id_prefix_len;
scratch = PTR_OFFSET(addr_storage->storage,
                     ctx_id_prefix_len * addr_storage->size);
memset(scratch, 0, ctx_id_prefix_len);
status = oob->allgather(scratch, addr_storage->storage,
                        ctx_id_prefix_len, oob->coll_info,
                        &addr_storage->oob_req);

Before pushing, build locally (./autogen.sh && ./configure --enable-gtest && make -j) to confirm the whole file compiles — the parallel build aborted at the first error, so later files in src/core were never compiled and may hide additional fallout from the same rename.

Related: PR #1245 (branch NIC_bid_auto) — this is the PR under test; no pre-existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id bb1018de-5579-4671-9b76-eb5a8d8d6e2c in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLint (ROCm) · commit 44ac5b45

TL;DR: The build step of the "Lint (ROCm)" job failed to compile src/core/ucc_context.c — the new ucc_core_ctx_id_exchange() function declares a local ctx_id_prefix_len but references an undeclared ctx_addr_prefix_len in six places. Rename the six uses to ctx_id_prefix_len (or rename the declaration back to ctx_addr_prefix_len) so the identifiers match.

Full analysis

Summary: bear -- make -j failed with 6 × error: use of undeclared identifier 'ctx_addr_prefix_len' in core/ucc_context.c, aborting the Lint (ROCm) job with exit code 2 (no compile_commands.json, so lint never ran).

Root cause: In ucc_core_ctx_id_exchange() the size constant is declared at line 678 as const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t);, but the body still uses the old name ctx_addr_prefix_len at lines 709, 713, 716, 719, 720 and 726. This is an incomplete rename — with -Wall -Werror under clang-17 it's a hard error. Not a ROCm/environment issue: UCX built fine, configure succeeded (ROCM/RCCL support: yes), and the failure is a pure C compile error in the UCC core, so this will break every compiling job on this branch.

Implicated commit: 44ac5b45 — "CORE: Get local rank from topology if unset", Noam Tsemah (the only recent commit touching this file, and the commit under test)

File: src/core/ucc_context.c:709 (also :713, :716, :719, :720, :726)

Suggested fix: Make the identifier consistent — simplest is a mechanical rename of the six usages:

sed -i '673,730s/ctx_addr_prefix_len/ctx_id_prefix_len/g' src/core/ucc_context.c

i.e. ucc_malloc((addr_storage->size + 1) * ctx_id_prefix_len, "ctx_ids_storage"), the matching ucc_error(...) size argument, addr_storage->addr_len = ctx_id_prefix_len;, the PTR_OFFSET(...)/memset(...) pair, and the oob->allgather(..., ctx_id_prefix_len, ...) call. Then verify locally with CC=clang-17 ./configure ... && make -j before pushing, since -Werror turns this into a full build failure.

Related: none found (no existing issue/PR references ctx_addr_prefix_len); PR #1245 is the failing change.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentLinter-NVIDIA · commit 44ac5b45

TL;DR: The UCC build in the Linter-NVIDIA job failed to compile src/core/ucc_context.c because the new ucc_core_ctx_id_exchange() function declares ctx_id_prefix_len but references a nonexistent ctx_addr_prefix_len in 6 places; rename the uses (or the declaration) so the names match.

Full analysis

Summary: make step of Linter-NVIDIA failed with 6 clang-17 errors: use of undeclared identifier 'ctx_addr_prefix_len'; did you mean 'ctx_id_prefix_len'? in core/ucc_context.c, aborting the build (make: *** [Makefile:541: all] Error 2).

Root cause: A pure compile error from an inconsistent variable rename in the PR's new ucc_core_ctx_id_exchange(). The local is declared at line 678 as const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t);, but the body still refers to the old name ctx_addr_prefix_len at lines 709, 713, 716, 719, 720 and 726. Since UCC builds with -Wall -Werror under clang-17, the translation unit fails and core/libucc_la-ucc_context.lo cannot be built. This is not a flake, an environment issue, or a timeout — the UCX prerequisite built and installed fine, configure succeeded (TL modules < cuda mlx5 self sharp ucp >), and the failure is deterministic at compile time.

Implicated commit: [REDACTED:Hex High Entropy String] — Noam Tsemah, "CORE: Get local rank from topology if unset" (the PR #1245 head commit, which is the most recent change to src/core/ucc_context.c)

File: src/core/ucc_context.c:678 (declaration) with errors at :709, :713, :716, :719, :720, :726

Suggested fix: Make the identifier consistent throughout the function — simplest is to replace all six ctx_addr_prefix_len occurrences with ctx_id_prefix_len:

sed -i '673,740s/ctx_addr_prefix_len/ctx_id_prefix_len/g' src/core/ucc_context.c

(Alternatively rename the line-678 declaration to ctx_addr_prefix_len, but ctx_id_prefix_len is the better name since the buffer holds ucc_context_addr_header_t ctx-id prefixes and matches the surrounding UCC_ADDR_STORAGE_FLAG_CTX_ID_PREFIX / "ctx_ids_storage" naming.) Before pushing, do a local ./configure && make -j — the error reproduces immediately and requires no GPU or Mellanox hardware, so it should have been caught pre-push. Worth also grepping the PR diff for other partial renames of the same variable in ucc_core_addr_exchange().

Related: PR #1245 (branch NIC_bid_auto) — this is a self-inflicted failure in the PR under test, not a pre-existing master breakage; master commit [REDACTED:Hex High Entropy String] was the merge base.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 5667f39d-6f2f-4a93-8e74-374657c9bb38 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-build-hpcsdk · commit ef15b357

TL;DR: The hpcsdk (nvc) build failed compiling src/core/ucc_context.c because ucc_core_ctx_id_exchange() declares a local named ctx_id_prefix_len but references it as ctx_addr_prefix_len — an incomplete rename in the PR under test; make the declaration and all uses use the same name.

Full analysis

Summary: Stage "Build UCC" failed: nvc compile error #20 "identifier ctx_addr_prefix_len is undefined" (plus #177 unused-variable, escalated by -Werror) in core/ucc_context.c, aborting make install.

Root cause: In ucc_core_ctx_id_exchange() the prefix-stride constant is declared at line 678 as const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t); but every use inside the allocation/allgather block (lines 709, 713, 716, 718–720, 726) spells it ctx_addr_prefix_len. No such identifier exists anywhere in the translation unit, so the compiler reports an undefined identifier, and the declared-but-never-referenced ctx_id_prefix_len triggers diagnostic #177 which -Werror turns fatal. This is a plain incomplete-rename typo in the new ctx-id-prefix exchange code introduced by the commit under test (ef15b35 / PR #1245); it is not an nvc-specific or environment issue — the Parallel stage failed at the same point.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #1245, new ucc_core_ctx_id_exchange code path; most recent history on this file is 44ac5b4 "CORE: Get local rank from topology if unset", Noam Tsemah)

File: src/core/ucc_context.c:678 (declaration) and :709, :713, :716, :718, :720, :726 (uses)

Suggested fix: Pick one name and use it consistently — simplest is to rename the declaration to match the uses:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

or alternatively sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' within that function. After fixing, re-run a -Werror build locally (./configure ... && make -j) since this would break every compiler, not just nvc. Worth also grepping the PR diff for other renamed identifiers, and confirming the addr_storage->addr_len stride set at line 716 matches the value used by ucc_core_addr_exchange().

Related: PR #1245 (the change under test); no existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id e0cac72d-f9e5-4510-9138-873390ce4745 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-run-coverity · commit 44ac5b45

TL;DR: The Coverity job failed in cov-build make, not in Coverity analysis: src/core/ucc_context.c doesn't compile because the new ucc_core_ctx_id_exchange() declares ctx_id_prefix_len but uses ctx_addr_prefix_len throughout the body. Rename the variable consistently (one line change) to fix the build.

Full analysis

Summary: Stage "Run Coverity" (node 53) failed at cov-build --dir ... make -j16 all with a GCC error compiling core/libucc_la-ucc_context.lo.

Root cause: Compile error, not a Coverity defect report:

  • core/ucc_context.c:709:40: error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'?
  • core/ucc_context.c:678:18: error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable]

In ucc_core_ctx_id_exchange() the local constant is declared at line 678 as ctx_id_prefix_len, but every use in the function body (lines 709, 713, 716, 719, 720, 726) refers to ctx_addr_prefix_len, which does not exist. Since the project builds with -Wall -Werror, both the undeclared identifier and the resulting "unused variable" are hard errors and make exits 2, which coverity.sh's handle_error propagates as the stage failure. This is an incomplete rename in the commit that added this function.

Implicated commit: [REDACTED:Hex High Entropy String] — "CORE: Get local rank from topology if unset", Noam Tsemah (the exact commit under test, PR #1245)

File: src/core/ucc_context.c:678 (declaration) and :709, :713, :716, :719, :720, :726 (uses)

Suggested fix: Make the name consistent — simplest is to rename the declaration at line 678:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

(or alternatively sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' over the function body). Build locally with ./configure && make -j before re-triggering; a plain make reproduces this without Coverity. Also worth checking the rest of the new function for the same rename slip (e.g. any log messages / addr_len assignments referring to the old name).

Related: PR #1245 (the PR under test); no pre-existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 59fd5512-7974-4479-ac53-c01112db1c0d in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-run-coverity · commit ef15b357

TL;DR: The Coverity build failed at compile time, not in analysis: src/core/ucc_context.c uses ctx_addr_prefix_len inside ucc_core_ctx_id_exchange() but the local is declared as ctx_id_prefix_len — an incomplete rename. Rename the identifier consistently (declare ctx_addr_prefix_len or update the 6 use sites) so the build compiles under -Werror.

Full analysis

Summary: cov-build ... make -j16 all failed with error: 'ctx_addr_prefix_len' undeclared in core/ucc_context.c:709, plus -Werror=unused-variable on ctx_id_prefix_len, aborting the Coverity stage with exit code 2.

Root cause: In ucc_core_ctx_id_exchange() the prefix-stride local is declared at line 678 as const size_t ctx_id_prefix_len, but every use inside the allocation/allgather block (lines 709, 713, 716, 719, 720, 726) refers to ctx_addr_prefix_len. That symbol does not exist in this scope, so GCC reports an undeclared identifier; because the declared variable is then never read, -Wunused-variable fires too, and -Werror (enabled in the configure line: gcc -O3 ... -Wall -Werror) turns both into hard errors. This is a plain incomplete rename in the PR under test — the failure is deterministic and unrelated to Coverity itself or to infrastructure (config/autogen/configure all succeeded, TLs cuda mlx5 nccl self sharp ucp configured fine). Note only line 709 is flagged for the undeclared identifier because GCC reports each undeclared name once per function; fixing the name resolves all six sites.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #1245, the commit under test — the ucc_core_ctx_id_exchange prefix-exchange code is new/renamed here; last upstream commits to this file are 44ac5b4 "CORE: Get local rank from topology if unset" by Noam Tsemah and a23fe6a by Ferrol Aderholdt, neither of which contains this identifier mismatch)

File: src/core/ucc_context.c:678 (declaration) and :709, :713, :716, :719, :720, :726 (uses)

Suggested fix: Pick one name and apply it everywhere in ucc_core_ctx_id_exchange(). Simplest patch — rename the declaration to match the uses:

-    const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) +
-                                     sizeof(ucc_host_info_t);
+    const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
+                                       sizeof(ucc_host_info_t);

(or conversely sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' over that function). Then verify locally with the same flags the CI uses before re-running: ./autogen.sh && ./configure --with-nccl --with-tls=cuda,nccl,self,sharp,shm,ucp,mlx5 ... && make -j16 all, since -Werror means any stray warning also breaks this job. Worth double-checking that ctx_addr_prefix_len isn't intended to be a different value than the local (e.g. a struct field or a header-level constant) — if the header prefix layout changed, confirm the size expression matches ucc_context_addr_header_t so the PTR_OFFSET/memset stride at lines 718-720 stays correct.

Related: PR #1245 (the change under test); no existing issue found matching this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 3adbd0f5-b7da-4856-a5c6-496cd05a939a in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-build-hpcsdk · commit 44ac5b45

TL;DR: The Build UCC stage failed compiling src/core/ucc_context.c — the new ucc_core_ctx_id_exchange() declares ctx_id_prefix_len but references ctx_addr_prefix_len, an undefined identifier; rename the uses (or the declaration) so they match.

Full analysis

Summary: nvc compile of core/libucc_la-ucc_context.lo failed with error #20: identifier "ctx_addr_prefix_len" is undefined (plus error #177: variable "ctx_id_prefix_len" was declared but never referenced, fatal under -Werror), aborting make install.

Root cause: In ucc_core_ctx_id_exchange() the prefix-stride constant is declared as const size_t ctx_id_prefix_len (line 678) but every use site inside the allocation/allgather block refers to ctx_addr_prefix_len (lines 709, 713, 716, 719, 720, 726). No such symbol exists in the file or headers, so the identifier is undefined and the declared variable is simultaneously unused — a leftover naming mismatch from a rename in the commit under test. This is a straight source bug, not an environment/toolchain issue; the hpcsdk job just surfaces it first because nvc builds with -Wall -Werror --display_error_number (the declared_but_not_referenced remark is promoted to an error).

Implicated commit: [REDACTED:Hex High Entropy String] — Noam Tsemah, "CORE: Get local rank from topology if unset" (the commit this build polled; it is the most recent change to src/core/ucc_context.c)

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:708-709, 713, 716, 718-720, 725-727 (uses)

Suggested fix: Make the names consistent — either rename the declaration to ctx_addr_prefix_len:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

or, preferably (keeps the "ctx id" semantics of the new function), replace all six ctx_addr_prefix_len occurrences with ctx_id_prefix_len. Then rebuild locally with -Werror (ideally once with nvc/hpcsdk, since GCC-only pre-merge checks can differ in warning promotion) before re-triggering UCC/ucc-build-hpcsdk.

Related: PR #1245 (branch NIC_bid_auto) is the change under test; no existing issue mentions ctx_addr_prefix_len.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 2165f432-0841-40b1-9aab-f944739d7c58 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-gtest · commit ef15b357

TL;DR: The build (not the gtest run) failed: src/core/ucc_context.c uses an undeclared identifier ctx_addr_prefix_len while the local variable is declared as ctx_id_prefix_len, so make aborted with -Werror. Rename the declaration (or the four uses) so they match.

Full analysis

Summary: Stage "Build Docker" (.ci/Dockerfile.nvls, STEP 27 → .ci/scripts/build_ucc.sh) failed compiling core/libucc_la-ucc_context.lo; the pipeline never reached gtest.

Root cause: In ucc_core_ctx_id_exchange() the prefix-stride variable is declared as ctx_id_prefix_len (line 678) but every use in the function body refers to ctx_addr_prefix_len (lines 709, 713, 716, 719, 720, 726). GCC reports two hard errors:

  • error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'? at ucc_context.c:709
  • error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable] at ucc_context.c:678

Since UCC configures with -Wall -Werror, the unused-variable warning is also fatal. This is a pure source-level rename mismatch — an incomplete identifier rename in the PR under test, not an environment or infrastructure issue. (Note the NVLS matrix entry also builds with UCC_ENABLE_NVLS=no and only sm_80/sm_86 gencodes, so NVLS/NCCL/SHARP/MLX5 get disabled by configure, but that is unrelated to the failure.)

Implicated commit: [REDACTED:Hex High Entropy String] (PR #1245 head); the most recent upstream commit to this file is 44ac5b4 "CORE: Get local rank from topology if unset" by Noam Tsemah, but the mismatched identifier is introduced by the PR's changes to ucc_core_ctx_id_exchange.

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709,713,716,719,720,726 (uses)

Suggested fix: Make the names consistent — simplest is to rename the declaration at line 678:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

(or, alternatively, change the six body references to ctx_id_prefix_len). Then rebuild locally with ./configure --enable-gtest && make -j before pushing — the failure is deterministic and reproducible without a GPU, since it is caught at compile time. Consider adding a fast compile-only job to the matrix so -Werror breakages fail in seconds rather than after a 4-minute Docker image build.

Related: none found (no matching issues/PRs for ctx_addr_prefix_len / ucc_core_ctx_id_exchange).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 98c7d6f1-27a4-4b34-b313-f1684922b6eb in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-gtest · commit 44ac5b45

TL;DR: The Build Docker stage failed because src/core/ucc_context.c doesn't compile — ucc_core_ctx_id_exchange() declares ctx_id_prefix_len but references an undeclared ctx_addr_prefix_len (lines 709/713/716/719/720/726); rename the uses (or the declaration) so the identifier matches.

Full analysis

Summary: UCC/ucc-test-gtest #150 failed in the "Build Docker" stage at RUN ${SRC_DIR}/ucc/.ci/scripts/build_ucc.sh (STEP 27/31) — make -j16 install aborted compiling core/libucc_la-ucc_context.lo.

Root cause: A pure compile error introduced by the commit under test. In ucc_core_ctx_id_exchange() the local variable is declared as const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t); (line 678), but every use in the function body refers to ctx_addr_prefix_len (lines 709, 713, 716, 719, 720, 726). GCC reports:

  • error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'?
  • error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable]

Since UCC is configured with -Wall -Werror, the unused-variable warning is also fatal. This is a half-finished rename — nothing environment- or timing-related; the build never got as far as the gtest stage, which is why the downstream pipline stop on build_helper / pipeline_stop stages also went red.

Implicated commit: [REDACTED:Hex High Entropy String] — "CORE: Get local rank from topology if unset", Noam Tsemah (2026-08-19), the head commit of branch NIC_bid_auto / PR #1245.

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709 (first bad use; also 713, 716, 719, 720, 726)

Suggested fix: Make the identifier consistent — simplest is to rename the declaration to match the body:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

(or alternatively sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' over the function body). Then rebuild locally with ./autogen.sh && ./configure --enable-gtest && make -j before re-pushing — the -Werror build would have caught this immediately. Worth also confirming the intended prefix size is indeed sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t) and that it matches the ucc_context_addr_header_t layout written at lines 721-723, since the allocation stride and the allgather element size both depend on it.

Related: PR #1245 (branch NIC_bid_auto) is the change under test; no existing issue found matching this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id ab944167-84be-4705-adb9-6e67757e2b96 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit ef15b357

TL;DR: The Build Docker stage failed compiling src/core/ucc_context.c — a local variable is declared as ctx_id_prefix_len but referenced as ctx_addr_prefix_len inside ucc_core_ctx_id_exchange(), an incomplete rename in PR #1245; rename the declaration (or the 6 uses) so they match.

Full analysis

Summary: UCC/ucc-test-mpi #61 failed in the Build Docker stage (Dockerfile.ngc_pytorch, RUN .ci/scripts/build_ucc.sh) with a hard compile error in core/libucc_la-ucc_context.lo.

Root cause: Pure source/compile error, not infrastructure. In ucc_core_ctx_id_exchange() the prefix-stride constant is declared at line 678 as ctx_id_prefix_len, but every use in the function body (lines 709, 713, 716, 719, 720, 726) refers to ctx_addr_prefix_len, which does not exist. GCC reports:

  • src/core/ucc_context.c:709:40: error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'?
  • src/core/ucc_context.c:678:18: error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable]

Because UCC configures with -Wall -Werror, the unused-variable warning is also fatal, so the build dies at make -j16 installError 1. This is an incomplete identifier rename introduced by the commit under test (the function ucc_core_ctx_id_exchange and UCC_ADDR_STORAGE_FLAG_CTX_ID_PREFIX are new relative to the last upstream commits on this file). No stage-timeout or hang is involved — the failure occurred ~20 s after make started.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #1245); the most recent upstream commit on this file is 44ac5b4 "CORE: Get local rank from topology if unset" (Noam Tsemah), which does not contain this code.

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709, 713, 716, 719, 720, 726 (uses)

Suggested fix: Make the names consistent — simplest is to rename the declaration at line 678:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

(or alternatively sed-replace the six ctx_addr_prefix_len uses with ctx_id_prefix_len). Then verify locally with the same flags the CI uses before pushing:

./autogen.sh && ./configure --with-ucx=... --with-cuda=... --enable-gtest && make -j

Note that -Werror is enabled in this configuration, so any leftover unused variable will still break the build. It would also be worth adding a fast compile-only job ahead of the Docker/MPI matrix so this class of error fails in seconds rather than after a ~5-minute base-image pull.

Related: PR #1245 (the change under test). No existing issue found matching ctx_addr_prefix_len.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id f210621e-a5f9-49e9-ad72-b68ad534d086 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit 44ac5b45

TL;DR: The Docker build stage failed because src/core/ucc_context.c doesn't compile: the new ucc_core_ctx_id_exchange() declares ctx_id_prefix_len but references ctx_addr_prefix_len; rename the uses (or the declaration) so they match.

Full analysis

Summary: UCC/ucc-test-mpi #61 failed in the "Build Docker" stage (node 76) at STEP 17/20: RUN ${SRC_DIR}/ucc/.ci/scripts/build_ucc.shmake -j16 install aborted compiling core/libucc_la-ucc_context.lo.

Root cause: Genuine compile error, not infra. In ucc_core_ctx_id_exchange() the local size variable is declared as ctx_id_prefix_len (line 678) but every use in the function body refers to ctx_addr_prefix_len (lines 709, 713, 716, 718–720, 726). GCC reports:

  • error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'? (line 709)
  • error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable] (line 678)

UCC builds with -Wall -Werror, so the unused-variable warning is also fatal. The function is new in the commit under test, so this code has never compiled — likely an incomplete rename during a rebase/refactor.

Implicated commit: [REDACTED:Hex High Entropy String] — "CORE: Get local rank from topology if unset", Noam Tsemah (the HEAD commit of branch NIC_bid_auto, PR #1245)

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709 (first bad use; also 713, 716, 718–720, 726)

Suggested fix: Make the identifier consistent — simplest is to rename the declaration to the name the body uses:

    const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                       sizeof(ucc_host_info_t);

(or, alternatively, sed-replace all ctx_addr_prefix_len occurrences inside ucc_core_ctx_id_exchange() with ctx_id_prefix_len). Verify the whole function compiles — check for any other stale identifiers from the same rename — then re-run the pipeline. Adding a plain ./autogen.sh && ./configure && make -j compile check to the PR gate would catch this class of failure before the Docker/MPI stages.

Related: PR #1245 (branch NIC_bid_auto); no existing issue found for this error signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id b04bf95c-98c5-4292-82c8-f534bf8f32ba in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-nvls · commit ef15b357

TL;DR: The Build Docker stage failed compiling src/core/ucc_context.c — a variable was declared as ctx_id_prefix_len but every use in the function body says ctx_addr_prefix_len (an incomplete rename), which is a hard error under -Werror. Fix by making the declaration and the uses agree on one name.

Full analysis

Summary: UCC build inside the NVLS Docker image failed at make -j16 install with error: 'ctx_addr_prefix_len' undeclared in ucc_core_ctx_id_exchange(), aborting the RUN .../build_ucc.sh Dockerfile step (exit status 2).

Root cause: Pure compile error from an inconsistent identifier rename in ucc_core_ctx_id_exchange(). The local is declared at line 678 as:

const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t);

but the function body references ctx_addr_prefix_len at lines 709, 713, 716, 718, 719, 720 and 726. GCC reports two -Werror failures:

  • ucc_context.c:709:40: error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'?
  • ucc_context.c:678:18: error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable]

The build is configured with -Wall -Werror, so the unused-variable warning is fatal too. This is not environment-related and not NVLS-specific — it will break every build target that compiles core; the NVLS job just happens to be the one reported. Note the second parallel Build Docker branch (stage 54) succeeded because it did not reach/compile the same configuration.

Implicated commit: [REDACTED:Hex High Entropy String] (PR #1245) — this hunk is not present in any of the prior commits to the file (latest being 44ac5b45, Noam Tsemah, "CORE: Get local rank from topology if unset").

File: src/core/ucc_context.c:678 (declaration) and :709, :713, :716, :718–:720, :726 (uses)

Suggested fix: Pick one name and use it consistently. Minimal change — rename the declaration to match the body:

const size_t ctx_addr_prefix_len = sizeof(ucc_context_id_t) +
                                   sizeof(ucc_host_info_t);

(or, alternatively, sed -i 's/ctx_addr_prefix_len/ctx_id_prefix_len/g' over the body of ucc_core_ctx_id_exchange, which reads more accurately since the value is the ctx-id prefix stride, not a full address length). Then rebuild locally with ./configure --with-tls=cuda,ucp --with-nvls and make -j before re-pushing; a plain make on any config would have caught this pre-submit.

Related: none found (searched for ctx_addr_prefix_len ucc_core_ctx_id_exchange — no matching issues/PRs).

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 445234a8-6c2a-4fd8-9d98-af49de425ab8 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-nvls · commit 44ac5b45

TL;DR: The Docker build stage failed compiling src/core/ucc_context.c — the new ucc_core_ctx_id_exchange() declares its prefix-length local as ctx_id_prefix_len but the function body still references the old name ctx_addr_prefix_len, so -Werror turns "undeclared identifier" + "unused variable" into a hard build error. Rename the variable (or its uses) so they match, ideally reusing offsetof(ucc_context_addr_header_t, n_components).

Full analysis

Summary: UCC/ucc-test-nvls #323 failed in the "Build Docker" stage at STEP 27/31: RUN .../.ci/scripts/build_ucc.shmake -j16 install aborted on a compile error in core/libucc_la-ucc_context.lo.

Root cause: Variable-name mismatch introduced by the commit under test. In ucc_core_ctx_id_exchange() the constant is declared as ctx_id_prefix_len (line 678) while every use inside the function (lines 709, 713, 716, 719, 720, 726) still says ctx_addr_prefix_len — a name that only exists as a local in the sibling function ucc_core_addr_exchange_after_ctx_id() (line 614). GCC therefore reports:

  • error: 'ctx_addr_prefix_len' undeclared (first use in this function); did you mean 'ctx_id_prefix_len'? at ucc_context.c:709
  • error: unused variable 'ctx_id_prefix_len' [-Werror=unused-variable] at ucc_context.c:678

Since UCC configures with -Wall -Werror, this fails the build outright. This is a plain compile break, not an NVLS/environment issue (configure completed fine: NVLS enabled, TLs < cuda ucp >).

Secondary concern worth fixing at the same time: the two functions compute the prefix length differently — ucc_core_addr_exchange_after_ucc_ctx_id uses offsetof(ucc_context_addr_header_t, n_components) (line 614) and is compared against addr_storage->addr_len at line 634, while the new code sets addr_len = sizeof(ucc_context_id_t) + sizeof(ucc_host_info_t). If the struct has any padding before n_components, that equality check will silently fail at runtime even after the build is fixed.

Implicated commit: [REDACTED:Hex High Entropy String] — "CORE: Get local rank from topology if unset", Noam Tsemah (the exact commit this build polled)

File: src/core/ucc_context.c:678 (declaration) and src/core/ucc_context.c:709,713,716,719,720,726 (uses)

Suggested fix: Make the name consistent, and share the length definition between both functions. Minimal patch:

 ucc_status_t ucc_core_ctx_id_exchange(ucc_context_t *context, ucc_oob_coll_t *oob,
                                       ucc_addr_storage_t *addr_storage)
 {
-    const size_t ctx_id_prefix_len = sizeof(ucc_context_id_t) +
-                                     sizeof(ucc_host_info_t);
+    const size_t ctx_addr_prefix_len =
+        offsetof(ucc_context_addr_header_t, n_components);

Using offsetof(...) (identical to line 614) both fixes the compile error and guarantees the addr_storage->addr_len == ctx_addr_prefix_len check in ucc_core_addr_exchange_after_ctx_id() matches. Better still, hoist it into a file-scope #define UCC_CTX_ADDR_PREFIX_LEN offsetof(ucc_context_addr_header_t, n_components) so the two call sites can't drift again. Please also build locally with -Werror (or run the plain non-NVLS x86 build job) before re-pushing, since this break is arch- and NVLS-independent.

Related: PR #1245 (branch NIC_bid_auto) is the change under test; no existing issue found mentioning ctx_addr_prefix_len.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 3eab70cb-b092-45ce-9bd6-21f2afb0cb52 in the triage console for the audit trail.

auto-merge was automatically disabled September 16, 2026 12:23

Head branch was pushed to by a user without write access

@janjust

janjust commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

/build

@janjust
janjust enabled auto-merge (squash) September 16, 2026 13:19
@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit be1f0004

TL;DR: The Run UCC MPI tests (bulk) stage was killed at the wall clock (exit 143) because ucc_test_mpi deadlocked for ~8 minutes right after reporting 4 wrong-result failures in the thread-multiple run of CL/HIER split_rail + pipeline allreduce; the root cause is the unsynchronized fragment-restart / dependency bookkeeping in ucc_schedule_pipelined.c, not PR #1245.

Full analysis

Summary: UCC/ucc-test-mpi #62, stage 198 "Run UCC MPI tests (bulk)" aborted (exit 143) after ucc_test_mpi -T ... -c allreduce with UCC_CL_HIER_TUNE=allreduce:@split_rail:inf + UCC_CL_HIER_ALLREDUCE_SPLIT_RAIL_PIPELINE=thresh=0:fragsize=256K produced data-validation failures and then hung.

Root cause: Log evidence:

  • Last application output at 13:46:49.163:
    FAILURE in: tc=Allreduce team=half msgsize=262144 inplace=0 persistent=0 dt=uint16 op=max and 3× tc=Allreduce team=reverse msgsize=64 ... dt=uint64 op=max.
  • Next line is 13:54:42.529 Sending interrupt signal to process → a 7 m 53 s gap with zero output, i.e. the process was hung, not slow. The 143/SIGTERM is only the symptom.
  • The identical test set passed in the single-threaded pass (INFO: CL/HIER+split_rail+pipeline ... DONE at 13:45:14) and CL/HIER+split_rail without pipelining passed in the -T pass too. So the failure is specific to THREAD_MULTIPLE and n_frags > 1.

That combination points at the pipelined-schedule restart path. In src/schedule/ucc_schedule_pipelined.c:

  • The restart loop (lines 89–117) runs under schedule->lock and does frag->tasks[i]->n_deps += n_deps_base (monotonic accumulation, n_deps_satisfied deliberately not reset), then calls ucc_frag_start_handler() which runs frag_setup() (buffer offsets/counts) and posts the frag.
  • ucc_dependency_handler() (lines 279–300) is driven by the previous fragment's UCC_EVENT_TASK_STARTED/COMPLETED from any progress thread and does not take schedule->lock. It does an atomic fadd32 on n_deps_satisfied but reads the non-atomically-updated n_deps and posts when n_deps == n_deps_satisfied + 1.

Both interleavings are harmful and match the two observed symptoms exactly:

  1. Dependency fires before the restart bumped n_deps → the equality never holds → the fragment is never posted → the pipeline stalls forever → hang.
  2. Dependency fires before frag_setup() re-computed the frag's counts/displs and buffer offsets (allreduce_split_rail.c:36-97 mutates the frag tasks' bargs in place) → the reduce_scatterv/allgatherv run against stale offsets → wrong data (the op=max mismatches).

Secondary inconsistency in the same area: restart resets the internal status (frag->super.status, line 102) while ucc_schedule_pipelined_post() resets the user-visible one (frags[i]->super.super.status, line 160), so a restarted fragment can still look completed to a concurrent observer.

Implicated commit: c8314d31 — Sergey Lebedev, "SCHEDULE: fix task restart in ppln schedule (#1262)" (introduced the UCC_COLL_TASK_FLAG_RESTART_PENDING / n_deps += restart scheme). Pre-existing on master; not 49de1ae3 (Noam Tsemah, PR #1245) — its new code path is gated on UCC_NODE_LOCAL_ID=topo (src/core/ucc_context.c:930), and .ci/scripts/run_tests_ucc_mpi.sh never sets that variable, so the default auto behaviour is unchanged.

File: src/schedule/ucc_schedule_pipelined.c:104-107 and :285-290 (with src/components/cl/hier/allreduce/allreduce_split_rail.c:36-97 as the victim)

Suggested fix:

  1. Make dependency bookkeeping race-free for pipelined schedules: update n_deps atomically (ucc_atomic_add32) and either take schedule->lock in ucc_dependency_handler() when the parent has UCC_COLL_TASK_FLAG_IS_PIPELINED_SCHEDULE, or publish the restart state with a release barrier after frag_setup() and the n_deps bump, so no dependent task can be posted with stale args/counters.
  2. Make the restart path reset exactly the same fields as ucc_schedule_pipelined_post() (frag->super.super.status, n_deps, n_deps_satisfied) instead of the current mixed internal/external reset.
  3. CI hygiene (does not fix the bug, but stops one hang from burning the whole stage): wrap each mpirun in .ci/scripts/run_tests_ucc_mpi.sh with timeout <N> so a deadlock fails the step with a diagnosable error instead of a 15-minute silent Jenkins abort; optionally skip CL/HIER+split_rail+pipeline in the -T pass until item 1 lands.
  4. Re-run the PR to confirm it is unrelated — the failure is expected to reproduce on master.

Related: PR #1245 (trigger only, #1245), PR #1262 (restart logic), PR #428 (original split_rail allreduce), PR #616 "SCHEDULE: fixes race on n_deps_satisfied" (same class of bug)

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-mpi · commit 49de1ae3

TL;DR: The Run UCC MPI tests (bulk) stage was killed (exit 143) after the CL/HIER+split_rail+pipeline allreduce test in thread-multiple mode produced 4 data-validation FAILUREs and then deadlocked for ~8 minutes; this is a pipelined-schedule fragment-restart/dependency race in CL/HIER split_rail allreduce, not a consequence of the commit under test.

Full analysis

Summary: Stage 198 "Run UCC MPI tests (bulk)" hung and was aborted by Jenkins (script returned exit code 143) while running ucc_test_mpi -c allreduce -T with UCC_CLS=basic,hier, UCC_CL_HIER_TUNE=allreduce:@split_rail:inf, UCC_CL_HIER_ALLREDUCE_SPLIT_RAIL_PIPELINE=thresh=0:fragsize=256K.

Root cause: Evidence from the stage log:

  • Last application output at 13:46:49:
    FAILURE in: tc=Allreduce team=half    msgsize=262144 inplace=0 persistent=0 dt=uint16 op=max
    FAILURE in: tc=Allreduce team=reverse msgsize=64     inplace=0 persistent=0 dt=uint64 op=max  (x3)
    
  • Then zero output for 7 min 53 s (13:46:49 → 13:54:42 "Sending interrupt signal"). The largest inter-line gap is essentially the whole remaining runtime, so the job was hung, not slow — the timeout is a symptom.
  • The hang is in the pipelined split_rail allreduce: the immediately preceding non-pipelined CL/HIER+split_rail run passed 2240/2240, and the same pipelined config passed in the first (single-threaded) pass of the loop in .ci/scripts/run_tests_ucc_mpi.sh. The only delta for the failing run is -T (thread_mode = UCC_THREAD_MULTIPLE, test/mpi/main.cc:570).

Both symptoms (wrong reduction results, then a stalled fragment) point at the pipelined-schedule fragment restart bookkeeping used by split_rail: in ucc_schedule_pipelined_completed_handler the restart loop mutates frag->tasks[i]->n_deps with a plain read-modify-write while holding schedule->lock, whereas ucc_dependency_handler bumps n_deps_satisfied with ucc_atomic_fadd32 without that lock and fires task->post() when n_deps == n_deps_satisfied + 1. A stale/torn n_deps observation either posts the NET allreduce before the node reduce-scatter has finished (→ wrong data, exactly the op=max mismatches seen) or never posts it (→ the deadlock). This restart logic is the code most recently reworked in this area.

Importantly, this is not attributable to the commit under test: 49de1ae3 only activates its new node-local-rank discovery when UCC_NODE_LOCAL_ID=topo (src/core/ucc_context.c:930); the default auto maps to UCC_ULUNITS_AUTO (src/utils/ucc_parser.c:1277), and neither .ci/scripts/env.sh nor the mpirun lines in the log set that variable, so the new path is dormant.

Implicated commit: c8314d31 — "SCHEDULE: fix task restart in ppln schedule (#1262)", Sergey Lebedev (pipelined restart logic). The build's head commit 49de1ae3 (Noam Tsemah, "CORE: Get local rank from topology if unset") is not implicated.

File: src/schedule/ucc_schedule_pipelined.c:89-117 (restart loop, esp. frag->tasks[i]->n_deps += n_deps_base at line 105) and src/schedule/ucc_schedule_pipelined.c:285-297 (ucc_dependency_handler); exercised via src/components/cl/hier/allreduce/allreduce_split_rail.c:36 (..._frag_setup).

Suggested fix:

  1. Make dependency bookkeeping consistent under UCC_THREAD_MULTIPLE: use ucc_atomic_add32 for the n_deps += n_deps_base update in the restart loop (or take schedule->lock in ucc_dependency_handler), and reset n_deps/n_deps_satisfied as a single atomic pair rather than accumulating them; also align the status resets — ucc_schedule_pipelined_post clears frags[i]->super.super.status/tasks[j]->super.status while the restart path clears frag->super.status/tasks[i]->status.
  2. Reproduce locally with the exact failing command plus -x UCC_LOG_LEVEL=trace to capture which fragment stalls (sched %p restarting frag ... traces).
  3. Short term, so PR CORE: Get local rank from topology if unset #1245 is not blocked by an unrelated defect: re-trigger the build, and consider temporarily narrowing the CL/HIER+split_rail+pipeline step to the non--T pass (or --num_tests 1 single-thread) until the schedule race is fixed. Do not raise the stage timeout — the process was deadlocked, not slow.

Related: PR #1245 (commit under test, #1245 — not the cause); prior work on this code: #1262 (c8314d31), #1051 (4f67436b, "fix n deps overflow in pipelined schedule"), #759 ([REDACTED:Hex High Entropy String], "CL/HIER: fix allreduce rab pipeline"). No existing issue found for this split_rail/pipeline+thread-multiple failure — worth opening one.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 5e1f92b8-5918-4bd1-ac85-2168a456ab97 in the triage console for the audit trail.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-nvls · commit be1f0004

TL;DR: The build's code compiled fine; it failed because the Allocate Slurm job stage could not get 2 nodes on the gb300nvl72_ci partition and salloc gave up after its full 1-hour --immediate=3600 window ("Unable to allocate resources: Connection timed out"). This is a cluster capacity/infra failure unrelated to commit be1f000 — retry the build once nodes free up, and separately fix the broken scancel in the cleanup stage which leaks the pending allocation.

Full analysis

Summary: Stage Allocate Slurm job (node 174) failed after 3613 s — salloc -N 2 -p gb300nvl72_ci --immediate=3600 timed out waiting for resources; the subsequent pipeline_stop stage also failed to cancel the orphaned job.

Root cause: Both Build Docker stages (x86_64 and aarch64) succeeded — the full UCC build, including the NVLS sources touched by this PR (tl_cuda_nvls.c, allreduce_nvls.c, reduce_scatter_nvls.c), compiled and installed cleanly with no warnings/errors. The failure is purely in resource acquisition:

+ ssh ... salloc -N 2 -p gb300nvl72_ci --job-name=jenkins-UCC-ucc-test-nvls-324 --immediate=3600 --time=01:30:00 --no-shell --account=blackwell-ci
salloc: lua: Setting job to exclusive mode as no gpu count was specified
salloc: Pending job allocation 2208096
salloc: job 2208096 queued and waiting for resources
salloc: error: Unable to allocate resources: Connection timed out

The gap 13:34:08 → 14:34:17 is exactly 3600 s, i.e. the --immediate=3600 deadline, and Slurm explicitly reported the job as queued and waiting for resources. So this is not a hang in UCC code or a build timeout — the gb300nvl72_ci GB300 partition simply had no 2 free exclusive nodes for a full hour (note --immediate also forces exclusive-node allocation because no GPU count is requested, which makes the request harder to satisfy). No test binary ever ran.

A second, genuine pipeline bug is visible in the cleanup path: slurm.stop was called with jobId:${SLURM_JOB_ID} passed through as a literal string, so on the remote (csh) shell it produced:

+ ssh ... scancel ${SLURM_JOB_ID}
SLURM_JOB_ID: Undefined variable.

scancel therefore never ran, so the pending allocation (2208096, and any future successful one) is not cleaned up by the pipeline — which itself contributes to queue starvation across repeated builds of this job.

Implicated commit: unknown for the allocation failure (infrastructure, not code — commit be1f000 / PR #1245 built successfully). The cleanup defect traces to the Slurm-lib conversion commits cc359973 "CI: convert nvls tests to use slurm jenkins lib" and 817fc967 "CI: add Slurm-based UCC MPI + DLRM test job" (Daniel Pressler).

File: .ci/ pipeline definition for ucc-test-nvls — the slurm.allocation call (partition:gb300nvl72_ci, nodes:2, immediateTimeout:3600) and the slurm.stop call (jobId:${SLURM_JOB_ID})

Suggested fix:

  1. Re-run build CI tester (DO NOT MERGE) #324 — there is no code defect to fix; the GB300 CI partition was saturated. Check sinfo -p gb300nvl72_ci / squeue -p gb300nvl72_ci for drained or held nodes before retrying.
  2. Make the allocation failure non-fatal-and-retryable rather than a hard red build: wrap the slurm.allocation step in retry(2), or reduce immediateTimeout (e.g. 900 s) and re-attempt, so a queue wait doesn't burn an hour of Jenkins agent time and then fail the PR.
  3. Fix the cleanup so SLURM_JOB_ID is interpolated in Groovy (double quotes / read the value written to jobIdFile: /mnt/pvc/job-id-jenkins-UCC-ucc-test-nvls-324.txt) instead of being shipped literally to the remote shell — e.g. scancel $(cat /mnt/pvc/job-id-...txt) — and guard against an empty ID. Also consider scancel --name=jenkins-UCC-ucc-test-nvls-${BUILD_NUMBER} as a belt-and-braces cleanup so aborted/leaked allocations are always reaped.
  4. Consider requesting GPUs explicitly (--gpus-per-node=...) so the Lua job submit plugin doesn't escalate the request to whole-node exclusive, improving schedulability on a busy partition.

Related: PR #1245 (the triggering PR, unaffected — its build passed); CI plumbing commits cc359973, 817fc967, 71963b2b.

@svcnbu-swx-hpcx

Copy link
Copy Markdown
Collaborator

🤖 CI Triage AgentUCC/ucc-test-nvls · commit 49de1ae3

TL;DR: The build failed in the "Allocate Slurm job" stage — salloc sat in the gb300nvl72_ci queue for the full --immediate=3600 hour and gave up ("Unable to allocate resources: Connection timed out"), so no UCC code was ever tested; this is a cluster-capacity/infra failure, not a regression from PR #1245.

Full analysis

Summary: UCC/ucc-test-nvls #324 failed while allocating a 2-node Slurm reservation on the gb300nvl72_ci partition; the allocation request timed out after 1 hour and the pipeline aborted before running any tests.

Root cause: In stage 174 (Allocate Slurm job), the pipeline ran:
salloc -N 2 -p gb300nvl72_ci --job-name=jenkins-UCC-ucc-test-nvls-324 --immediate=3600 --time=01:30:00 --no-shell --account=blackwell-ci
Log timestamps show the command started at 13:34:08Z and the next output arrived at 14:34:17Z — a single ~60-minute gap that matches --immediate=3600 exactly. The Slurm output is explicit about what happened during that gap: salloc: Pending job allocation 2208096 / salloc: job 2208096 queued and waiting for resources / salloc: error: Unable to allocate resources: Connection timed out. So the process was queued and waiting on the scheduler, not hung and not slow in application code — there were no free nodes in gb300nvl72_ci within the immediate window. All preceding stages (Checkout, both Build Docker stages) succeeded, and there are no test artifacts, confirming nothing under test ever executed. The commit under test (49de1ae, PR #1245 "CORE: Get local rank from topology if unset") cannot have contributed, since no UCC binary was run.

Secondary (real but non-fatal) bug exposed by the failure path: the cleanup stage 224 called slurm.stop with jobId:${SLURM_JOB_ID} un-interpolated and failed with SLURM_JOB_ID: Undefined variable., running a bare scancel with no argument. When allocation fails there is no job ID to cancel, but this also means a successfully allocated job would be leaked if the variable is single-quoted in the Groovy source — worth checking, as leaked reservations would themselves starve the partition and cause exactly this queue timeout on subsequent builds.

Implicated commit: unknown — no UCC source commit is implicated. The Slurm allocation wiring dates to cc35997 (Daniel Pressler, "CI: convert nvls tests to use slurm jenkins lib") and the stop-stage call pattern to 817fc96 / 6d54511 (Daniel Pressler), but the trigger here is cluster capacity, not a code change.

File: .ci/ Jenkinsfile for ucc-test-nvls (the slurm.allocation call with immediateTimeout:3600, and the slurm.stop call passing jobId:${SLURM_JOB_ID})

Suggested fix: Retry the build — this is a transient/capacity failure with no code signal. Then, to stop it recurring and to stop it being misreported as a code failure:

  1. Verify gb300nvl72_ci capacity and check for leaked reservations from prior Jenkins builds (squeue -p gb300nvl72_ci -u svcnbu-swx-hpcx); cancel any orphaned jenkins-UCC-ucc-test-nvls-* jobs.
  2. Fix the cleanup stage so SLURM_JOB_ID is actually interpolated (use a double-quoted Groovy string / read the ID back from the jobIdFile at /mnt/pvc/job-id-jenkins-UCC-ucc-test-nvls-324.txt) and guard scancel against an empty argument — otherwise real allocations can leak and starve the partition.
  3. Make allocation timeouts distinguishable from test failures: on Unable to allocate resources mark the build UNSTABLE/ABORTED (or auto-retry the allocation) rather than failure, so poll-triggered infra timeouts don't page as PR regressions. Do not simply raise --time; the 90-minute job time limit was never reached.

Related: PR #1245 (commit under test, not implicated). No existing issue tracks gb300nvl72_ci allocation timeouts — consider opening one to track CI partition capacity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants