wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

fix(client): bug-1436 count-aware failover macro for xce_get-family calls

Browse source 
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.
This commit is contained in:
Rongsong Shen 2026-07-13 07:18:52 +00:00
parent 510b566bbf
commit 40c2351578
2 changed files with 79 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

34
src/lib/client.c
View file

@ -283,8 +283,9 @@ int xcecli_reslist(xcecli_ctx_t c, xce_resource_t*** res)
return -EINVAL;
}
return XCECLI_FAILOVER_CALL(ctx, /*is_write=*/false,
xce_reslist(&(ctx->ctx), res));
/* xce_reslist returns a non-negative ROW COUNT on success, not a 0/status
* code — must go through the count-aware macro (bug-1436). */
return XCECLI_FAILOVER_CALL_COUNT(ctx, xce_reslist(&(ctx->ctx), res));
}
int xcecli_join(xcecli_ctx_t c, int role, hostattrs_t* attrs)
@ -305,8 +306,9 @@ int xcecli_workerlist(xcecli_ctx_t c, xce_worker_t*** workers)
return -EINVAL;
}
return XCECLI_FAILOVER_CALL(ctx, /*is_write=*/false,
xce_workerlist(&(ctx->ctx), workers));
/* xce_workerlist returns a non-negative ROW COUNT on success, not a
* 0/status code — must go through the count-aware macro (bug-1436). */
return XCECLI_FAILOVER_CALL_COUNT(ctx, xce_workerlist(&(ctx->ctx), workers));
}
int xcecli_jobdispatch(xcecli_ctx_t c, xce_job_dispatch_t* info)
@ -341,8 +343,14 @@ int xcecli_joblist(xcecli_ctx_t c, xce_job_t*** jobs)
return -EINVAL;
}
return XCECLI_FAILOVER_CALL(ctx, /*is_write=*/false,
xce_joblist(&(ctx->ctx), jobs));
/* xce_joblist returns a non-negative ROW COUNT on success, not a 0/status
* code — a count that happens to equal a retriable errno's magnitude
* (e.g. exactly 111 jobs == ECONNREFUSED) would be misclassified as a
* transport failure by the generic macro, forcing a spurious reconnect
* storm that eventually surfaces a real "Connection reset by peer" once
* it coincides with a genuine transient (bug-1436). Must go through the
* count-aware macro instead. */
return XCECLI_FAILOVER_CALL_COUNT(ctx, xce_joblist(&(ctx->ctx), jobs));
}
int xcecli_clusterconf(xcecli_ctx_t c, xce_clusterconf_t** conf)
@ -583,8 +591,9 @@ int xcecli_resval(xcecli_ctx_t c, char* host, char* res, xce_resval_t*** val)
return -EINVAL;
}
return XCECLI_FAILOVER_CALL(ctx, /*is_write=*/false,
xce_resval(&(ctx->ctx), host, res, val));
/* xce_resval returns a non-negative ROW COUNT on success, not a 0/status
* code — must go through the count-aware macro (bug-1436). */
return XCECLI_FAILOVER_CALL_COUNT(ctx, xce_resval(&(ctx->ctx), host, res, val));
}
int xcecli_notify(xcecli_ctx_t c, xce_notify_data_t* data)
@ -913,7 +922,12 @@ int xcecli_jobinfo(xcecli_ctx_t c, uint64_t jobid, xce_job_t** job)
return -EINVAL;
}
rc = XCECLI_FAILOVER_CALL(ctx, /*is_write=*/false,
xce_jobinfo(&(ctx->ctx), jobid, job));
/* xce_jobinfo returns 1 (found) / 0 (not found) / negative (error) — a
* fixed non-zero success value, not a 0/status code. 1 never happens to
* collide with a recognized retriable magnitude today, but it is the same
* class of bug as bug-1436 (xcecli_joblist) — go through the count-aware
* macro for correctness and consistency rather than relying on that
* coincidence. */
rc = XCECLI_FAILOVER_CALL_COUNT(ctx, xce_jobinfo(&(ctx->ctx), jobid, job));
return rc;
}

55
src/lib/client_internal.h
View file

@ -171,6 +171,61 @@ bool xcecli_retry_next(api_ctx_t* ac, int* rc, bool is_write,
_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