From fa65e794ec764d54b78bce82f64bc0e2877f77f5 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 17 Jun 2026 04:46:01 +0000 Subject: [PATCH] fix(libxce-failover): known-dead ctx (XCE_CTX_ERR) triggers reconnect instead of surfacing -EINVAL Live 3-node E2E revealed the reported 'Invalid argument' bug was still reachable: after the primary dies, handle_disconnect leaves ctx.state= XCE_CTX_ERR, so a FRESH xce_* call returns -EINVAL from the xceapi guard before reaching a primary. The classifier mapped -EINVAL to TERMINAL, so the loop bailed without rotating/recovering. decide() now detects the known-dead ctx and treats it as a prompt reconnect cause (RETRY_NOW), surfacing the distinct cluster-not-ready code on budget exhaustion, never -EINVAL. Regression test: test_decide_known_dead_ctx_retries_not_terminal. --- src/lib/client_failover.c | 22 ++++++++++++++ src/lib/testsuite/test_libxce_retry.c | 41 +++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/lib/client_failover.c b/src/lib/client_failover.c index b3a0cb1..8f51e66 100644 --- a/src/lib/client_failover.c +++ b/src/lib/client_failover.c @@ -271,6 +271,28 @@ bool xcecli_retry_decide(api_ctx_t* ac, int* rc, bool is_write, if (ac == NULL || rc == NULL || cfg == NULL || st == NULL) return false; + /* Known-dead connection (the reported bug). After the primary dies, + * handle_disconnect leaves ctx.state == XCE_CTX_ERR, so a FRESH xce_* call + * hits the per-op xceapi guard and returns -EINVAL *before* the op reaches a + * primary — and -EINVAL would otherwise classify as TERMINAL and surface the + * opaque "Invalid argument". Detect the dead-ctx state directly and treat it + * as a prompt transport-style failover cause (reconnect via rotate+recover), + * regardless of *rc. After the next successful reconnect the ctx returns to + * RUN and subsequent attempts classify normally (success / no-primary / + * transport). On budget exhaustion we never reached a primary → surface the + * distinct cluster-not-ready code, never -EINVAL. */ + if (ac->ctx.state == XCE_CTX_ERR) { + if (action_out != NULL) + *action_out = XCE_FO_RETRY_NOW; + st->attempt++; + if (st->attempt > cfg->max_retries) { + *rc = -E_XCE_NOT_READY; + return false; + } + st->delay_ms = 0; + return true; + } + xce_fo_action_t action = xce_failover_classify(*rc, is_write, st->attempt); if (action_out != NULL) diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index f6a532c..033fd46 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -1022,6 +1022,46 @@ static void test_decide_transport_exhaustion_keeps_last_rc(void) printf("test_decide_transport_exhaustion_keeps_last_rc: PASS\n"); } +/* + * Regression for the live-E2E bug: after the connected primary dies, + * handle_disconnect leaves ctx.state == XCE_CTX_ERR, so a FRESH xce_* call + * returns -EINVAL from the per-op xceapi guard BEFORE reaching a primary. + * decide() must treat the known-dead ctx as a prompt reconnect cause + * (RETRY_NOW, no backoff) rather than classifying -EINVAL as TERMINAL, and on + * budget exhaustion surface the distinct cluster-not-ready code, never -EINVAL. + */ +static void test_decide_known_dead_ctx_retries_not_terminal(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + + xce_retry_config_t cfg = fo_test_retry_cfg(); + xcecli_retry_t st = XCECLI_RETRY_INIT(&ac); + xce_fo_action_t action; + + /* Known-dead ctx + the guard's -EINVAL must RETRY (reconnect), not surface. */ + ac.ctx.state = XCE_CTX_ERR; + int rc = -EINVAL; + bool a = xcecli_retry_decide(&ac, &rc, false, &cfg, &st, &action); + TEST_ASSERT_TRUE(a); + TEST_ASSERT_EQUAL_INT(XCE_FO_RETRY_NOW, action); /* prompt rotate, no backoff */ + TEST_ASSERT_EQUAL_INT(0, st.delay_ms); + + /* ctx stays ERR (no real reconnect in this unit) → keep retrying until the + * budget is exhausted, then surface -E_XCE_NOT_READY (never -EINVAL). */ + int guard = 0; + do { + rc = -EINVAL; + a = xcecli_retry_decide(&ac, &rc, false, &cfg, &st, &action); + } while (a && ++guard < 100); + TEST_ASSERT_FALSE(a); + TEST_ASSERT_EQUAL_INT(-E_XCE_NOT_READY, rc); + + printf("test_decide_known_dead_ctx_retries_not_terminal: PASS\n"); +} + /* Req 6.3: when MANY clients fail over simultaneously they must NOT retry in * lockstep — randomized (jittered) backoff staggers their delays. Each client * carries its OWN retry carrier (seeded from its own api_ctx pointer, exactly as @@ -1657,6 +1697,7 @@ void test_libxce_retry(void) RUN_TEST(test_decide_terminal_surfaces_unchanged); RUN_TEST(test_decide_dup_on_resent_write_is_success); RUN_TEST(test_decide_transport_exhaustion_keeps_last_rc); + RUN_TEST(test_decide_known_dead_ctx_retries_not_terminal); RUN_TEST(test_decide_jitter_staggers_many_clients); RUN_TEST(test_recover_resets_err_state_to_run); RUN_TEST(test_recover_leaves_run_state_untouched);