Skip to content

UCP/PROTO: Initialize short thresholds from protocol selection - v1.23.x - #11921

Closed
yosefe wants to merge 2 commits into
openucx:v1.23.xfrom
yosefe:topic/ucp-proto-initialize-short-thresholds-from-protocol-v1.23.x
Closed

yosefe wants to merge 2 commits into
openucx:v1.23.xfrom
yosefe:topic/ucp-proto-initialize-short-thresholds-from-protocol-v1.23.x

Conversation

@yosefe

@yosefe yosefe commented Sep 8, 2026 •

Copy link
Copy Markdown
Member

Why

Backport #11817 to v1.23.x branch

If check_events_do could not arm the iface (returned BUSY), we can hang
forever in ucp_worker_wait because one of the inactive ifaces is not
armed to be re-activated and trigger wakeup via worker->eventfd channel.
@yosefe yosefe changed the title UCP/PROTO: Initialize short thresholds from protocol selection UCP/PROTO: Initialize short thresholds from protocol selection - v1.23.x Sep 8, 2026
@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Starting review — findings will be posted here when done.

}
}

/* FIXME proto_select could get invalidated by adding rkey config */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what's the plan for this FIXME? if proto_select can really be invalidated by adding an rkey config while a cached pointer is live, that's a use-after-realloc, not just a note. can we open an issue and reference it here, or is this already handled by ucp_proto_select_cache_reset()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

will fix it as a separate PR for master branch

EXPECT_FALSE(ep_config->proto_init_flags &
UCP_EP_PROTO_SHORT_INITIALIZED);

/* No protocol can be cached while the initialization flag is clear */

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor: the comment says "No protocol can be cached while the initialization flag is clear", but the flag being clear is checked just above; here we're asserting the flag gets set by the first send. maybe reword to "short init is deferred until the first send".

@svc-nvidia-pr-review

Copy link
Copy Markdown

Style note (non-blocking): The enriched ucp_worker_iface_activate debug message keeps the a_count=/uct_flags=0x%x = form, which docs/LoggingStyle.md discourages, but it matches the pre-existing style of that same line, so not worth changing in isolation.

@svc-ucx

svc-ucx commented Sep 8, 2026

Copy link
Copy Markdown

🤖 CI Triage Agent — UCX PR (Tests roce on worker 0) · commit a26bb450

TL;DR: This is not a code failure — the Azure agent on agent-07 was shut down mid-configure ("The agent has received a shutdown signal"), cancelling the job ~16 s into the build; simply re-run the "roce on worker 0" job.

Full analysis

Summary: The Run ./contrib/test_jenkins.sh step of "Tests roce on worker 0" (build 134968) was cancelled during the second (configure-devel) configure pass, before anything was compiled or any test ran.

Root cause: Infrastructure, not the PR. The log shows continuous activity from 13:20:32 to 13:20:48 (largest inter-line gap < 0.5 s, so no hang and no wall-clock timeout), then:

  • 13:20:38.0386363Z ##[error]The agent has received a shutdown signal. This can happen when the agent service is stopped, or a manually started agent is canceled.
  • 13:20:48.1464654Z ##[error]The operation was canceled. → Finishing: roce on worker 0

The self-hosted agent ([REDACTED:Base64 High Entropy String]) was stopped/drained by the host while the job was in progress, so Azure DevOps cancelled the step. There is zero test or compile output, no assertion, no signal from UCX code — the commit under test (a26bb45, ucp proto short-threshold work) cannot be implicated.

A secondary, corroborating environment smell: on this RoCE worker the first configure pass detected no verbs support — configure: Compiling without verbs support, IB modules: < >, build_modules=':cma:knem:xpmem:fuse:mad', and the grep -wE '^build_modules=.*:(ib|rdmacm|efa|mlx5)' config.log guard in the script matched nothing. That means rdma-core devel headers were not visible on the machine, i.e. the node was already in a degraded/partially-provisioned state when the agent went down. (Also cosmetic and pre-existing: configure: line 29313: test: : integer expression expected from the ROCm version check with an empty version variable.)

Implicated commit: none — no UCX commit is implicated; the failure is agent/host-level.

File: N/A (job cancelled inside contrib/test_jenkins.sh → buildlib/tools/common.sh:121 ../contrib/configure-${mode})

Suggested fix:

  1. Re-run the "roce on worker 0" job — this is a transient CI infrastructure failure and should not block PR UCP/PROTO: Initialize short thresholds from protocol selection - v1.23.x #11921.
  2. Ask CI ops to check agent-07: why the agent service was stopped/restarted at 2026-09-08 13:20:38, and whether the node is missing rdma-core/libibverbs-dev (a RoCE worker configuring with IB modules: < > will silently run a non-RDMA test matrix even when it does complete, which is a real coverage gap).
  3. Optional hardening: make contrib/test_jenkins.sh fail fast with a clear message when the existing grep -wE '^build_modules=.*:(ib|rdmacm|efa|mlx5)' check finds no IB modules on an IB/RoCE-designated worker, instead of proceeding to build a verbs-less library.

Related: none (no matching issue found; the search hits for the agent-shutdown string were unrelated PRs).

🛡️ This comment had 1 potential secret(s) redacted (Base64 High Entropy String). See request_id 86dc89e1-2eb7-464c-be05-c236fade1a16 in the triage console for the audit trail.

@tvegas1

tvegas1 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Merged as #12035 as we missed that one.

@tvegas1 tvegas1 closed this Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants