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

fix(repl): harden standby slot allocation against WAL-retention guard clobber (bug-1474)

Browse source 
repl_add_standby()'s "find first empty slot" scan treated any ep==NULL
slot as available, with no awareness that a disconnected slot can still
hold an unexpired bug-1147 WAL-retention guard (last_disconnected_lsn,
set by repl_remove_standby, meant to be cleared only by
repl_expire_disconnect_slots once snap_interval_s genuinely elapses).
The scan could pick a guard-holding slot over a genuinely virgin one
purely by lower index, then unconditionally memset it -- destroying the
guard before trim_wal_segments/repl_trim_lsn could ever honor it.
Confirmed via 2/2 reproducible live 3-node soak runs (config-upgrade-
snapshot task 8.9's new --config-upgrade-wal-trim mode): a killed
standby's still-needed WAL segment got trimmed out from under it.

Now a two-pass scan: prefer a genuinely virgin slot; only fall back to
reusing a guard-holding slot (oldest last_disconnect_time first, to
discard the least remaining protection) when no virgin slot exists --
logged via WARN() so the discarded guard is operator-visible rather
than silent. Defense-in-depth at the slot-allocation layer, independent
of whatever triggers a given repl_add_standby call (a separate,
not-fully-root-caused ha.c reconnect-sweep defect is the specific
trigger seen in the soak, out of this fix's scope).

Independently reviewed across two rounds: round 1 flagged the
exhausted-fallback path silently discarding a guard with zero logging;
remediated with a WARN() matching this file's existing repl_broadcast
convention, empirically confirmed to fire only on the fallback path
with correct content. Suite: 1028 -> 1030 tests, 0 failures (net of
this fix; final combined total with bug-1472 landing concurrently:
1033 tests, 0 failures).
This commit is contained in:
Rongsong Shen 2026-07-26 04:11:27 +00:00
parent 7547a8f14b
commit c0b0c7d1de
2 changed files with 189 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

60
src/controller/repl.c
View file

@ -93,15 +93,60 @@ int repl_add_standby(replicator_t *r, repl_ep_t ep, const char *host, uint32_t p
return -ENOSPC;
}
/* Find first empty slot */
/* Slot allocation (bug-1474 hardening): prefer a genuinely virgin slot
* over one still holding an unexpired bug-1147 disconnect guard
* (last_disconnected_lsn != 0) for a DIFFERENT peer. repl_add_standby
* cannot know whether this call is a legitimate new connection or a
* duplicate/late-joiner reconnect for an already-connected peer (that
* classification defect lives in ha.c, out of scope here) — but either
* way, clobbering an unexpired guard is never correct: it silently
* defeats repl_trim_lsn's WAL retention floor for whichever standby
* the guard was protecting (see .wolf/buglog.json bug-1474).
*
* Pass 1: any slot with ep==NULL && last_disconnected_lsn==0 (virgin —
* never used, or its guard already expired via
* repl_expire_disconnect_slots, which fully zeroes the slot). Always
* safe to take; first such slot by index.
*
* Pass 2 (fallback, only when Pass 1 finds nothing): among slots with
* ep==NULL && last_disconnected_lsn!=0 (guard-holding), reuse the one
* with the OLDEST last_disconnect_time. Deliberate policy: that guard
* is closest to its own natural expiry (repl_expire_disconnect_slots'
* snap_interval_s window), so reusing it discards the LEAST remaining
* WAL-retention protection of any guard-holding slot. With
* MAX_STANDBYS==3 it is possible — under a chaotic multi-disconnect
* scenario with no virgin slot free — for every non-primary slot to be
* guard-holding simultaneously; refusing this connect (-ENOSPC) would
* trade a bounded, already-degraded WAL-retention risk for an outright
* connection failure, which is worse for cluster availability. This
* fallback does NOT touch the -ENOSPC path above, which remains
* reserved for the genuinely-impossible case of more distinct live
* peers than slots exist. */
int idx = -1;
for (int i = 0; i < MAX_STANDBYS; i++) {
if (r->standbys[i].ep == NULL) {
if (r->standbys[i].ep == NULL && r->standbys[i].last_disconnected_lsn == 0) {
idx = i;
break;
}
}
/* Pass 1 found nothing -- about to fall back to Pass 2 (guard-holding
* slot reuse) below. Tracked so the WARN below fires only for the
* genuine fallback case, not the common virgin-slot path. */
int used_fallback_slot = (idx < 0);
if (idx < 0) {
for (int i = 0; i < MAX_STANDBYS; i++) {
if (r->standbys[i].ep != NULL) {
continue;
}
if (idx < 0 ||
r->standbys[i].last_disconnect_time < r->standbys[idx].last_disconnect_time) {
idx = i;
}
}
}
if (idx < 0) {
/* Should not happen given nstandbys guard above, but be safe */
mylock_unlock(&r->lock);
@ -109,6 +154,17 @@ int repl_add_standby(replicator_t *r, repl_ep_t ep, const char *host, uint32_t p
}
standby_conn_t *s = &r->standbys[idx];
if (used_fallback_slot) {
/* bug-1474 review finding: this silently destroys the slot's
* WAL-retention guard below (via memset) -- log it loudly, mirroring
* repl_broadcast's existing "degraded but continuing" WARN
* convention (e.g. "standby %s:%u evicted", line ~314 above), so the
* discarded protection is operator-visible instead of silent. */
WARN("repl_add_standby: reusing guard-holding slot %d (host=%s:%u,"
" last_disconnected_lsn=%lu) -- no virgin slot available,"
" WAL-retention protection discarded",
idx, s->host, s->port, (unsigned long)s->last_disconnected_lsn);
}
memset(s, 0, sizeof(*s));
s->ep = ep;
s->port = port;