Skip to content

UCP/RMA: Preserve fence ordering for rendezvous operations - #11927

Draft
nbellalou wants to merge 5 commits into
openucx:masterfrom
nbellalou:fence-split-5-rma-rndv
Draft

UCP/RMA: Preserve fence ordering for rendezvous operations#11927
nbellalou wants to merge 5 commits into
openucx:masterfrom
nbellalou:fence-split-5-rma-rndv

Conversation

@nbellalou

@nbellalou nbellalou commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

What

Preserve endpoint-fence ordering for RMA rendezvous PUT and GET operations.

Why

This is Part 5 of 5 of an updated split of the earlier endpoint-fence PR #11199.

Rendezvous operations can start delayed nested remote work. Part 4 activates asynchronous endpoint fences for normal RMA/atomic traffic, but without rendezvous accounting a later fence could advance before that nested remote work is known to be complete. This PR completes the fence design for rendezvous operations.

How

  • Count active RMA rendezvous operations in the endpoint flush state and make fence admission account for them.
  • Track, transfer, reclaim, and cancel the accounting correctly across PUT/GET retries and nested requests.
  • Carry a compact completion-header flag for the remote completion and retain compatibility with the legacy header format.
  • Gate the new protocol behavior on peer compatibility and cover retry visibility and header decoding.

Draft — merge only after PR #11926 is merged.

Allow internal callers to flush a selected set of endpoint lanes while preserving the existing all-lane flush behavior. Track lane replacement generations so an in-flight flush safely reconciles failover topology changes.

Overlay the new lane-selection snapshot with the later memory-flush phase, keeping ucp_request_t at its existing release size.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Add an epoch-ordered per-endpoint queue and progress callback that can advance a fence with a selective remote flush before retrying blocked work. Track the in-flight flush explicitly and purge queued requests on terminal errors.

Overlay the new fence metadata with the existing RMA and atomic request storage through an anonymous union. This preserves the established send.rma and send.amo field names and limits the request size increase to eight bytes.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Track lane replacements that affect pre-fence work and conservatively normalize the fence lane mask before flushing. Keep queued fence requests across endpoint reconfiguration and restart protocol requests on the current endpoint configuration.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Gate RMA and atomic request admission on the endpoint fence epoch without progressing UCT recursively. Queue blocked requests on the endpoint, make endpoint and worker flush wait for relevant fence work, and cover fresh requests that defer before selecting a transport lane.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
Track RMA rendezvous transfers until their nested remote operation completes, so endpoint-based fence ordering cannot bypass them. Make retry accounting transactional, tag rendezvous completions with a backward-compatible one-byte header flag, and gate the protocol on compatible peers and error modes.

Signed-off-by: Nathan Bellalou <nbellalou@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

