myxce/src/lib/client_internal.h
Rongsong Shen 40c2351578 fix(client): bug-1436 count-aware failover macro for xce_get-family calls
xcecli_joblist/reslist/workerlist/resval/jobinfo return a non-negative ROW
COUNT on success (rc = getreply.nrows), not a 0/status code. They were
wrapped in XCECLI_FAILOVER_CALL, which unconditionally classifies the raw
return value against fixed constants (E_XCE_* and POSIX errno magnitudes
accepted in either sign) — a row count that happens to equal one of those
constants (e.g. exactly 111 jobs == ECONNREFUSED) was silently
misclassified as a transport failure, forcing a spurious
xce_failover_rotate() on every call with that exact count. In
/xce-test-ha's arithmetic progression (3 + 3*round) this hit at round 36,
burning the retry budget on self-inflicted churn until it coincided with
a genuine transient and surfaced a real ECONNRESET to the caller.

Add XCECLI_FAILOVER_CALL_COUNT: only enters the retry/classify loop when
EXPR's raw result is negative; any non-negative result (0 or a genuine
row count of any magnitude) is unconditionally success. Switch the five
xce_get-family wrappers to it. Verified via 40-round sequential and
40-round chaotic HA soak past round 36, zero committed-job loss.
2026-07-13 07:18:52 +00:00

322 lines
16 KiB
C

