diff --git a/.kiro/specs/libxce-failover/tasks.md b/.kiro/specs/libxce-failover/tasks.md index 1a5e32c6..57fd4274 100644 --- a/.kiro/specs/libxce-failover/tasks.md +++ b/.kiro/specs/libxce-failover/tasks.md @@ -86,7 +86,7 @@ - _Boundary: ControllerRedirectAddress_ - [ ] 6. Validation: unit suite and live failover -- [ ] 6.1 Bring the realigned unit suite green +- [x] 6.1 Bring the realigned unit suite green - Make the retry tests pass for classification, budget-to-cluster-not-ready, sticky cursor, hint-first then rotate, duplicate-replay-to-success, jitter and many-client staggering, stale-reply drain, context-state recovery, and concurrent shared-context use - Done: the full libxce retry unit suite passes under the project's test and ASAN build - _Requirements: 2.4, 2.5, 3.2, 3.3, 4.1, 4.2, 4.3, 4.4, 4.5, 5.2, 6.2, 6.3, 8.3, 8.4_ @@ -110,3 +110,4 @@ - 5.1: `XCECLI_FAILOVER_CALL(ac,is_write,EXPR)` macro (client_internal.h) = lock(failover.lock) → retry init → do{rc=EXPR}while(xcecli_retry_next) → unlock → yield rc. Wrapped 18 reply-bearing ops; writes=true for jobsubmit/jobdispatch/jobctrl/queuectrl/hostctrl/update_resvals/update_jusage/runtask/join/set_pend_reason/clear_reason, reads=false otherwise. R1.3: zero signature changes. DESIGN CORRECTION: `xce_nop` is fire-and-forget (no cbuf_get, xceapi.c:30) — moved nop reply-read→fire-and-forget in design OpCategory; nop wraps in 5.2 not 5.1. - 5.2: `XCECLI_FF_CALL` macro (single-shot, NO retry loop/backoff/budget): lock → if known-dead (xcecli_ff_needs_reconnect = ctx.state==XCE_CTX_ERR) one rotate+recover → one send → unlock. Applied at the 3 wire chokepoints xcecli_notify/heartbeat/nop (notify_* delegate to xcecli_notify). wakeup/try_wakeup are LOCAL-only (cbuf_tryput own wbuf, no wire send) so correctly NOT wrapped. - 5.3: `fill_redirect_reply(xce_reply_t*, const ha_state_t*)` (non-static, join.c) sets redirect_host=ha.primary_host + redirect_port=(uint16_t)ha.primary_port; called ONLY on the E_XCE_REDIRECT branch before xce_reply. Audit: join.c is the SOLE E_XCE_REDIRECT emitter. NOMASTER path unchanged (8.2). host alias (no copy) safe: ha lives cluster-lifetime, xce_reply serializes synchronously. New test_join_redirect.c registered in controller_test.c + meson.build. +- 6.1: full lib suite green (161 tests). All 14 reqs (2.4,2.5,3.2,3.3,4.1-4.5,5.2,6.2,6.3,8.3,8.4) map to genuine passing tests. Gap-filled 6.3 with `test_decide_jitter_staggers_many_clients` (64 distinct-seed carriers → staggered delays, anti-thundering-herd). No production bugs found. diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index 8d6a5c6d..f6a532c8 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -1022,6 +1022,116 @@ static void test_decide_transport_exhaustion_keeps_last_rc(void) printf("test_decide_transport_exhaustion_keeps_last_rc: 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 + * XCECLI_RETRY_INIT does in production), so N independent carriers driven through + * the SAME no-primary backoff attempt must produce delays that VARY across + * clients (not a single shared value) while every delay stays within the + * [base/2, max_retry_ms] envelope. This drives the pure xcecli_retry_decide path + * — socket-free — for many concurrent carriers and proves the anti-thundering- + * herd property the single-carrier growth/bounds test does not. */ +#define FO_JITTER_NCLIENTS 64 + +typedef struct { + api_ctx_t *ac; /* shared cfg source; seeds are per-carrier */ + const xce_retry_config_t *cfg; + int delays[FO_JITTER_NCLIENTS]; /* one client's computed delay */ + pthread_mutex_t lock; +} fo_jitter_ctx_t; + +/* Each "client" runs one no-primary attempt through a FRESH retry carrier seeded + * from a distinct value (its thread index), mirroring how distinct api_ctx + * pointers seed distinct carriers. We record the jittered RETRY_BACKOFF delay it + * computed for the first no-primary retry. */ +static void fo_jitter_worker(void *ctx, int idx) +{ + fo_jitter_ctx_t *c = (fo_jitter_ctx_t *)ctx; + + /* a distinct carrier per client: same init shape as XCECLI_RETRY_INIT but a + * distinct seed per client (production seeds from the distinct ctx pointer; + * here each thread stands in for a distinct client). */ + xcecli_retry_t st = XCECLI_RETRY_INIT(c->ac); + st.rand_seed = (unsigned int)(idx + 1) * 2654435761u; /* distinct/client */ + + int rc = E_XCE_NOMASTER; /* no primary → RETRY_BACKOFF */ + xce_fo_action_t action; + bool again = xcecli_retry_decide(c->ac, &rc, /*is_write=*/false, c->cfg, &st, + &action); + + /* every client agrees on the action and stays within budget on attempt 1 */ + TEST_ASSERT_TRUE(again); + TEST_ASSERT_EQUAL_INT(XCE_FO_RETRY_BACKOFF, action); + TEST_ASSERT_TRUE(st.delay_ms > 0); + TEST_ASSERT_TRUE(st.delay_ms <= c->cfg->max_retry_ms); + + pthread_mutex_lock(&c->lock); + c->delays[idx] = st.delay_ms; + pthread_mutex_unlock(&c->lock); +} + +static void test_decide_jitter_staggers_many_clients(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + + /* a wide backoff window so jitter has room to spread: base after the first + * retry is initial_retry_ms, jitter ∈ [base/2, base] = [100, 200]. */ + xce_retry_config_t cfg = XCE_RETRY_DEFAULTS; + cfg.max_retries = 20; + cfg.initial_retry_ms = 200; + cfg.max_retry_ms = 5000; + cfg.backoff_multiplier = 2.0; + + fo_jitter_ctx_t jc; + memset(&jc, 0, sizeof(jc)); + jc.ac = ∾ + jc.cfg = &cfg; + TEST_ASSERT_EQUAL_INT(0, pthread_mutex_init(&jc.lock, NULL)); + + int rc = fo_run_threads(FO_JITTER_NCLIENTS, fo_jitter_worker, &jc); + pthread_mutex_destroy(&jc.lock); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* Count distinct delay values: lock-step retry would yield a SINGLE value + * across all clients. Jitter must spread them across many distinct delays. */ + int distinct = 0; + int min_d = jc.delays[0]; + int max_d = jc.delays[0]; + for (int i = 0; i < FO_JITTER_NCLIENTS; i++) { + bool seen = false; + for (int j = 0; j < i; j++) { + if (jc.delays[j] == jc.delays[i]) { + seen = true; + break; + } + } + if (!seen) + distinct++; + if (jc.delays[i] < min_d) + min_d = jc.delays[i]; + if (jc.delays[i] > max_d) + max_d = jc.delays[i]; + /* every client's delay stays within the jittered envelope [base/2, base] + * for the first retry: base == initial_retry_ms == 200 here. */ + TEST_ASSERT_TRUE(jc.delays[i] >= cfg.initial_retry_ms / 2); + TEST_ASSERT_TRUE(jc.delays[i] <= cfg.initial_retry_ms); + } + + /* the decisive anti-thundering-herd assertion: the clients did NOT retry in + * lockstep — their delays span many distinct values, not one shared delay. */ + TEST_ASSERT_TRUE(distinct > 1); + /* with 64 clients over a ~100ms jitter span the spread must be substantial */ + TEST_ASSERT_TRUE(distinct >= FO_JITTER_NCLIENTS / 4); + TEST_ASSERT_TRUE(max_d > min_d); /* a real spread, not a constant */ + + printf("test_decide_jitter_staggers_many_clients: PASS " + "(%d distinct delays across %d clients, span [%d,%d]ms)\n", + distinct, FO_JITTER_NCLIENTS, min_d, max_d); +} + /* ════════════════════════════════════════════════════════════════════════ * Task 3.2 — Reconnect recovery (xcecli_ctx_recover) RED→GREEN tests. * @@ -1547,6 +1657,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_jitter_staggers_many_clients); RUN_TEST(test_recover_resets_err_state_to_run); RUN_TEST(test_recover_leaves_run_state_untouched); RUN_TEST(test_recover_leaves_down_state_untouched);