diff --git a/src/include/xce/client_failover.h b/src/include/xce/client_failover.h index e43583f8..359c1967 100644 --- a/src/include/xce/client_failover.h +++ b/src/include/xce/client_failover.h @@ -18,10 +18,13 @@ typedef enum { (DUP on a re-sent write) */ XCE_FO_RETRY_NOW, /* E_XCE_REDIRECT / E_XCE_STANDBY / transport error — rotate, no backoff */ - XCE_FO_RETRY_BACKOFF, /* E_XCE_NOMASTER / E_XCE_NOT_READY — rotate after - jittered backoff */ - XCE_FO_TERMINAL /* AUTH/PERM/INVALID/NOENT and other non-retriable + XCE_FO_RETRY_BACKOFF, /* E_XCE_NOMASTER — rotate after jittered backoff */ + XCE_FO_TERMINAL, /* AUTH/PERM/INVALID/NOENT and other non-retriable — surface to caller */ + XCE_FO_RETRY_SAME /* E_XCE_NOT_READY (bug-1850) — the controller + answered but is briefly not ready: jittered + backoff, then retry the SAME controller (no + rotate/reconnect/rejoin) */ } xce_fo_action_t; /* diff --git a/src/lib/client_failover.c b/src/lib/client_failover.c index 238b7722..bc7df954 100644 --- a/src/lib/client_failover.c +++ b/src/lib/client_failover.c @@ -59,11 +59,19 @@ xce_fo_action_t xce_failover_classify(int status, bool is_write, int attempt) if (code == E_XCE_REDIRECT || code == E_XCE_STANDBY) return XCE_FO_RETRY_NOW; - /* No primary known yet / controller initializing or electing — rotate after - * jittered backoff (4.1). */ - if (code == E_XCE_NOMASTER || code == E_XCE_NOT_READY) + /* No primary known yet — rotate after jittered backoff (4.1). */ + if (code == E_XCE_NOMASTER) return XCE_FO_RETRY_BACKOFF; + /* bug-1850: the controller answered, so this connection works; it is only + * briefly not ready (electing, or a primary that lost its write quorum or + * hit commit-wait admission). Back off and retry the SAME controller -- + * rotating away from a healthy primary forced a reconnect + rejoin + full + * re-push. If it turns out to be a standby it answers REDIRECT/STANDBY/ + * NOMASTER next time, and if it dies the transport error rotates. */ + if (code == E_XCE_NOT_READY) + return XCE_FO_RETRY_SAME; + /* Transport-level failures — reconnect/rotate and retry now (4.4). */ if (code == ECONNREFUSED || code == ETIMEDOUT || code == ECONNRESET) return XCE_FO_RETRY_NOW; @@ -304,6 +312,7 @@ bool xcecli_retry_decide(api_ctx_t* ac, int* rc, bool is_write, case XCE_FO_RETRY_NOW: case XCE_FO_RETRY_BACKOFF: + case XCE_FO_RETRY_SAME: break; } @@ -313,14 +322,15 @@ bool xcecli_retry_decide(api_ctx_t* ac, int* rc, bool is_write, /* Budget exhausted. Distinguish the cause (design D6): a no-primary * window maps to the distinct cluster-not-ready code; a transport-only * outage keeps the last transport rc so the caller still sees it. */ - if (action == XCE_FO_RETRY_BACKOFF) + if ((action == XCE_FO_RETRY_BACKOFF) || (action == XCE_FO_RETRY_SAME)) *rc = -E_XCE_NOT_READY; return false; } /* Compute the pre-attempt delay: none for prompt rotation (RETRY_NOW), * a jittered exponential backoff for the no-primary case (RETRY_BACKOFF). */ - st->delay_ms = (action == XCE_FO_RETRY_BACKOFF) + st->delay_ms = ((action == XCE_FO_RETRY_BACKOFF) + || (action == XCE_FO_RETRY_SAME)) ? fo_backoff_delay_ms(cfg, st->attempt, &st->rand_seed) : 0; return true; @@ -361,6 +371,16 @@ bool xcecli_retry_next(api_ctx_t* ac, int* rc, bool is_write, if (action == XCE_FO_RETRY_BACKOFF) fo_sleep_ms(st->delay_ms); + /* bug-1850: RETRY_SAME -- back off and re-send on the current connection; + * no rotate, no reconnect, no rejoin. */ + if (action == XCE_FO_RETRY_SAME) { + fo_sleep_ms(st->delay_ms); + INFO("xcecli failover: attempt %d retrying the same controller after " + "%dms (status=%d: not ready)", + st->attempt, st->delay_ms, *rc); + return true; + } + /* Re-point the endpoint to the redirect hint or the next master and * reconnect (2.x, 4.2, 4.4). Log the rotation target (7.1). A rotate * failure is not fatal here: the next xce_* attempt observes the dead diff --git a/src/lib/testsuite/failover_harness.h b/src/lib/testsuite/failover_harness.h index ec27e56c..bee6c86f 100644 --- a/src/lib/testsuite/failover_harness.h +++ b/src/lib/testsuite/failover_harness.h @@ -71,8 +71,9 @@ const xce_candidate_t *fo_cursor_current(const fo_master_cursor_t *cur); typedef enum { FO_ACT_SUCCESS = 0, /* status 0, or idempotent DUP replay on a re-sent write */ FO_ACT_RETRY_NOW, /* REDIRECT / STANDBY / transport error — rotate, no backoff */ - FO_ACT_RETRY_BACKOFF,/* NOMASTER / NOT_READY — rotate after jittered backoff */ - FO_ACT_TERMINAL /* non-retriable — surface to caller */ + FO_ACT_RETRY_BACKOFF,/* NOMASTER — rotate after jittered backoff */ + FO_ACT_TERMINAL, /* non-retriable — surface to caller */ + FO_ACT_RETRY_SAME /* NOT_READY — backoff, retry the SAME controller */ } fo_expected_action_t; typedef struct { diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index aed94bfe..a711c3e8 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -116,8 +116,12 @@ const fo_status_row_t FO_STATUS_TABLE[] = { /* no-primary / not-ready — rotate after backoff (4.1) */ { "E_XCE_NOMASTER", E_XCE_NOMASTER, false, false, FO_ACT_RETRY_BACKOFF, "standby knows no primary yet — backoff + rotate (4.1)" }, - { "E_XCE_NOT_READY", E_XCE_NOT_READY, false, false, FO_ACT_RETRY_BACKOFF, - "controller initializing/electing — backoff + rotate (4.1)" }, + /* bug-1850: the controller answered, so the connection works -- it is + * briefly not ready (electing, or a primary that lost its write quorum). + * Back off and retry the same controller; rotating away from a healthy + * primary forced a scheduler rejoin + full re-push. */ + { "E_XCE_NOT_READY", E_XCE_NOT_READY, false, false, FO_ACT_RETRY_SAME, + "controller not ready — backoff, retry the same controller" }, /* idempotent-replay duplicate (write replay) — DUP/EEXIST (5.2) */ { "E_XCE_DUP_JOB", E_XCE_DUP_JOB, false, false, FO_ACT_TERMINAL, @@ -290,7 +294,7 @@ static void test_status_table_is_complete_and_resolves(void) fo_status_lookup(E_XCE_STANDBY)->expect); TEST_ASSERT_EQUAL_INT(FO_ACT_RETRY_BACKOFF, fo_status_lookup(E_XCE_NOMASTER)->expect); - TEST_ASSERT_EQUAL_INT(FO_ACT_RETRY_BACKOFF, + TEST_ASSERT_EQUAL_INT(FO_ACT_RETRY_SAME, fo_status_lookup(E_XCE_NOT_READY)->expect); TEST_ASSERT_EQUAL_INT(FO_ACT_RETRY_NOW, fo_status_lookup(-ECONNRESET)->expect); @@ -372,6 +376,7 @@ static xce_fo_action_t fo_expected_to_action(fo_expected_action_t e) case FO_ACT_RETRY_NOW: return XCE_FO_RETRY_NOW; case FO_ACT_RETRY_BACKOFF: return XCE_FO_RETRY_BACKOFF; case FO_ACT_TERMINAL: return XCE_FO_TERMINAL; + case FO_ACT_RETRY_SAME: return XCE_FO_RETRY_SAME; } return XCE_FO_TERMINAL; } @@ -867,6 +872,61 @@ static void test_decide_no_primary_exhausts_to_not_ready(void) * loop stops with success and the master cursor advanced across the retries * (the rotation side runs in the wrapper; here we advance it explicitly to * mirror that and assert the sticky cursor moved). */ +/* bug-1850: NOT_READY still backs off (and exhausts to -E_XCE_NOT_READY), + * but as RETRY_SAME -- the caller must not rotate. */ +static void test_decide_not_ready_backs_off_on_same_controller(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); + int rc = E_XCE_NOT_READY; + xce_fo_action_t action; + + TEST_ASSERT_TRUE(xcecli_retry_decide(&ac, &rc, false, &cfg, &st, &action)); + TEST_ASSERT_EQUAL_INT(XCE_FO_RETRY_SAME, action); + TEST_ASSERT_TRUE(st.delay_ms > 0); + + for (int i = 0; i < cfg.max_retries + 1; i++) { + rc = E_XCE_NOT_READY; + if (!xcecli_retry_decide(&ac, &rc, false, &cfg, &st, &action)) + break; + } + TEST_ASSERT_EQUAL_INT(-E_XCE_NOT_READY, rc); + printf("test_decide_not_ready_backs_off_on_same_controller: PASS\n"); +} + +/* bug-1850: the live retry loop must not rotate (reconnect to the next + * master, fire the rejoin hook) on NOT_READY from a controller that + * answered -- a healthy primary briefly without write quorum was abandoned + * for the standbys, forcing a scheduler rejoin and a full re-push. */ +static void test_retry_next_not_ready_does_not_rotate(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + ac.failover.retry = fo_test_retry_cfg(); + ac.failover.retry.initial_retry_ms = 1; + ac.failover.retry.max_retry_ms = 2; + + xcecli_retry_t st = XCECLI_RETRY_INIT(&ac); + int rc = E_XCE_NOT_READY; + + TEST_ASSERT_TRUE(xcecli_retry_next(&ac, &rc, false, &st)); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, ac.failover.cur_master, + "NOT_READY must be retried on the same controller, not rotated"); + + /* a genuine redirect still rotates */ + rc = E_XCE_STANDBY; + TEST_ASSERT_TRUE(xcecli_retry_next(&ac, &rc, false, &st)); + TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master); + printf("test_retry_next_not_ready_does_not_rotate: PASS\n"); +} + static void test_decide_no_primary_then_becomes_primary(void) { xce_client_config_t conf; @@ -949,7 +1009,7 @@ static void test_decide_backoff_grows_and_is_bounded(void) int prev_base = 0; for (int i = 0; i < 8; i++) { - rc = E_XCE_NOT_READY; + rc = E_XCE_NOMASTER; bool again = xcecli_retry_decide(&ac, &rc, false, &cfg, &st, &action); TEST_ASSERT_TRUE(again); TEST_ASSERT_EQUAL_INT(XCE_FO_RETRY_BACKOFF, action); @@ -1844,5 +1904,7 @@ void test_libxce_retry(void) RUN_TEST(test_is_connection_broken_false_on_run); RUN_TEST(test_is_connection_broken_null_safe); RUN_TEST(test_ff_call_is_single_shot_no_budget); + RUN_TEST(test_decide_not_ready_backs_off_on_same_controller); + RUN_TEST(test_retry_next_not_ready_does_not_rotate); UNITY_END(); }