UCP/EP/FT: handle the case when no more lanes to fallback by evgeny-leksikov · Pull Request #11351 · openucx/ucx

evgeny-leksikov · 2026-04-16T14:19:50Z

What?

Handle the case when no more lanes to fallback, fixes:

recursive request restart from zcopy completion
am header leak
ep_config ref counter fix for ifaces reactivation in lanes discard flow

Why?

to fix various failures like hang, segfaults and assertions

- recursive request restart from zcopy completion - am header leak - ep_config ref counter fix for ifaces reactivation in lanes discard flow

Copilot

Pull request overview

Handles failover edge-cases when an endpoint runs out of fallback lanes, aiming to prevent recursive restarts, fix AM header handling during failures, and correct worker interface reactivation/refcount behavior during lane discard/reactivation flows.

Changes:

Gate protocol restart on zcopy completion when the endpoint is already in FAILED state, and improve restart/reset assertions for easier debugging.
Extend ucp_ep_set_cfg_index() with a reactivate flag and adjust wireup/reconfig/discard flows to avoid incorrect iface (de)activation/refcounting.
Update AM eager multi-zcopy to cope with “no AM lane available” scenarios and adjust FT tests to cover exhaustion behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/gtest/ucp/test_ucp_fault_tolerance.cc	Updates AM failure-injection logic/tests for “no more lanes to fallback” scenarios and log/error expectations.
src/ucp/wireup/wireup_cm.c	Adapts to new `ucp_ep_set_cfg_index(..., reactivate)` signature.
src/ucp/wireup/wireup.c	Adapts to new `ucp_ep_set_cfg_index(..., reactivate)` signature.
src/ucp/proto/proto_common.inl	Prevents zcopy restart when EP is already failed.
src/ucp/proto/proto_common.c	Improves assertion diagnostics when proto reset fails during restart.
src/ucp/core/ucp_ep.h	Updates `ucp_ep_set_cfg_index()` API to include `reactivate` parameter.
src/ucp/core/ucp_ep.c	Implements conditional iface reactivation in `ucp_ep_set_cfg_index()` and adjusts discard/reconfig flows.
src/ucp/am/eager_multi.c	Adds “no AM lane” handling in eager multi-zcopy init/reset/completion.
src/ucp/am/eager.inl	Adds tracing for AM header descriptor allocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

yosefe · 2026-04-19T05:53:27Z

+    /**
+     * At least 1 non-failed AM lane must be available.
+     */
+    if (ucp_ep_get_am_lane(req->send.ep) == UCP_NULL_LANE) {


can we avoid extra branch in data path?

yosefe · 2026-04-19T05:54:20Z

+                                          send.state.uct_comp);
+
+    if (ucs_likely(self->status == UCS_OK) ||
+        (req->send.ep->flags & UCP_EP_FLAG_FAILED)) {


why needed, do we set flag FAILED in case of failover?

Only when all lanes are failed

Needed to avoid leak of descriptor with copy of user AM header

yosefe · 2026-04-19T05:55:04Z

+        return status;
+    }
+
+    return (ucp_ep_get_am_lane(req->send.ep) == UCP_NULL_LANE) ?


what does it mean if ucp_ep_get_am_lane(req->send.ep) == UCP_NULL_LANE?

It means that the are no live AM lanes after reconfiguration.
I converted such places to assertion since proto selection logic should not lead to this situation.

gleon99 · 2026-04-19T15:34:58Z

+            UCS_TEST_MESSAGE << "Keep 1 live lane for UD transports since "
+                             << "local error injection on all lanes leads to "
+                                "failed assertion in ud_ep_purge";
+            num_lanes_to_fail--;


Can num_lanes_to_fail be 0 or 1, before the subtraction?

No, shuffle_lanes at very begin skips the test if num_lanes < 2

gleon99 · 2026-04-19T15:38:43Z

+                short_progress_loop();
+                size_t min_expected_err_count = 1;
+                if ((failure_side == FAILURE_SIDE_TARGET) &&
+                    has_transport("dc_x")) {


Please document DC special case

gleon99 · 2026-04-19T15:40:28Z

    status = ucp_ep_reconfig_internal(ucp_ep, failed_lanes);
    if (status != UCS_OK) {
+        ucs_assertv(ucp_ep->cfg_index == old_cfg_index,
+                    "ep %p: cfg_index %u -> %u after reconfiguration error %s", ucp_ep, old_cfg_index,


Minor: long line

gleon99 · 2026-04-19T15:41:18Z

        return UCS_ERR_NO_MEMORY;
    }

+    ucp_trace_req(req, "allocating AM header descriptor 0x%p", reg_desc);


Minor: 0x is redundant.

gleon99 · 2026-04-19T15:41:36Z

        add_variant_with_value(variants, UCP_FEATURE_AM,  TEST_OP_AM,
                               op_name(TEST_OP_AM));
-        add_variant_with_value(variants, UCP_FEATURE_AM | UCP_FEATURE_RMA,  TEST_OP_AM | TEST_OP_FLUSH,
+        add_variant_with_value(variants, UCP_FEATURE_AM,  TEST_OP_AM | TEST_OP_FLUSH,


UCP_FEATURE_RMA was added by mistaken earlier for AM test. Do you want to open separate PR for this trivial fix?

gleon99 · 2026-04-19T15:49:37Z

+    }
+
+    return (ucp_ep_get_am_lane(req->send.ep) == UCP_NULL_LANE) ?
+           UCS_ERR_UNREACHABLE : UCS_OK;


The func returns OK if ucp_am_proto_request_zcopy_reset is OK, AND no AM lanes?
Needs at least commenting..

gleon99 · 2026-04-19T16:05:00Z

+                size_t min_expected_err_count = 1;
+                if ((failure_side == FAILURE_SIDE_TARGET) &&
+                    has_transport("dc_x")) {
+                    min_expected_err_count = 0;


Can skip the test, the next assert is always true.

gleon99 · 2026-04-19T16:05:19Z

    }

    ucs_status_t do_am_send_and_wait(ucp_ep_h ep, size_t size, bool flush_after) {
+


minor: remove

evgeny-leksikov added 2 commits April 16, 2026 17:14

UCP/EP/FT: handle the case when no more lanes to fallback

b85e35d

- recursive request restart from zcopy completion - am header leak - ep_config ref counter fix for ifaces reactivation in lanes discard flow

GTEST: extend UCP/FT/AM test to all lanes failure

d3115c4

evgeny-leksikov mentioned this pull request Apr 16, 2026

GTEST/UCP/FT: test FT in sequence of PUT/AM/FLUSH #11352

Draft

evgeny-leksikov changed the title ~~Ucp ft all lanes failure~~ UCP/EP/FT: handle the case when no more lanes to fallback Apr 16, 2026

UCP/EP/FT: fix mem leak of discard arg on failed UCP EP

b38b52b

evgeny-leksikov requested a review from Copilot April 17, 2026 11:45

Copilot started reviewing on behalf of evgeny-leksikov April 17, 2026 11:46 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread src/ucp/am/eager_multi.c Outdated

Comment thread test/gtest/ucp/test_ucp_fault_tolerance.cc

Comment thread src/ucp/core/ucp_ep.h

yosefe reviewed Apr 19, 2026

View reviewed changes

gleon99 reviewed Apr 19, 2026

View reviewed changes

UCP/EP/FT: handle the case when no more lanes to fallback, CR1

5402f65

		}

		ucs_status_t do_am_send_and_wait(ucp_ep_h ep, size_t size, bool flush_after) {

Conversation

evgeny-leksikov commented Apr 16, 2026

What?

Why?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants