[nbellalou]

What

Add the asynchronous endpoint-fence engine: a per-endpoint queue of deferred requests, a single in-flight strong fence, and ordered fence epochs. This PR does not enable the asynchronous endpoint fence yet

Why

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

This PR deliberately prepares and tests the engine before ordinary RMA and atomic requests use it. Keeping lifecycle, ordering, error handling, and request deferral separate makes that machinery independently reviewable; Part 4 will activate it for the normal data path.

How

• Add endpoint state for the pending fence queue, the single in-flight flush, status, and scheduled progress.
• Associate each request and flush completion with an exact fence sequence, so an epoch advances only after its transport flush completes.
• Dispatch deferred requests in sequence order and correctly resume or purge them after completion or error.
• Use the selective-flush primitive from Part 1 and add focused asynchronous queue and lifecycle coverage.

Draft — merge only after PR #11923 is merged.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

ucp_ep_ext_t grows 216->256 and ucp_request_t 272->280. Given the compactness guidance, consider whether fence_status (ucs_status_t) and the uint8_t fence_pending_scheduled can be packed, and whether lane_generation/fence_seq both need to be 64-bit, to limit per-ep growth.

[svc-nvidia-pr-review]

PR Size limit exceeded: REVIEW.md sets a hard 500 added-line limit, but this PR is well over that, spanning ucp_ep.c (~180 lines), flush.c, ucp_request.h, rma.h (~55), ucp_ep.h/.inl, and ~470 lines of new tests. It also bundles several independent changes: (1) lane-generation tracking + ucp_ep_set_lane no-op guard + memset init refactor, (2) selective-lane flush (ucp_ep_flush_lanes_internal, lane_mask) and the flush lane-state helper, (3) request-struct union reorg moving rma/amo under a shared fence union, and (4) the async fence pending-queue state machine. Per guidance ('Feature PRs must not refactor code unrelated to the feature'; 'Refactoring must be in a separate PR'), please split at minimum the lane-generation/set_lane refactor and the request-struct reorg into their own PRs, and land the async-fence machinery separately from the selective-flush primitive.

Dead production code: the new async-fence entry points appear to have no production caller — ucp_ep_fence_strong_nb, ucp_ep_fence_pending_add, ucp_ep_fence_pending_resume, UCP_STATUS_FENCE_DEFER, and UCP_REQUEST_FLAG_FENCE_BLOCKED are only exercised by the new gtests (only ucp_ep_fence_pending_purge is wired into ucp_ep_reqs_purge). REVIEW.md's compactness rule says to avoid adding code that isn't used. Is there a follow-up caller planned, or should the enqueue/dispatch path land together with its user? Please clarify intent in the description, or drop the currently-unreachable paths.

Test-only aliasing risk: the new tests build fence requests on the stack and set req->send.state.completed_size, then feed them through the fence dispatch path, while real flush requests use the overlapping req->send.state.uct_comp in the same union. This works only because the synthetic requests never take the flush path, but it couples the tests to the current union layout; a short comment noting that these are deliberately non-flush requests would help future maintainers.

Coverage note: the async-fence dispatch path has no production caller yet, so there is no end-to-end/integration coverage of it under real transports and failover. The new logic is otherwise covered by focused gtests for the flush lane-state helper, selective flush, lane generation, and the fence pending queue. Worth confirming end-to-end coverage before it ships as a live path.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests roce on worker 1) · commit 96defc24

TL;DR: The "roce on worker 1" job didn't fail on any test or compile error — the Azure Pipelines agent on agent-06 received a shutdown signal mid-build (while compiling gtest sources), so the job was cancelled; just re-run the pipeline.

Full analysis

Summary: Job roce on worker 1 (build 134992) was aborted during ./contrib/test_jenkins.sh while compiling test/gtest objects, because the self-hosted Azure agent was shut down/cancelled.

Root cause: Infrastructure, not code. The log shows a clean, continuously progressing build: make compiles test/gtest/... objects with no warnings or errors from 15:07:01 through 15:11:45 (largest gap between consecutive log lines is only a few seconds — so nothing hung, and no time-limit was exceeded). At 15:11:45.66 the agent emitted:

• ##[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. The next steps may not contain expected logs.
• ##[error]The operation was canceled.

followed by Start cleaning up orphan processes and SIGTERM of the in-flight make/g++/cc1plus children (pids 3213535, 3214235, …). The last application activity was g++ ... -c -o ucp/gtest-test_ucp_rma.o ucp/test_ucp_rma.cc — an ordinary compile that was killed, not a failure. The gtest binary was never linked and no tests ever ran, so nothing in PR #11924 (commit 96defc2, branch fence-split-2-async-engine) was exercised by this job.

