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

feat(raft-drive-async): offload requester-less config-push to slow lane (task 3.3)

Browse source 
push_config_transport_only spawns one thread per standby then joins them with a
CONF_PUSH_TIMEOUT_MS bounded wait — blocking the caller. On the requester-less
drive-thread paths (promotion + heartbeat convergence) that wait stalls the
io_worker. Route those through ha_config_push_offloadable, gated by
g_drive_async_configpush_offload (default OFF → inline, byte-for-byte legacy;
ON → slow lane, non-blocking).

Flag ON deep-copies the config bytes + sha into a lane ctx (caller keeps owning
and freeing its own buffer); a single cluster-wide single-flight guard
(ha_state_t.configpush_inflight scalar CAS + lane key=1) bounds it to one
config-push fan-out in flight; the in-flight flag is cleared on every terminal
path (two alloc failures, -EEXIST, hard submit error, lane completion). Send
body factored into ha_config_push_do_send.

Three requester-less drive-thread call sites converted: raft_cb_become_leader
(promotion), the HA_REPL_PUSH_CONFIG heartbeat case, and the direct-CTRL_PRIMARY
FSM promotion branch. The admin-upgrade barrier (push_config_to_standbys, has a
waiting requester) and join.c are untouched.

Lands DORMANT like tasks 3.1/3.2: the lane reads c->ha.standby_endpoints and
writes those borrowed standby endpoints, racing io_worker RDHUP teardown if run
off-thread (bug-1197 family). Flag OFF keeps push+teardown serialized on the
io_worker. Task 3.4 flips this ON together with the snapshot and peerctrl flags
after establishing endpoint-lifetime discipline.

Tests (controller suite EXIT=0): test_configpush_offload_runs_on_lane,
test_configpush_inline_when_flag_off, test_configpush_offload_dedups.
Adversarial cpp-review: no CRITICAL/HIGH/MEDIUM findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S6fDCtXkkEgupu7rUah1y2
This commit is contained in:
Rongsong Shen 2026-06-26 16:05:54 +00:00
parent 6c0523b69b
commit bd02064ebc
5 changed files with 515 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

267
src/controller/ha.c
View file

@ -87,6 +87,13 @@ extern int push_config_transport_only(cluster_t *c, const uint8_t *bytes,
uint64_t new_tag, int *n_out);
#endif
/* raft-drive-async Task 3.3: requester-less config-push offload router (defined
* below, near the snapshot helpers). Forward-declared because
* raft_cb_become_leader (earlier) is one of its two call sites. */
static void ha_config_push_offloadable(cluster_t *c, const uint8_t *bytes,
size_t blen, const uint8_t sha256[32],
uint64_t new_tag);
/* config_repl.c is linked in BOTH production and XCE_TESTING builds.
* ha-term-safety task 4.3 (remediation r1): the commit-advance hook activates a
* newly-committed config marker (Finding B); ha_dispatch_gate / the pre-promotion
@ -1190,14 +1197,16 @@ static void raft_cb_become_leader(void *ctx, uint64_t term)
extern int conf_read_raw(const char *, uint8_t **, size_t *);
if (c->myconf && c->myconf->conffile &&
conf_read_raw(c->myconf->conffile, &cfg_bytes, &cfg_len) == 0) {
push_config_transport_only(c, cfg_bytes, cfg_len,
/* Task 3.3: inline (flag OFF, default) or slow-lane (flag ON).
* The offload deep-copies cfg_bytes, so this free is still
* correct regardless of routing. */
ha_config_push_offloadable(c, cfg_bytes, cfg_len,
c->myconf->config_sha256,
c->myconf->config_version_tag, NULL);
c->myconf->config_version_tag);
free(cfg_bytes);
} else {
push_config_transport_only(c, NULL, 0, NULL,
c->myconf ? c->myconf->config_version_tag : 0,
NULL);
ha_config_push_offloadable(c, NULL, 0, NULL,
c->myconf ? c->myconf->config_version_tag : 0);
}
}
#endif
@ -2783,6 +2792,190 @@ static void ha_snapshot_send_dispatch(ha_state_t *ha, cluster_t *c,
/* 0 or -EAGAIN: enqueued exactly once; snapshot_lane_fn clears the flag. */
}
/* ───────────────────────────────────────────────────────────────────────
* raft-drive-async Task 3.3 — requester-less ConfigPush slow-lane offload
* (bug-1198).
*
* The two REQUESTER-LESS config-push sites — post-election promotion
* (raft_cb_become_leader) and the heartbeat-driven HA_REPL_PUSH_CONFIG
* convergence case — call push_config_transport_only, which spawns one push
* thread per standby and then BLOCKS the caller in wait_all_with_timeout up to
* CONF_PUSH_TIMEOUT_MS. Run inline on the single io_worker that drives both of
* those sites, that wait stalls every heartbeat/vote for the whole timeout.
* This offloads it to the bounded slow lane. Config push is a CLUSTER-WIDE
* fan-out, so a single cluster-wide single-flight guard (configpush_inflight) +
* a single dedup key (key=1) suffice — at most one push in flight at a time.
*
* The admin-upgrade path (push_config_to_standbys, which has a waiting
* requester / majority barrier) is NOT routed here and stays unchanged.
* ─────────────────────────────────────────────────────────────────────── */
#ifdef XCE_TESTING
extern void (*ha_configpush_probe)(void);
#endif
typedef struct {
cluster_t *c;
uint8_t *bytes; /* deep copy, owned by the ctx; NULL when blen==0 */
size_t blen;
uint8_t sha[32]; /* valid only when has_sha != 0 */
int has_sha;
uint64_t new_tag;
} configpush_ctx_t;
/* ha_config_push_do_send: the actual (blocking) config-push work — shared by
* the inline path and the lane worker so both behave identically. In
* XCE_TESTING builds a non-NULL ha_configpush_probe() is invoked right around
* the send so a test can record the running thread / park the lane worker. */
static void ha_config_push_do_send(cluster_t *c, const uint8_t *bytes,
size_t blen, const uint8_t sha256[32],
uint64_t new_tag)
{
#ifdef XCE_TESTING
/* XCE_TESTING does not link cluster.c and uses hook shims for the real
* transport (push_config_transport_only's extern is #ifndef XCE_TESTING).
* The probe stands in for the send and records the running thread. */
if (ha_configpush_probe != NULL)
ha_configpush_probe();
(void)c; (void)bytes; (void)blen; (void)sha256; (void)new_tag;
#else
push_config_transport_only(c, bytes, blen, sha256, new_tag, NULL);
#endif
}
/*
* configpush_lane_fn: slow-lane worker entry. Runs the bounded blocking
* config push off the drive thread, then ALWAYS clears the cluster-wide
* in-flight flag (success or failure) so the dedup never wedges and a later
* promotion/heartbeat can retry. Frees the deep-copied bytes and the ctx.
*
* ⚠ BORROWED-ENDPOINT LIFETIME HAZARD (bug-1197 family — why this path is
* gated OFF by default). push_config_transport_only here reads
* c->ha.standby_endpoints and writes to those standby endpoints, which are
* shared with the io_worker epoll loop, NOT owned by this lane (contrast
* reconnect_lane_fn, which owns the ep it creates). If a standby
* RDHUP-disconnects while this push runs, the io_worker can endpoint_fini/free
* those endpoints concurrently → UAF. Today (flag OFF) the push runs inline ON
* the io_worker, so teardown and push are serialized on one thread and cannot
* overlap. Flipping g_drive_async_configpush_offload ON is therefore only safe
* once Task 3.4 establishes the endpoint-lifetime discipline for the whole
* pool/lane dispatch model. Until then this code lands DORMANT and is
* exercised only by unit tests with a stable (or absent) endpoint set.
*/
static void configpush_lane_fn(void *arg)
{
configpush_ctx_t *cx = (configpush_ctx_t *)arg;
if (!cx) return;
cluster_t *c = cx->c;
ha_config_push_do_send(c, cx->bytes, cx->blen,
cx->has_sha ? cx->sha : NULL, cx->new_tag);
/* Clear in-flight LAST so a concurrent promotion/heartbeat tick cannot
* enqueue a second config push until this one has fully finished. */
atomic_store_explicit(&c->ha.configpush_inflight, 0, memory_order_release);
free(cx->bytes);
free(cx);
}
/*
* ha_config_push_offloadable: route a requester-less config push either inline
* (flag OFF, production default — runs on the caller/drive thread, unchanged
* legacy behavior; the CALLER still owns `bytes` and frees them after we
* return) or onto the slow lane (flag ON — non-blocking; we take a deep copy of
* `bytes` so the caller's free is safe). The cluster-wide single-flight CAS on
* ha.configpush_inflight is the authoritative dedup; the lane's per-key dedup
* is a redundant backstop. Mirrors the snapshot-offload terminal-path handling
* (Task 3.2). Never self-terminates: on any failure it clears the guard and
* the next cycle retries.
*/
static void ha_config_push_offloadable(cluster_t *c, const uint8_t *bytes,
size_t blen, const uint8_t sha256[32],
uint64_t new_tag)
{
/* Flag OFF (default) or no cluster handle (no lane) → inline. */
if (!ha_configpush_offload_enabled() || c == NULL) {
ha_config_push_do_send(c, bytes, blen, sha256, new_tag);
return;
}
/* Cluster-wide single-flight: CAS 0->1. Already 1 → a config push is in
* flight; enqueue nothing. */
int expected = 0;
if (!atomic_compare_exchange_strong_explicit(
&c->ha.configpush_inflight, &expected, 1,
memory_order_acq_rel, memory_order_acquire)) {
return;
}
configpush_ctx_t *cx = malloc(sizeof(*cx));
if (!cx) {
atomic_store_explicit(&c->ha.configpush_inflight, 0,
memory_order_release);
return;
}
cx->c = c;
cx->new_tag = new_tag;
cx->has_sha = (sha256 != NULL) ? 1 : 0;
if (cx->has_sha)
memcpy(cx->sha, sha256, 32);
if (bytes != NULL && blen > 0) {
cx->bytes = malloc(blen);
if (!cx->bytes) {
free(cx);
atomic_store_explicit(&c->ha.configpush_inflight, 0,
memory_order_release);
return;
}
memcpy(cx->bytes, bytes, blen);
cx->blen = blen;
} else {
cx->bytes = NULL;
cx->blen = 0;
}
uint64_t key = 1; /* single cluster-wide dedup key (one config-push stream) */
int srv = slowlane_submit(&c->slowlane, key, configpush_lane_fn, cx);
if (srv == -EEXIST) {
/* Lane still holds the key from a prior config-push task that has
* already cleared configpush_inflight (its last action) but whose key
* the lane has not yet removed. Our CAS saw 0 and set 1, but the
* submit is refused — release the flag we set so no clear is lost, and
* let the next tick retry. */
free(cx->bytes);
free(cx);
atomic_store_explicit(&c->ha.configpush_inflight, 0,
memory_order_release);
return;
}
if (srv < 0 && srv != -EAGAIN) {
/* Hard submit failure (alloc / lane stopping). -EAGAIN is NOT a
* failure (still queued, will run). Only on a real failure tear down
* and release the guard so the next tick retries; never self-terminate. */
WARN("HA repl: slowlane_submit configpush failed: %d — retry next cycle",
srv);
free(cx->bytes);
free(cx);
atomic_store_explicit(&c->ha.configpush_inflight, 0,
memory_order_release);
return;
}
/* 0 or -EAGAIN: enqueued exactly once; configpush_lane_fn clears the flag. */
}
#ifdef XCE_TESTING
/* Test seam: directly drive ha_config_push_offloadable (the two production call
* sites are #ifndef XCE_TESTING). Returns 0 (the dispatch itself cannot fail
* fatally — failures clear the guard and retry). */
int ha_config_push_offloadable_for_test(cluster_t *c, const uint8_t *bytes,
size_t blen, const uint8_t sha256[32],
uint64_t new_tag)
{
ha_config_push_offloadable(c, bytes, blen, sha256, new_tag);
return 0;
}
#endif
/*
* ha_dispatch_repl_action_for_standby — execute the decision-loop action for
* one standby on the current heartbeat cycle (Task 3.3).
@ -2836,14 +3029,16 @@ int ha_dispatch_repl_action_for_standby(ha_state_t *ha,
size_t cfg_len = 0;
if (c->myconf && c->myconf->conffile &&
conf_read_raw(c->myconf->conffile, &cfg_bytes, &cfg_len) == 0) {
push_config_transport_only(c, cfg_bytes, cfg_len,
/* Task 3.3: inline (flag OFF, default) or slow-lane (flag ON).
* The offload deep-copies cfg_bytes, so this free is still
* correct regardless of routing. */
ha_config_push_offloadable(c, cfg_bytes, cfg_len,
c->myconf->config_sha256,
c->myconf->config_version_tag, NULL);
c->myconf->config_version_tag);
free(cfg_bytes);
} else {
push_config_transport_only(c, NULL, 0, NULL,
c->myconf ? c->myconf->config_version_tag : 0,
NULL);
ha_config_push_offloadable(c, NULL, 0, NULL,
c->myconf ? c->myconf->config_version_tag : 0);
}
#endif
}
@ -3960,6 +4155,42 @@ int ha_snapshot_offload_enabled(void)
memory_order_acquire);
}
/* raft-drive-async task 3.3: ConfigPush feature flag.
*
* DEFAULT OFF (0): the requester-less config push runs INLINE on the caller
* (io_worker/drive) thread — byte-for-byte the legacy behavior, i.e.
* push_config_transport_only's fan-out followed by its blocking
* wait_all_with_timeout(CONF_PUSH_TIMEOUT_MS). This task lands DORMANT,
* exactly like the Task 3.1/3.2 flags, because the offload borrows standby
* endpoints it does not own (bug-1197 family — push_config_transport_only
* reads c->ha.standby_endpoints): moving the push to a lane thread while the
* io_worker may RDHUP-free those endpoints is a cross-thread UAF. Today (flag
* OFF) the push and the endpoint teardown are serialized on the single
* io_worker, so they cannot overlap.
*
* Flipping ON is the job of Task 3.4, which establishes the endpoint-lifetime
* discipline for the whole pool/lane dispatch model and flips this together
* with the snapshot + peer-control dispatch flags. Until then this offload is
* exercised only by unit tests with a stable (or absent) endpoint set.
*
* ONLY the requester-less call sites (post-election promotion, heartbeat-driven
* HA_REPL_PUSH_CONFIG) route through this flag; the admin-upgrade barrier path
* (push_config_to_standbys) is unaffected. Compiled in ALL builds; the setter
* is the test seam to flip RED<->GREEN. */
static _Atomic int g_drive_async_configpush_offload = 0;
void ha_set_configpush_offload_for_test(int on)
{
atomic_store_explicit(&g_drive_async_configpush_offload, on ? 1 : 0,
memory_order_release);
}
int ha_configpush_offload_enabled(void)
{
return atomic_load_explicit(&g_drive_async_configpush_offload,
memory_order_acquire);
}
/* raft-drive-async task 3.1: PeerCtrlDispatch feature flag.
*
* DEFAULT OFF (0): the registered peer-control handler runs its routing body
@ -4085,6 +4316,16 @@ int (*ha_repl_stream_wal_hook)(void *ep, const char *wal_path,
*/
int (*ha_repl_send_snapshot_hook)(int peer_idx) = NULL;
/*
* ha_configpush_probe: test-only observation seam for the requester-less
* config-push offload (raft-drive-async Task 3.3). When non-NULL,
* ha_config_push_do_send() invokes it right around push_config_transport_only
* (on BOTH the inline and the lane path), so a test can record the running
* thread and optionally park the lane worker in flight to prove the
* cluster-wide single-flight dedup. NULL in production (zero cost).
*/
void (*ha_configpush_probe)(void) = NULL;
#endif /* XCE_TESTING */
/* ── Election Evaluation (Req 2.1, 2.7, 2.8) ───────────────────────────────
@ -4226,13 +4467,15 @@ apply_role:
", first_boot=%d)", atomic_load_explicit(&ha->term, memory_order_acquire),
is_first_boot);
/* Push config to standbys with lower config_tag (Req 7.1) */
/* Push config to standbys with lower config_tag (Req 7.1).
* Task 3.3: requester-less promotion push → offloadable (dormant
* flag OFF = inline, unchanged; flag ON = slow lane). */
#ifdef XCE_TESTING
if (ha_push_config_hook != NULL)
ha_push_config_hook(c);
#else
if (c != NULL)
push_config_transport_only(c, NULL, 0, NULL, 0, NULL);
ha_config_push_offloadable(c, NULL, 0, NULL, 0);
#endif
/* Send CtrlHelloRequest to all standbys to trigger WAL catch-up (Req 7.2) */