diff --git a/.kiro/specs/libxce-failover/tasks.md b/.kiro/specs/libxce-failover/tasks.md index 4feb205..27cc0ae 100644 --- a/.kiro/specs/libxce-failover/tasks.md +++ b/.kiro/specs/libxce-failover/tasks.md @@ -71,7 +71,7 @@ - _Requirements: 1.1, 1.3, 2.4, 5.1, 5.3_ - _Depends: 3.1, 3.2, 4.1_ - _Boundary: XcecliRetryLoop, FailoverState_ -- [ ] 5.2 Apply best-effort reconnect to fire-and-forget ops +- [x] 5.2 Apply best-effort reconnect to fire-and-forget ops - Give the notify, heartbeat, and wakeup ops a transport-reconnect-only variant: reconnect on a known-dead connection, with no status classification and no no-primary backoff - Done: a fire-and-forget op on a dead connection triggers a reconnect attempt and never blocks on the no-primary backoff budget; lost-notification redelivery remains out of scope - _Requirements: 1.1, 5.3_ @@ -108,3 +108,4 @@ - 3.2: `xcecli_ctx_recover` (socket-free): flips ac->ctx.state ERR→RUN ONLY, drains stale wbuf via non-blocking cbuf_tryget (-EAGAIN on empty); wired into xcecli_retry_next after successful rotate, before return true. THIS is the -EINVAL bug fix. Added missing prototype `extern int cbuf_tryget(cbuf_t*,void*)` to mylock.h (function already defined mylock.c:166, just undeclared). - 4.1: public `xcecli_reconnect_cb` typedef + `xcecli_set_reconnect_handler` (client.h/client.c, stores cb/arg in failover); socket-free `xcecli_invoke_rejoin` wired into xcecli_retry_next AFTER rotate+recover, BEFORE resume. hook<0 = failed attempt → bounded tail-recursion (consumes attempt budget, capped at max_retries). typedef MOVED from client_internal.h to client.h. API additive. - 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. diff --git a/src/lib/client.c b/src/lib/client.c index ed8713d..d94286c 100644 --- a/src/lib/client.c +++ b/src/lib/client.c @@ -211,7 +211,10 @@ int xcecli_nop(xcecli_ctx_t c) return -EINVAL; } - return xce_nop(&(ctx->ctx)); + /* Fire-and-forget: best-effort transport reconnect on a known-dead + * connection, then a single-shot send. No status classification, no + * no-primary backoff, no retry budget (task 5.2). */ + return XCECLI_FF_CALL(ctx, xce_nop(&(ctx->ctx))); } int xcecli_heartbeat(xcecli_ctx_t c, int hb_interval) @@ -222,7 +225,10 @@ int xcecli_heartbeat(xcecli_ctx_t c, int hb_interval) return -EINVAL; } - return xce_heartbeat(&(ctx->ctx), hb_interval); + /* Fire-and-forget: best-effort transport reconnect on a known-dead + * connection, then a single-shot send (task 5.2). A heartbeat must NEVER go + * through the no-primary backoff retry budget. */ + return XCECLI_FF_CALL(ctx, xce_heartbeat(&(ctx->ctx), hb_interval)); } int xcecli_shutdown(xcecli_ctx_t c) @@ -403,7 +409,12 @@ int xcecli_notify(xcecli_ctx_t c, xce_notify_data_t* data) return -EINVAL; } - return xce_notify(&(ctx->ctx), data); + /* Fire-and-forget chokepoint: every thin notify_* wrapper delegates + * here, so applying the best-effort reconnect at this single point covers + * them all without double-wrapping/double-locking. Best-effort transport + * reconnect on a known-dead connection, then a single-shot send (task + * 5.2). */ + return XCECLI_FF_CALL(ctx, xce_notify(&(ctx->ctx), data)); } int xcecli_notify_jobstart(xcecli_ctx_t c, job_pidinfo_t* pidinfo) diff --git a/src/lib/client_failover.c b/src/lib/client_failover.c index 63c1f46..b3a0cb1 100644 --- a/src/lib/client_failover.c +++ b/src/lib/client_failover.c @@ -189,6 +189,22 @@ int xcecli_ctx_recover(api_ctx_t* ac) return 0; } +/* ════════════════════════════════════════════════════════════════════════ + * Fire-and-forget best-effort reconnect predicate (task 5.2) + * ════════════════════════════════════════════════════════════════════════ */ + +bool xcecli_ff_needs_reconnect(const api_ctx_t* ac) +{ + if (ac == NULL) + return false; + + /* Only a transport-ERR connection (set by the disconnect path) is treated + * as known-dead and reconnected best-effort. A live RUN ctx sends directly; + * a deliberately-stopped DOWN or not-yet-started INIT ctx is never + * resurrected by a fire-and-forget op. */ + return ac->ctx.state == XCE_CTX_ERR; +} + /* ════════════════════════════════════════════════════════════════════════ * ReconnectRejoinHook — invoke the consumer rejoin callback (1.2) * ════════════════════════════════════════════════════════════════════════ */ diff --git a/src/lib/client_internal.h b/src/lib/client_internal.h index 3b172cc..36398ac 100644 --- a/src/lib/client_internal.h +++ b/src/lib/client_internal.h @@ -192,6 +192,60 @@ bool xcecli_retry_next(api_ctx_t* ac, int* rc, bool is_write, */ int xcecli_ctx_recover(api_ctx_t* ac); +/* + * xcecli_ff_needs_reconnect (task 5.2) — PURE predicate for the fire-and-forget + * best-effort reconnect decision. Fire-and-forget ops (notify_*, heartbeat, nop) + * send a *_req only — there is NO reply to classify, so they must NOT go through + * the full failover retry loop (no status classification, no no-primary backoff, + * no retry budget; a heartbeat must never block on the backoff budget). Instead + * the FF path makes AT MOST ONE best-effort transport reconnect when the + * connection is KNOWN-DEAD, then does a single-shot send (which may be lost — + * acceptable; redelivery is deferred to worker-rejoin-resync). + * + * The "known-dead" signal is ctx->state == XCE_CTX_ERR (set by the disconnect + * path after a detected transport failure). Returns true ONLY for ERR so that a + * deliberately-stopped (DOWN) or not-yet-started (INIT) ctx is never resurrected + * by a fire-and-forget op. NULL-safe: returns false. Socket-free + side-effect + * free so it is unit-testable. + */ +bool xcecli_ff_needs_reconnect(const api_ctx_t* ac); + +/* + * XCECLI_FF_CALL(ac, EXPR) — the fire-and-forget best-effort wrapper (task 5.2). + * Mirrors XCECLI_FAILOVER_CALL's per-ctx locking (CI2 — shared-ctx safety) but + * is single-shot, NOT a retry loop: there is no reply to classify for an FF op, + * so the full failover machinery (classification, no-primary backoff, retry + * budget) is deliberately NOT used. Putting a heartbeat through the retry loop + * would let it block on the no-primary backoff budget — explicitly forbidden. + * + * Behaviour, holding ac->failover.lock for the whole sequence: + * 1. if the connection is KNOWN-DEAD (xcecli_ff_needs_reconnect), attempt ONE + * best-effort reconnect: xce_failover_rotate(ac) + xcecli_ctx_recover(ac). + * A failed reconnect is non-fatal — the send below is simply best-effort. + * 2. evaluate EXPR exactly ONCE (the unchanged one-shot xce_* fire-and-forget + * send) and yield its rc verbatim. A lost send rc is surfaced as-is, never + * retried. + * + * ac — api_ctx_t* (already NULL-checked by the caller's guard) + * EXPR — the unchanged one-shot xce_* *_req-only call + * + * Implemented as a GCC/clang statement-expression so it yields the send rc and + * keeps each public xcecli_* signature/return convention intact (R1.3). + */ +#define XCECLI_FF_CALL(ac, EXPR) \ + __extension__({ \ + api_ctx_t* _ff_ac = (ac); \ + int _ff_rc; \ + mylock_lock(&_ff_ac->failover.lock); \ + if (xcecli_ff_needs_reconnect(_ff_ac)) { \ + (void)xce_failover_rotate(_ff_ac); \ + xcecli_ctx_recover(_ff_ac); \ + } \ + _ff_rc = (EXPR); \ + mylock_unlock(&_ff_ac->failover.lock); \ + _ff_rc; \ + }) + /* * xcecli_invoke_rejoin (task 4.1) — invoke the consumer rejoin hook once after a * successful reconnect/recovery and BEFORE the interrupted op resumes. Socket- diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index a6f619f..8d6a5c6 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -1395,6 +1395,125 @@ static void test_failover_call_success_runs_once(void) printf("test_failover_call_success_runs_once: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * Task 5.2 — Best-effort reconnect for fire-and-forget ops. + * + * Fire-and-forget ops (notify_*, heartbeat, nop) send a *_req only — there is + * NO reply to classify, so they do NOT use the full failover loop (no status + * classification, no no-primary backoff, no retry budget). Putting a heartbeat + * through the retry loop would let it block on the no-primary backoff budget, + * which is explicitly forbidden. Instead the FF path makes AT MOST ONE + * best-effort transport reconnect when the connection is KNOWN-DEAD, then does + * a single-shot send (which may be lost — acceptable, redelivery is deferred to + * worker-rejoin-resync). + * + * The "known-dead" signal is ctx->state == XCE_CTX_ERR (set by the disconnect + * path after a detected transport failure). xcecli_ff_needs_reconnect is the + * pure predicate factored out so this decision is unit-testable socket-free: + * ERR → true (reconnect once, best-effort) + * RUN → false (connection live, send directly) + * DOWN → false (deliberately stopped — do not resurrect) + * INIT → false (not yet started) + * + * Requirements: 1.1 (all ops go through failover machinery — FF via the + * best-effort transport-reconnect variant), 5.3 (no silent drop on a path that + * CAN reconnect; a known-dead connection is repaired before the single shot). + * Full live reconnect-then-send is E2E task 6.2. + * ════════════════════════════════════════════════════════════════════════ */ + +/* Req 1.1/5.3: a known-dead (ERR) connection needs a best-effort reconnect. */ +static void test_ff_needs_reconnect_on_err(void) +{ + api_ctx_t ac; + memset(&ac, 0, sizeof(ac)); + ac.ctx.state = XCE_CTX_ERR; + + TEST_ASSERT_TRUE(xcecli_ff_needs_reconnect(&ac)); + + printf("test_ff_needs_reconnect_on_err: PASS\n"); +} + +/* Req 1.1: a live (RUN) connection needs NO reconnect — send directly. */ +static void test_ff_no_reconnect_on_run(void) +{ + api_ctx_t ac; + memset(&ac, 0, sizeof(ac)); + ac.ctx.state = XCE_CTX_RUN; + + TEST_ASSERT_FALSE(xcecli_ff_needs_reconnect(&ac)); + + printf("test_ff_no_reconnect_on_run: PASS\n"); +} + +/* Req 1.1: a DOWN (deliberately stopped) or INIT (not yet started) ctx is NOT + * resurrected by a fire-and-forget op — only a transport-ERR connection is. */ +static void test_ff_no_reconnect_on_down_or_init(void) +{ + api_ctx_t ac; + + memset(&ac, 0, sizeof(ac)); + ac.ctx.state = XCE_CTX_DOWN; + TEST_ASSERT_FALSE(xcecli_ff_needs_reconnect(&ac)); + + memset(&ac, 0, sizeof(ac)); + ac.ctx.state = XCE_CTX_INIT; + TEST_ASSERT_FALSE(xcecli_ff_needs_reconnect(&ac)); + + printf("test_ff_no_reconnect_on_down_or_init: PASS\n"); +} + +/* Guard: a NULL api_ctx is rejected as "no reconnect" without crashing. */ +static void test_ff_needs_reconnect_null_safe(void) +{ + TEST_ASSERT_FALSE(xcecli_ff_needs_reconnect(NULL)); + printf("test_ff_needs_reconnect_null_safe: PASS\n"); +} + +/* Req 5.3 (structural): the fire-and-forget path is single-shot — it makes AT + * MOST ONE reconnect attempt and NEVER consumes the no-primary retry budget / + * sleeps. We prove this structurally: the FF macro drives the body exactly once + * regardless of the rc it returns (a "lost" send rc is surfaced verbatim, NOT + * retried), and on a known-dead ctx the single reconnect attempt fires exactly + * once before that single body invocation. */ +typedef struct { + api_ctx_t *ac; + int body_calls; /* number of times the FF body ran */ + int body_rc; /* rc the body returns each call */ +} fo_ff_probe_t; + +static int fo_ff_body(fo_ff_probe_t *p) +{ + p->body_calls++; + return p->body_rc; +} + +static void test_ff_call_is_single_shot_no_budget(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + TEST_ASSERT_EQUAL_INT(0, mylock_init(&ac.failover.lock)); + ac.failover.retry = XCE_RETRY_DEFAULTS; + /* a live RUN ctx: no reconnect, just the single-shot send */ + ac.ctx.state = XCE_CTX_RUN; + + fo_ff_probe_t p; + memset(&p, 0, sizeof(p)); + p.ac = ∾ + + /* a "lost" fire-and-forget send (e.g. -ECONNRESET) is surfaced verbatim and + * is NOT retried — the body must run exactly once. */ + p.body_rc = -ECONNRESET; + int rc = XCECLI_FF_CALL(&ac, fo_ff_body(&p)); + mylock_fini(&ac.failover.lock); + + TEST_ASSERT_EQUAL_INT(-ECONNRESET, rc); /* best-effort: surfaced as-is */ + TEST_ASSERT_EQUAL_INT(1, p.body_calls); /* single shot, no retry loop */ + + printf("test_ff_call_is_single_shot_no_budget: PASS\n"); +} + /* ── Suite entry point ──────────────────────────────────────────────── */ void test_libxce_retry(void) @@ -1440,5 +1559,10 @@ void test_libxce_retry(void) RUN_TEST(test_invoke_rejoin_null_ctx_safe); RUN_TEST(test_failover_call_serialises_shared_ctx); RUN_TEST(test_failover_call_success_runs_once); + RUN_TEST(test_ff_needs_reconnect_on_err); + RUN_TEST(test_ff_no_reconnect_on_run); + RUN_TEST(test_ff_no_reconnect_on_down_or_init); + RUN_TEST(test_ff_needs_reconnect_null_safe); + RUN_TEST(test_ff_call_is_single_shot_no_budget); UNITY_END(); }