Implicated commit: none — not a code regression. (The failure is unrelated to [REDACTED:Hex High Entropy String].)

File: n/a — build was killed while compiling test/gtest/ucp/test_ucp_rma.cc (agent agent-06, workspace [REDACTED:Base64 High Entropy String])

Suggested fix: Re-trigger the UCX PR pipeline for this commit; no code change is warranted. If this recurs on the same host, ask CI admins to check agent-06 for agent-service restarts, host reboots/maintenance, or OOM/disk pressure on /scrap/azure/agent-06 around 2026-09-08 15:11 UTC (an agent that is stopped mid-job produces exactly this signature). Do not raise any job timeout — the log shows uninterrupted progress with no stall, so this was an external cancellation rather than a hang or a slow build.

Related: none found — searches for the agent-shutdown signature and for this job name returned no matching issues or PRs in openucx/ucx.

🛡️ This comment had 2 potential secret(s) redacted (Base64 High Entropy String, Hex High Entropy String). See request_id bdf4fd09-4af9-40c6-9c81-95e600a80c5c in the triage console for the audit trail.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

blocker: the flushed callback can be invoked synchronously from ucp_ep_flush_lanes_internal() (ucp_ep_flush_is_completed() -> ucp_ep_flush_mem_start() -> ucp_ep_flush_mem_progress() -> flushed_cb) when ep->ext->flush_sys_dev_map != 0, while mem_start() still returns UCS_INPROGRESS so a valid-looking req + 1 is returned. In that flow the callback already did ucp_request_put(req) and its assert fires (fence_inflight_req is still NULL); here we then write flush_req->send.flush.fence_seq into freed memory and store a dangling fence_inflight_req that blocks the pending queue forever.

[svc-nvidia-pr-review]

ep->ext->unflushed_lanes = 0 in the async completion callback also clears lanes that got dirty after the flush started, while only req->send.flush.lane_mask was actually flushed, so a later fence can skip or weak-fence those lanes; pls clear just the flushed subset (ep->ext->unflushed_lanes &= ~req->send.flush.lane_mask).

[svc-nvidia-pr-review]

UCP_STATUS_FENCE_DEFER (UCS_ERR_LAST + 1 = -99) is inside the ucs_status_t error range, so UCS_STATUS_IS_ERR() treats it as a real error and ucs_status_string() cannot name it; if a deferring progress function is reached from ucp_request_try_send() (the regular RMA/AMO send path) or from a UCT pending callback we get ucs_fatal("unexpected error"). Can we use a request flag or an explicit out-param instead of a fake status?

[svc-nvidia-pr-review]

How is the lane_mask contract from the ucp_ep_flush_lanes_internal() doc kept here ("defer new such writes on excluded lanes until this flush completes")? While this flush is in flight, non-fenced ops can still write on lanes outside the snapshot, and the mem flush will consume/clear flush_sys_dev_map for them.

[svc-nvidia-pr-review]

minor: fence_status looks redundant (effectively write-only) - its only read (fence_status == UCS_OK in ucp_ep_fence_try_advance_epoch()) is reachable only right after a synchronous completion where it is always UCS_OK; dropping it would also shrink ucp_ep_ext_t.

[svc-nvidia-pr-review]

A failed ASSERT_NE inside the critical section returns from the test with the worker lock still held and iface->ops.ep_flush still pointing to the test stub, which breaks all following tests in the process; pls restore the ops and exit the CS before asserting (or use EXPECT_* with explicit cleanup).

[svc-nvidia-pr-review]

minor: (status != UCS_OK) && UCS_STATUS_IS_ERR(status) is redundant since UCS_OK == 0 can never satisfy UCS_STATUS_IS_ERR(). Suggestion:

Suggested change

	if (ucs_unlikely((status != UCS_OK) && UCS_STATUS_IS_ERR(status))) {
	if (ucs_unlikely(UCS_STATUS_IS_ERR(status))) {

[svc-nvidia-pr-review]

Residual coverage gap: the inline-completion path (flush_sys_dev_map != 0, i.e. GPU/sys-dev flush) is not exercised by any of the new tests, and the self-transport-only tests give no coverage under failover or real RDMA transports.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Coverity coverity devel on coverity_rh7) · commit fc656d55

TL;DR: The Coverity gate failed with 4 new defects — all in test/gtest/ucp/test_ucp_fence.cc added by this PR's commit fc656d5: two "Missing unlock" and two "Explicit null dereferenced" findings caused by ASSERT_NE(...) early-returning while the worker critical section is held, and by Coverity not being able to prove test_flush_comp is non-NULL. Fix by releasing the CS via an RAII guard (or explicit unlock before failing) and by null-checking the captured completion with a plain if.

Full analysis

Summary: Azure "Coverity coverity devel on coverity_rh7" job exited non-zero: ##[error]Coverity found 4 issues: — all four in the newly added gtest test_ucp_fence32.async_fence_tracks_inflight_flush.

Root cause: Pure static-analysis gate failure (no runtime/test failure, no hang — the log shows the Coverity analysis completing and return 4 from the issue count):

1. Missing unlock (LOCK) at test_ucp_fence.cc:248 and :269 — the test takes the worker lock at line 241 with UCP_WORKER_THREAD_CS_ENTER_CONDITIONAL(ep->worker) and only releases it at line 280 (..._EXIT_CONDITIONAL). Between those points the test uses ASSERT_NE(static_cast<uct_completion_t*>(NULL), test_flush_comp) (lines 248 and 269). gtest ASSERT_* macros expand to return on failure, so on an assertion failure the function returns while still holding ep->worker->async.thread.mutex.lock — exactly the path Coverity walks ("25. missing_unlock" / "45. missing_unlock"). Any UCS_TEST_SKIP/FAIL()/ASSERT_* inside that region has the same problem.

2. Explicit null dereferenced (FORWARD_NULL) at :249 and :270 — test_flush_comp is a file-static pointer (test/gtest/ucp/test_ucp_fence.cc:16) that is explicitly set to NULL at lines 238 and 264, and only re-assigned inside test_flush_inprogress() (line 22), which is reached indirectly via the installed function pointer iface->ops.ep_flush = test_flush_inprogress (line 239). Coverity cannot follow that indirect call, so it keeps modelling test_flush_comp == NULL and reports the subsequent uct_invoke_completion(test_flush_comp, ...) (lines 249, 270) as dereferencing NULL via src/uct/base/uct_iface.h:1009. The ASSERT_NE does not clear the model on the path Coverity chose.

Implicated commit: [REDACTED:Hex High Entropy String] — Nathan Bellalou, "UCP/FENCE: Add asynchronous endpoint fence engine" (the PR head commit; it is the only recent change to this file)

File: test/gtest/ucp/test_ucp_fence.cc:241-249 and :263-270 (lock acquired at 241, released only at 280; asserts at 248/269; derefs at 249/270); static decl at test/gtest/ucp/test_ucp_fence.cc:16

Suggested fix: Restructure the critical section in async_fence_tracks_inflight_flush (and audit the sibling test pending_purge_preserves_scheduled_progress for the same pattern):

1. Make the lock exception/early-return safe with a small RAII guard instead of the bare macros, e.g.

class scoped_worker_cs {
public:
    scoped_worker_cs(ucp_worker_h w) : m_worker(w) {
        UCP_WORKER_THREAD_CS_ENTER_CONDITIONAL(m_worker);
    }
    ~scoped_worker_cs() {
        UCP_WORKER_THREAD_CS_EXIT_CONDITIONAL(m_worker);
    }
private:
    ucp_worker_h m_worker;
};

and replace lines 241 / 280 with scoped_worker_cs cs(ep->worker);. This removes both LOCK defects and is also the correct fix functionally (today an assertion failure deadlocks the rest of the gtest run).

2. Kill the FORWARD_NULL by copying the completion into a local and guarding it with an explicit if, which Coverity does model:

uct_completion_t *comp = test_flush_comp;
ASSERT_TRUE(comp != NULL);
if (comp == NULL) {
    return; /* guard for static analysis; CS released by RAII guard */
}
uct_invoke_completion(comp, UCS_OK);

(Alternatively keep ASSERT_NE and add the plain if (comp == NULL) return; right after it.) Do the same for the second occurrence at 269-270. Avoid an inline Coverity suppression annotation here, since the missing-unlock finding is a real latent bug, not a false positive.

Related: #11923 ("UCP/FLUSH: Add topology-safe selective endpoint flush", closed) appears to be the predecessor in this fence/flush series; no existing issue tracks these Coverity findings.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 4fd6965d-33c3-4b74-9198-1305de2cddc2 in the triage console for the audit trail.