@@ -55,6 +55,7 @@ static const char *ucp_request_flag_names[] = {
[ucs_ilog2(UCP_REQUEST_FLAG_RNDV_GET_REQ)] = "rndv_get_req",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UCP_REQUEST_FLAG_FENCE_BLOCKED (bit 30) is added but has no entry in ucp_request_flag_names[], while UCP_REQUEST_FLAG_RMA_RNDV_TRACKED (bit 31) does. A blocked fenced request will render without that flag name in ucp_request_str() debug output. Add an entry, e.g. [ucs_ilog2(UCP_REQUEST_FLAG_FENCE_BLOCKED)] = "fence_blk".

@@ -69,7 +79,9 @@ enum {
UCP_REQUEST_FLAG_RNDV_SEND_INTERNAL = UCS_BIT(26),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UCP_STATUS_FENCE_DEFER (UCS_ERR_LAST + 1) evaluates to -99, which UCS_STATUS_IS_ERR() reports as an error and which is not a named status; if it ever escapes the hand-written interception points (e.g. into ucs_status_string() or a generic error path) it will be misinterpreted. The interception is currently careful, but consider a comment noting it is an internal sentinel that must never reach generic status handling, and double-check the static-assert message ("must be an error pointer") — the wording is confusing since this is a status code, not a pointer.

@svc-nvidia-pr-review

Copy link
Copy Markdown

PR scope blocker: The PR adds well over 500 lines of non-test source plus ~650 lines of tests in a single change. Per the project size limit, this is too large to review and land as one unit. Please split it into separately reviewable PRs — e.g. (1) the request-union restructuring (rma/amo sharing fence_seq/fence_pending_elem), (2) selective-lane flush + lane-generation tracking, (3) the async fence queue, and (4) RMA/RNDV visibility tracking. Each should be independently testable.

Test coverage note: The RMA/RNDV visibility and fence-queue tests are gated on is_self(), so several of the new behaviors are only exercised on the self transport. Worth confirming CI runs an inter-node/failover job that also exercises the RMA/RNDV dst_version/err-mode gating and the reconfiguration-restart path in ucp_ep_fence_dispatch_request, since those are the highest-risk paths and are not covered by the synthetic self-only tests.

@svc-ucx

svc-ucx commented Sep 8, 2026

Copy link
Copy Markdown

🤖 CI Triage AgentUCX PR (Tests roce on worker 1) · commit e36c877f

TL;DR: The "roce on worker 1" job didn't fail on any test or compile error — the Azure DevOps agent (agent-10) received a shutdown signal mid-compile and the job was force-cancelled. Just re-run the pipeline; nothing in PR #11927 caused this.

Full analysis

Summary: Job Tests roce on worker 1 (build 135008) was cancelled during the Run ./contrib/test_jenkins.sh step while still compiling the gtest sources — no test ever executed.

Root cause: Infrastructure/agent termination, not a code defect. The log shows continuous, healthy build progress with no stalls: make output every 1–10 seconds from 15:07:15 through 15:11:42 (compiling test/gtest/..., last file started was ucp/test_ucp_fault_tolerance.cc). At 15:11:45.826 the log emits:

  • ##[error]The agent has received a shutdown signal. This can happen when the agent service is stopped, or a manually started agent is canceled.
  • ##[error]The Operation will be canceled...##[error]The operation was canceled.

followed by Start cleaning up orphan processes killing the still-running make/g++/cc1plus children on [REDACTED:Base64 High Entropy String]. There is no compiler error, no linker error, no gtest output, and no wall-clock timeout (the step had only been running ~5 minutes, with the largest inter-line gap under ~11 seconds). That signature is the Azure Pipelines agent service being stopped/drained or the host being reclaimed, which aborts the build regardless of its content.

Implicated commit: none — the failure is unrelated to [REDACTED:Hex High Entropy String] (the commit's code was never even fully compiled, let alone tested).

File: n/a (failure occurred in CI infrastructure; last build activity: test/gtest/ucp/test_ucp_fault_tolerance.cc compile, log line at 2026-09-08T15:11:51)

Suggested fix:

  1. Re-queue build 135008 / re-trigger CI on PR UCP/RMA: Preserve fence ordering for rendezvous operations #11927 (/azp run or push an empty commit). Do not change any source or raise any timeout — neither is implicated.
  2. Have CI ops check the health of the agent-10 pool member (/scrap/azure/agent-10): confirm whether the agent service was restarted, the VM was drained/patched, or the agent went offline around 2026-09-08 15:11:45 UTC. Repeated shutdown signals on the same agent indicate an unstable host that should be taken out of the pool.
  3. If this recurs across multiple PRs on the same agent, consider enabling agent auto-reconnect/job retry (retryCountOnTaskFailure) so infra cancellations don't surface as PR-blocking red builds.

Related: none found — a search of the repo's issues/PRs for this agent-shutdown signature returned no matching reports (only unrelated open PRs).

🛡️ This comment had 2 potential secret(s) redacted (Base64 High Entropy String, Hex High Entropy String). See request_id 64b861ff-9018-47d0-a27c-68e325c4966c in the triage console for the audit trail.

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.

3 participants