#if !defined(_CLIENT_INTERNAL_H_)
#define _CLIENT_INTERNAL_H_ 1
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
#include "client.h"
#include "mylock.h"
#include "netio.h"
#include "request.h"
#include "xce/client_failover.h" /* xce_fo_action_t */
#include "xcectx.h"
/*
* client_internal.h — private libxce client state shared between client.c and
* the forthcoming failover helper (client_failover.c, task 1.x).
*
* api_ctx_t was previously file-local to client.c. It is moved here so the
* failover helper can operate on it without exposing it through the public
* API in client.h.
*/
typedef enum {
API_CTX_INIT = 0,
API_CTX_RUN,
API_CTX_DOWN,
API_CTX_ERR
} api_ctx_state_t;
/*
* xcecli_reconnect_cb (consumer rejoin hook) and its public setter
* xcecli_set_reconnect_handler() are declared in the public client.h (included
* above) as of task 4.1. The failover state below carries the registered hook.
*/
/*
* xce_failover_state_t: per-context failover state. Persists across calls so a
* sticky primary and the retry tunables survive between requests.
*
* lock — serializes xcecli_* per ctx (shared-ctx safety)
* cur_master — index into conf->masters[] (sticky primary)
* retry — defaults from XCE_RETRY_DEFAULTS unless overridden
* rejoin_cb — consumer rejoin hook; NULL for pure clients
* rejoin_arg — opaque argument passed to rejoin_cb
*/
typedef struct {
mylock_t lock;
int cur_master;
xce_retry_config_t retry;
xcecli_reconnect_cb rejoin_cb;
void* rejoin_arg;
} xce_failover_state_t;
typedef struct {
xce_client_config_t* conf;
endpoint_t ep;
xce_ctx_t ctx;
xce_failover_state_t failover;
} api_ctx_t;
/*
* EndpointRotator (task 2.2) — these operate on the private api_ctx_t, so they
* live in this internal header rather than the public xce/client_failover.h.
*
* xce_failover_next_target(): PURE target-selection (no I/O). If a redirect-
* address hint is set on ac->ctx, write it out and CONSUME it (one-shot,
* advisory). Otherwise advance the sticky master cursor (ac->failover.cur_master)
* to the next configured master in ac->conf, wrapping around. Returns 0,
* -EINVAL on a NULL/zero argument, or -ENOENT if no masters and no hint.
* Correctness never depends on the hint.
*
* xce_failover_rotate(): close the stale connection on ac->ep and reconnect the
* SAME endpoint object to the chosen target. Thin wrapper over the selector +
* endpoint_connect(); the live socket path is exercised by the E2E task.
*/
int xce_failover_next_target(api_ctx_t* ac, char* host_out, size_t host_sz,
uint16_t* port_out);
int xce_failover_rotate(api_ctx_t* ac);
/*
* XcecliRetryLoop / decision helper (task 3.1) — see design.md
* "Components and Interfaces → xcecli_retry_next".
*
* xcecli_retry_t: per-op retry carrier. One instance lives on the caller's
* stack for the duration of a single xcecli_* op's do/while loop.
* attempt — number of re-sends so far (0 on the first send).
* delay_ms — backoff to apply BEFORE the next attempt (0 for RETRY_NOW).
* rand_seed — per-op rand_r() state for jittered backoff (6.3).
*/
typedef struct {
int attempt;
int delay_ms;
unsigned int rand_seed;
} xcecli_retry_t;
/*
* XCECLI_RETRY_INIT(ac): initialise a retry carrier from an api_ctx_t. The seed
* mixes the ctx pointer so concurrent ops on distinct ctxs jitter independently.
*/
#define XCECLI_RETRY_INIT(ac) \
((xcecli_retry_t){ .attempt = 0, \
.delay_ms = 0, \
.rand_seed = (unsigned int)(uintptr_t)(ac) })
/*
* xcecli_retry_decide(): the PURE decision core (no I/O, no sleep, no socket,
* no logging) so it is unit-testable. Given the rc returned by the last xce_*
* attempt, it:
* - classifies *rc via xce_failover_classify(*rc, is_write, st->attempt);
* - on SUCCESS applies the DUP-replay rule (sets *rc = 0) and returns false;
* - on TERMINAL returns false leaving *rc as-is;
* - on a retriable action advances st->attempt, computes st->delay_ms
* (0 for RETRY_NOW; a jittered exponential backoff bounded by
* cfg->max_retry_ms for RETRY_BACKOFF), reports the action in *action_out,
* and returns true — UNLESS the retry budget (cfg->max_retries) is now
* exhausted, in which case it returns false and, for a no-primary cause,
* sets *rc = -E_XCE_NOT_READY (a transport-only cause keeps the last rc).
*
* It does NOT reset ctx state, drain wbuf, sleep, rotate, or invoke the rejoin
* hook — those are the side-effecting wrapper's job (xcecli_retry_next) and
* later tasks (3.2 state-reset/drain, 4.1 rejoin hook).
*/
bool xcecli_retry_decide(api_ctx_t* ac, int* rc, bool is_write,
const xce_retry_config_t* cfg, xcecli_retry_t* st,
xce_fo_action_t* action_out);
/*
* xcecli_retry_next(): the side-effecting wrapper the xcecli_* do/while loops
* call. = xcecli_retry_decide() + (RETRY_BACKOFF: a real st->delay_ms sleep) +
* (any retry: xce_failover_rotate(ac) + INFO log of the rotation target, 7.1) +
* (budget exhaustion: WARN log of the attempt count, 7.2). Returns true → the
* caller re-invokes its xce_* op; false → stop, *rc is final.
*/
bool xcecli_retry_next(api_ctx_t* ac, int* rc, bool is_write,
xcecli_retry_t* st);
/*
* XCECLI_FAILOVER_CALL(ac, is_write, EXPR) — the shared XcecliRetryLoop wrapper
* (task 5.1). Turns a one-shot reply-bearing xce_* call into a transparent
* failover loop, holding the per-ctx lock (ac->failover.lock) for the FULL loop
* duration so concurrent ops on a shared ctx are serialised (CI2): the
* single-slot wbuf already implies one in-flight request per ctx; the lock makes
* that explicit and covers the reconnect/rotate window.
*
* ac — api_ctx_t* (already NULL-checked by the caller's guard)
* is_write — true for mutating ops (DUP-on-replay → success, 5.2), false reads
* EXPR — the unchanged one-shot xce_* call, evaluated each iteration; its
* rc is what the loop classifies and ultimately returns
*
* Evaluates to the final rc (>=0 success; -E_XCE_NOT_READY on no-primary budget
* exhaustion; terminal errors surfaced as-is). Used as the RHS of `return`.
*
* Implemented as a GCC/clang statement-expression so it yields a value and keeps
* each public xcecli_* signature/return convention intact (R1.3). The lock is
* released on every exit path because the do/while always falls through to the
* unlock before the result is produced.
*/
#define XCECLI_FAILOVER_CALL(ac, is_write, EXPR) \
__extension__({ \
api_ctx_t* _fo_ac = (ac); \
xcecli_retry_t _fo_st = XCECLI_RETRY_INIT(_fo_ac); \
int _fo_rc; \
mylock_lock(&_fo_ac->failover.lock); \
do { \
_fo_rc = (EXPR); \
} while ( \
xcecli_retry_next(_fo_ac, &_fo_rc, (is_write), &_fo_st)); \
mylock_unlock(&_fo_ac->failover.lock); \
_fo_rc; \
})
/*
* XCECLI_FAILOVER_CALL_COUNT(ac, EXPR) — sibling of XCECLI_FAILOVER_CALL for
* xce_get-family calls (bug-1436).
*
* xce_get_with_condition (request.c) — and every xcecli_* wrapper built on it
* (joblist, reslist, workerlist, resval, jobinfo) — returns a NON-NEGATIVE ROW
* COUNT on success (`rc = getreply.nrows`), not a fixed 0/status code; a
* negative return is the ONLY failure signal. XCECLI_FAILOVER_CALL assumes
* every wrapped EXPR follows the "0 = success" status-code convention and
* unconditionally feeds the raw return value through xce_failover_classify(),
* which recognizes retriable/terminal outcomes by comparing MAGNITUDE against
* fixed constants (E_XCE_* >= 10001, and POSIX errno values it deliberately
* accepts in either sign — ECONNRESET=104, ETIMEDOUT=110, ECONNREFUSED=111,
* client_failover.c fo_abs()). A joblist/reslist/workerlist/resval reply whose
* row count happens to equal one of those constants (e.g. exactly 111 jobs ==
* ECONNREFUSED) is silently misclassified as a transport failure on EVERY
* call with that exact count — forcing a spurious xce_failover_rotate() to the
* NEXT configured master even though the read fully succeeded. Since
* get-class ops are primary-only, landing on a standby correctly (but
* pointlessly) fails with E_XCE_STANDBY and rotates again; the cycle repeats
* for every subsequent same-sized call in the same batch, burning the finite
* max_retries budget (20) on self-inflicted churn until it coincides with a
* genuine transient (e.g. a peer mid-restart after a just-completed election)
* and exhausts the budget while rotating, surfacing a real
* ECONNRESET/ECONNREFUSED to the caller — bug-1436 ("xce_joblist: Connection
* reset by peer" immediately after a successful wait-primary; reproduced live
* at exactly 111 tracked jobs, the first count in the /xce-test-ha soak's
* arithmetic progression — 3 + 3*round — that lands on 111 == ECONNREFUSED).
*
* Fix: only enter the retry/classify loop when EXPR's raw result is NEGATIVE.
* A non-negative result (0 or a genuine row count, of ANY magnitude) is
* unconditionally success and returned as-is — it can never be misread as a
* status code. A dead ctx (XCE_CTX_ERR) is still caught correctly: the per-op
* XCE_CTX_RUN guard in xceapi.c makes EXPR itself return a negative -EINVAL in
* that state, which enters the loop and is reclassified by
* xcecli_retry_decide's dead-connection branch (checked before magnitude
* classification, regardless of the stale *rc it was invoked with).
*
* is_write is hardcoded false: every xce_get-family op is a read.
*/
#define XCECLI_FAILOVER_CALL_COUNT(ac, EXPR) \
__extension__({ \
api_ctx_t* _fc_ac = (ac); \
xcecli_retry_t _fc_st = XCECLI_RETRY_INIT(_fc_ac); \
int _fc_rc; \
mylock_lock(&_fc_ac->failover.lock); \
do { \
_fc_rc = (EXPR); \
} while ((_fc_rc < 0) && \
xcecli_retry_next(_fc_ac, &_fc_rc, /*is_write=*/false, \
&_fc_st)); \
mylock_unlock(&_fc_ac->failover.lock); \
_fc_rc; \
})
/*
* xcecli_ctx_recover (task 3.2) — repair the inner xce_ctx_t after a successful
* reconnect so the next xce_* attempt can proceed. Socket-free (no I/O, no
* sleep) so it is unit-testable; xcecli_retry_next invokes it after a successful
* xce_failover_rotate(), before re-sending the interrupted op.
*
* CI2 (state reset): the transport-error path left ctx->state = XCE_CTX_ERR
* (xcectx.c:174), which the per-op XCE_CTX_RUN guard rejects as -EINVAL
* (xceapi.c) — the reported bug. recover restores XCE_CTX_RUN, but ONLY from
* ERR: a DOWN/INIT ctx (deliberately stopped / not yet started) is left
* as-is, and a RUN ctx is already fine.
* CI1 (stale-reply drain): handle_disconnect may have injected a stale status
* into the single-slot ctx->wbuf (xcectx.c:180). recover drains any pending
* entry with a non-blocking cbuf_tryget so the next attempt's cbuf_get
* cannot consume a prior-attempt reply. Reconnect closes the old fd first,
* so the dead connection yields no further replies — this drain is enough.
*
* Returns 0 on success, -EINVAL on a NULL ac.
*/
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-
* free (it only calls the stored callback), so it is unit-testable. The
* xcecli_retry_next wrapper calls it AFTER xce_failover_rotate +
* xcecli_ctx_recover and BEFORE re-sending the op.
*
* If no hook is registered (rejoin_cb == NULL, a pure client) it is a no-op and
* returns 0 so the loop resumes directly. Otherwise it returns the callback's
* rc: 0 to proceed, <0 to treat the rejoin as FAILED (the new primary rejected
* the daemon's join) — the wrapper then does NOT resume but rotates/backs off on
* the next iteration.
*
* Returns 0 on a successful (or absent) rejoin, the hook's <0 rc on a failed
* rejoin, or -EINVAL on a NULL ac.
*/
int xcecli_invoke_rejoin(api_ctx_t* ac);
#endif