lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat(raft-drive-async): snapshot SlowLane offload + WAL-trim cursor guard (task 3.2)

Browse source 
Part B (LIVE, default-ON): trim_wal_segments now bounds WAL-segment deletion
by repl_trim_lsn(&p->repl, active_seg_lsn) = min(active segment LSN, every
active standby acked_lsn, every valid disconnected-slot last_disconnected_lsn),
after repl_expire_disconnect_slots ages out stale slot guards. floor <=
active_seg_lsn structurally, so the guard can only preserve more segments,
never delete one a live standby still needs — closes the bug-795/828
trim-vs-cursor race for connected/recently-disconnected standbys (bug-1147
stopgap; empty/cursor=0 join still needs entity-bearing snapshots).

Part A (DORMANT, g_drive_async_snapshot_offload default OFF): ha_snapshot_send_
dispatch routes HA_REPL_SEND_SNAPSHOT inline (flag OFF — byte-for-byte legacy,
on io_worker) or to slowlane_submit+snapshot_lane_fn (flag ON). Send body
factored into ha_snapshot_do_send. Per-peer single-flight via
ha_state_t.snapshot_inflight[] CAS + slowlane key; in-flight flag cleared on
all four terminal paths, mirroring the reconnect offload (task 2.1).

Lands dormant like task 3.1: snapshot_lane_fn borrows a standby endpoint it
does not own, so flag-ON would race io_worker RDHUP teardown (cross-thread UAF,
bug-1197 family). Flag OFF keeps send+teardown serialized on the io_worker.
Task 3.4 is the single gate that flips both this and the peerctrl flag ON after
establishing endpoint-lifetime discipline; bug-1198 stays open by design until
then. The offload flag was initially authored default-ON (would ship the UAF)
and corrected to OFF in ha.c and the ha.h accessor doc (bug-1210).

Tests (controller 196/0, 0 new leaks): test_snapshot_offload_runs_on_lane,
test_snapshot_inline_when_flag_off, test_snapshot_offload_dedups_per_peer,
test_trim_guard_preserves_standby_segments,
test_trim_without_standbys_proceeds_normally. Adversarial cpp-review: GO.

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 15:43:24 +00:00
parent e17a6a613c
commit 6c0523b69b
8 changed files with 2046 additions and 850 deletions
Download patch file Download diff file Expand all files Collapse all files

221
src/controller/ha.c
View file

@ -2644,6 +2644,145 @@ void ha_process_standby_state(ha_state_t *ha, int peer_idx,
}
}
/* ───────────────────────────────────────────────────────────────────────
* raft-drive-async Task 3.2 — SnapshotSend slow-lane offload (bug-1198).
*
* The snapshot transfer (a fresh persist_checkpoint → fdatasync, then a
* chunked socket write of the snapshot file) is the one unbounded operation
* reachable from the repl-ack decision loop. Run inline it blocks the
* io_worker/drive thread for the whole transfer (and queues that peer's later
* messages behind it). This offloads it to the bounded slow lane, keyed per
* standby so at most one snapshot per peer is in flight (Req 1.5).
* ─────────────────────────────────────────────────────────────────────── */
typedef struct {
cluster_t *c;
endpoint_t *ep;
int peer_idx;
} snapshot_send_ctx_t;
/* ha_snapshot_do_send: the actual (blocking) snapshot work — shared by the
* inline path and the lane worker so both behave identically. In XCE_TESTING
* builds the send is replaced by ha_repl_send_snapshot_hook(peer_idx). */
static void ha_snapshot_do_send(cluster_t *c, endpoint_t *ep, int peer_idx)
{
#ifdef XCE_TESTING
if (ha_repl_send_snapshot_hook != NULL) {
ha_repl_send_snapshot_hook(peer_idx);
return;
}
#endif
if (c && c->persist.snap_path[0]) {
extern int ha_send_snapshot_to_standby(endpoint_t *, const char *,
uint64_t);
/* Req 4.1: create a FRESH checkpoint so the transferred snapshot
* reflects current state (also trims WAL + rotates old snaps).
* -EBUSY means another checkpoint is in flight — fall back to the
* existing checkpoint_lsn rather than skipping the cycle (at most one
* checkpoint in flight is the existing persist_checkpoint invariant). */
int ckpt_rc = persist_checkpoint(&c->persist);
if (ckpt_rc != 0 && ckpt_rc != -EBUSY)
WARN("HA repl: persist_checkpoint before snapshot send "
"failed rc=%d — sending existing checkpoint", ckpt_rc);
ha_send_snapshot_to_standby(ep, c->persist.snap_path,
c->persist.checkpoint_lsn);
}
}
/*
* snapshot_lane_fn: slow-lane worker entry. Runs the bounded blocking
* snapshot send off the drive thread, then ALWAYS clears the per-peer
* in-flight flag (success or failure) so the dedup never wedges and a later
* heartbeat can retry. Frees the ctx.
*
* ⚠ BORROWED-ENDPOINT LIFETIME HAZARD (bug-1197 family — why this path is
* gated OFF by default). `ep` here is a standby endpoint shared with the
* io_worker epoll loop, NOT one this lane owns (contrast reconnect_lane_fn,
* which owns the ep it creates). If the standby RDHUP-disconnects while this
* transfer runs, the io_worker can endpoint_fini/free `ep` concurrently → UAF.
* Today (flag OFF) the send runs inline ON the io_worker, so teardown and send
* are serialized on one thread and cannot overlap. Flipping
* g_drive_async_snapshot_offload ON is therefore only safe once Task 3.4
* establishes the endpoint-lifetime discipline for the whole pool/lane
* dispatch model (refcount or lane-serialized teardown). Until then this code
* lands DORMANT and is exercised only by unit tests with a stable endpoint. */
static void snapshot_lane_fn(void *arg)
{
snapshot_send_ctx_t *sx = (snapshot_send_ctx_t *)arg;
if (!sx) return;
cluster_t *c = sx->c;
int peer_idx = sx->peer_idx;
ha_snapshot_do_send(c, sx->ep, peer_idx);
/* Clear in-flight LAST so a concurrent heartbeat tick cannot enqueue a
* second snapshot for this peer until this one has fully finished. */
atomic_store_explicit(&c->ha.snapshot_inflight[peer_idx], 0,
memory_order_release);
free(sx);
}
/*
* ha_snapshot_send_dispatch: route a SEND_SNAPSHOT decision either inline
* (flag OFF, production default — runs on the caller/drive thread, unchanged
* legacy behavior) or onto the slow lane (flag ON — non-blocking). The
* per-peer single-flight CAS on ha->snapshot_inflight[] is the authoritative
* dedup; the lane's per-key dedup is a redundant backstop. Mirrors the
* reconnect-offload terminal-path handling (Task 2.1).
*/
static void ha_snapshot_send_dispatch(ha_state_t *ha, cluster_t *c,
endpoint_t *ep, int peer_idx)
{
/* Flag OFF (default) or no cluster handle (no lane) → inline. */
if (!ha_snapshot_offload_enabled() || c == NULL) {
ha_snapshot_do_send(c, ep, peer_idx);
return;
}
/* Per-peer single-flight: CAS 0->1. Already 1 → a snapshot to this peer
* is in flight; enqueue nothing (Req 1.5). */
int expected = 0;
if (!atomic_compare_exchange_strong_explicit(
&ha->snapshot_inflight[peer_idx], &expected, 1,
memory_order_acq_rel, memory_order_acquire)) {
return;
}
snapshot_send_ctx_t *sx = malloc(sizeof(*sx));
if (!sx) {
atomic_store_explicit(&ha->snapshot_inflight[peer_idx], 0,
memory_order_release);
return;
}
sx->c = c; sx->ep = ep; sx->peer_idx = peer_idx;
uint64_t key = (uint64_t)(peer_idx + 1); /* +1: key 0 disables dedup */
int srv = slowlane_submit(&c->slowlane, key, snapshot_lane_fn, sx);
if (srv == -EEXIST) {
/* Lane still holds this peer's key from a prior snapshot task that has
* already cleared snapshot_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(sx);
atomic_store_explicit(&ha->snapshot_inflight[peer_idx], 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 snapshot peer[%d] failed: %d — retry "
"next cycle", peer_idx, srv);
free(sx);
atomic_store_explicit(&ha->snapshot_inflight[peer_idx], 0,
memory_order_release);
return;
}
/* 0 or -EAGAIN: enqueued exactly once; snapshot_lane_fn clears the flag. */
}
/*
* ha_dispatch_repl_action_for_standby — execute the decision-loop action for
* one standby on the current heartbeat cycle (Task 3.3).
@ -2714,26 +2853,11 @@ int ha_dispatch_repl_action_for_standby(ha_state_t *ha,
INFO("HA repl: standby[%d] lsn=%" PRIu64
" < oldest_wal_lsn=%" PRIu64 " → requesting snapshot transfer",
peer_idx, standby_lsn, oldest_wal_lsn);
#ifdef XCE_TESTING
if (ha_repl_send_snapshot_hook != NULL) {
ha_repl_send_snapshot_hook(peer_idx);
break;
}
#endif
if (c && c->persist.snap_path[0]) {
extern int ha_send_snapshot_to_standby(endpoint_t *, const char *,
uint64_t);
/* Req 4.1: create a FRESH checkpoint so the transferred snapshot
* reflects current state (also trims WAL + rotates old snaps).
* -EBUSY means another checkpoint is in flight — fall back to
* the existing checkpoint_lsn rather than skipping the cycle. */
int ckpt_rc = persist_checkpoint(&c->persist);
if (ckpt_rc != 0 && ckpt_rc != -EBUSY)
WARN("HA repl: persist_checkpoint before snapshot send "
"failed rc=%d — sending existing checkpoint", ckpt_rc);
ha_send_snapshot_to_standby(ep, c->persist.snap_path,
c->persist.checkpoint_lsn);
}
/* Task 3.2: route inline (flag OFF, default) or onto the slow lane
* (flag ON). The send body — fresh checkpoint + chunked transfer, or
* the XCE_TESTING hook — lives in ha_snapshot_do_send so both paths
* are identical. */
ha_snapshot_send_dispatch(ha, c, ep, peer_idx);
break;
case HA_REPL_STREAM_WAL:
@ -3802,6 +3926,40 @@ int ha_reconnect_offload_enabled(void)
memory_order_acquire);
}
/* raft-drive-async task 3.2: SnapshotSend feature flag.
*
* DEFAULT OFF (0): the snapshot transfer runs INLINE on the caller (io_worker/
* drive) thread — byte-for-byte the legacy behavior. This task lands DORMANT,
* exactly like the Task 3.1 peer-control dispatch flag, because the offload
* captures a BORROWED standby endpoint (see snapshot_lane_fn's bug-1197 hazard
* note): moving the transfer to a lane thread while the io_worker may
* RDHUP-free that endpoint is a cross-thread UAF. Today (flag OFF) the send
* and the endpoint teardown are serialized on the single io_worker, so they
* cannot overlap.
*
* Flipping ON is the job of Task 3.4 (receiver-side integration), which
* establishes the endpoint-lifetime discipline for the pool/lane dispatch
* model and proves the transfer no longer stalls other peers' heartbeats/votes
* with the pool near saturation. Until then, bug-1198 (the repl-ack body is
* still unbounded when run inline) remains open by design.
*
* Keyed per standby (Req 1.5) so at most one snapshot per peer is in flight;
* at most one persist_checkpoint in flight (existing atomic guard). Compiled
* in ALL builds; the setter is the test seam to flip RED↔GREEN. */
static _Atomic int g_drive_async_snapshot_offload = 0;
void ha_set_snapshot_offload_for_test(int on)
{
atomic_store_explicit(&g_drive_async_snapshot_offload, on ? 1 : 0,
memory_order_release);
}
int ha_snapshot_offload_enabled(void)
{
return atomic_load_explicit(&g_drive_async_snapshot_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
@ -4937,17 +5095,24 @@ static int peer_ctrl_vtask_entry(vtask_t *task)
* everything this handler dispatches is liveness work today.)
*
* ┌─ HIGH FLAG-ON HAZARD (raft-drive-async finding-2; bug-1198) ─────────────┐
* │ LIVENESS is correct ONLY once task 3.2 offloads the snapshot send off the │
* │ pool. TODAY the CTRL_PRIMARY repl-ack path can reach │
* │ HA_REPL_SEND_SNAPSHOT → persist_checkpoint (a BLOCKING fdatasync) INLINE │
* │ in repl_ack_decode_and_dispatch → ha_dispatch_repl_action_for_standby. │
* │ LIVENESS is correct ONLY once the snapshot send runs OFF the pool. The │
* │ CTRL_PRIMARY repl-ack path can reach HA_REPL_SEND_SNAPSHOT → │
* │ persist_checkpoint (a BLOCKING fdatasync) in │
* │ repl_ack_decode_and_dispatch → ha_dispatch_repl_action_for_standby. │
* │ Running that blocking op while tagged VTASK_LIVENESS (never gated) breaks │
* │ the deadlock-freedom reservation — a burst of snapshot sends could occupy │
* │ every liveness worker and starve the acks that advance committed_lsn. │
* │ Task 3.2 offloads HA_REPL_SEND_SNAPSHOT to the SlowLane → the repl-ack │
* │ body becomes bounded → LIVENESS becomes correct. │
* │ DO NOT flip g_drive_async_peerctrl_dispatch_enabled ON until task 3.2 │
* │ has landed (and task 3.4's deadlock gate is green). │
* │ │
* │ Task 3.2 has LANDED the slow-lane offload (ha_snapshot_send_dispatch), │
* │ but it is DORMANT: g_drive_async_snapshot_offload defaults OFF (the send │
* │ still runs INLINE) because the offload borrows a standby endpoint whose │
* │ lifetime is not yet pool-safe (bug-1197). So bug-1198 is NOT yet closed │
* │ in the default build. Task 3.4 is the single gate that flips BOTH │
* │ g_drive_async_snapshot_offload AND g_drive_async_peerctrl_dispatch ON │
* │ together (offload bounds the body → LIVENESS correct) and proves the │
* │ deadlock-freedom + endpoint-lifetime properties under load. │
* │ DO NOT flip g_drive_async_peerctrl_dispatch_enabled ON until task 3.4's │
* │ gate is green WITH the snapshot offload also ON. │
* └──────────────────────────────────────────────────────────────────────────┘
*/
static vtask_class_t peer_ctrl_classify(ha_state_t *ha, io_packet_t *pkt)