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

fix(raft-ha-core-safety): close 5.10a — WAL segment rotation + endpoint UAF blocking real snapshot catch-up

Browse source 
Chain of fixes needed to get --force-snapshot soak to a real live PASS
(raft-ha-core-safety task 5.10a / ha-entity-snapshots task 7.1):

1. WAL segment rotation was gated on a hardcoded 64MB compile-time
   constant; trim_wal_segments() never deletes the active (currently-open)
   segment. At soak job counts a segment never rotated, so nothing was
   ever physically trimmable and ha_evaluate_repl_action always chose
   STREAM_WAL over SEND_SNAPSHOT for an empty-rejoining node. Fixed by
   threading a wal_segment_bytes override through persist_config_t
   (mirrors the --max-wal-records pattern: config.h default, persist.h
   setter, persist.c 3 usage sites + minimum-floor enforcement, 6-point
   CLI wiring through main.c/myconf.h/myapp.c/cluster.c) plus a new
   --wal-segment-bytes CLI flag. run-ha-soak.sh's --force-snapshot mode
   now sets WAL_SEGMENT_BYTES=4096 to force real rotation at soak scale.

2. This unmasked a genuine heap-use-after-free in INVOKE_MSG_HANDLER
   (include/netio.h): the macro calls ep->msg_handler(ep) synchronously
   (not via the vtask dispatcher, so the bug-1298 vtask ref/unref pairing
   doesn't cover it) then writes ep->fsm_in.pkt = NULL. A stale vote-reply
   handler chain (raft_on_vote_reply -> raft_cb_become_follower ->
   endpoint_unref) could drop the endpoint's last ref and free it inside
   the handler call, so the trailing write hit freed memory. This is the
   first-ever live exercise of a node receiving a snapshot transfer while
   mid-election. Fixed by holding a ref across the synchronous call, same
   pattern task 5.8 established for the async path.

With both fixes: --force-snapshot soak now PASSES live (committed
divergence=0, job_loss=0, confirmed real "HA: snapshot" install on the
restarted node) — first genuine live proof of this scenario. 3-round
--chaotic regression clean. Full 217-test controller suite green
before/after every fix in the chain.

Builds on the delaymgr starvation fix (b9ad018) and walcompare
checkpoint-floor fix (f3c86a6) from earlier in this investigation.
This commit is contained in:
Rongsong Shen 2026-07-02 04:11:20 +00:00
parent b9ad01878c
commit 1439e2655e
11 changed files with 115 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/controller/cluster.c
View file

@ -1330,6 +1330,14 @@ int cluster_init_phase2(cluster_t* c)
* deterministic record count instead of the 10000-record default. */
persist_config_set_max_wal_records(&pcfg, c->myconf->max_wal_records);
/* --wal-segment-bytes physical-segment-size override (0 = keep the
* persist default). raft-ha-core-safety task 5.10a / ha-entity-
* snapshots task 7.1: a segment is only trimmable once it rotates
* out, so forcing a real snapshot-triggered catch-up in the soak
* needs a small deterministic segment size instead of the 64 MB
* production default. */
persist_config_set_wal_segment_bytes(&pcfg, c->myconf->wal_segment_bytes);
persist_set_workdir(&c->persist, workdir, name);
rc = persist_init(&c->persist, &pcfg, c);
if (rc != 0) {

15
src/controller/main.c
View file

@ -32,6 +32,7 @@ const char* argp_program_bug_address = BUG_EMAIL;
#define OPT_SYNC_TIMEOUT 261
#define OPT_WAL_SYNC 262
#define OPT_MAX_WAL_RECORDS 263
#define OPT_WAL_SEGMENT_BYTES 264
struct arguments {
char* myname;
@ -46,6 +47,7 @@ struct arguments {
uint32_t sync_timeout_ms;
uint32_t wal_sync_ms;
uint32_t max_wal_records;
uint32_t wal_segment_bytes;
};
static struct argp_option options[] = {
@ -71,6 +73,10 @@ static struct argp_option options[] = {
{ "max-wal-records", OPT_MAX_WAL_RECORDS, "N", 0,
"WAL records since last checkpoint before an async checkpoint is "
"scheduled (0 = default; persist enforces a minimum of 100)" },
{ "wal-segment-bytes", OPT_WAL_SEGMENT_BYTES, "N", 0,
"Physical WAL segment file size in bytes; a segment is only eligible "
"for trim/delete after it rotates out (0 = default 64 MB; persist "
"enforces a minimum of 4096)" },
{ 0 }
};
@ -134,6 +140,13 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state)
arguments->max_wal_records = (uint32_t)v;
break;
}
case OPT_WAL_SEGMENT_BYTES: {
long v = atol(arg);
if (v < 0)
argp_error(state, "--wal-segment-bytes must be >= 0 (got: %s)", arg);
arguments->wal_segment_bytes = (uint32_t)v;
break;
}
default:
return ARGP_ERR_UNKNOWN;
}
@ -188,6 +201,7 @@ int main(int argc, char* argv[])
arguments.sync_timeout_ms = 0; /* 0 = not set; use persist config default */
arguments.wal_sync_ms = 0; /* 0 = not set; use persist config default */
arguments.max_wal_records = 0; /* 0 = not set; use persist config default */
arguments.wal_segment_bytes = 0; /* 0 = not set; use persist config default */
argp_parse(&argp, argc, argv, 0, 0, &arguments);
@ -239,6 +253,7 @@ int main(int argc, char* argv[])
myconf.sync_timeout_ms = arguments.sync_timeout_ms;
myconf.wal_sync_ms = arguments.wal_sync_ms;
myconf.max_wal_records = arguments.max_wal_records;
myconf.wal_segment_bytes = arguments.wal_segment_bytes;
memset(&myconf.peers_cfg, 0, sizeof(myconf.peers_cfg));
if (arguments.peers_str) {

8
src/controller/myapp.c
View file

@ -177,6 +177,14 @@ int myapp_run(myconf_t *myconf)
* deterministic record count instead of the 10000-record default. */
persist_config_set_max_wal_records(&pcfg, myconf->max_wal_records);
/* --wal-segment-bytes physical-segment-size override (0 = keep the
* persist default). raft-ha-core-safety task 5.10a / ha-entity-
* snapshots task 7.1: a segment is only trimmable once it rotates
* out, so forcing a real snapshot-triggered catch-up in the soak
* needs a small deterministic segment size instead of the 64 MB
* production default. */
persist_config_set_wal_segment_bytes(&pcfg, myconf->wal_segment_bytes);
persist_set_workdir(&c.persist, workdir, name);
rc = persist_init(&c.persist, &pcfg, &c);
if (rc != 0) {

16
src/controller/persist.c
View file

@ -94,7 +94,6 @@ static int capnp_encode_worker(const xce_worker_t* w, uint8_t** out, size_t* out
/* Constants */
/* ------------------------------------------------------------------ */
#define WAL_SEGMENT_SIZE_BYTES (64UL * 1024 * 1024) /* 64 MB per segment */
#define WAL_SEG_FMT "%s/seg-%016" PRIx64 ".wal"
/* NFS_SUPER_MAGIC — reject WAL on NFS mounts (unreliable fsync) */
@ -148,7 +147,7 @@ static int open_new_segment(persist_t* p)
/* Pre-allocate segment space to avoid fragmentation and metadata updates
* on every write. posix_fallocate is best-effort — ignore errors. */
posix_fallocate(fd, 0, (off_t)WAL_SEGMENT_SIZE_BYTES);
posix_fallocate(fd, 0, (off_t)p->config.wal_segment_bytes);
p->wal_fd = fd;
p->seg_offset = 0;
@ -571,6 +570,15 @@ int persist_init(persist_t* p, persist_config_t* cfg, cluster_t* c)
p->config.max_wal_records = 100;
}
/* Enforce minimum value for wal_segment_bytes to prevent a rotation storm
* (must comfortably fit at least one max-payload entry: header + up to
* MAX_WAL_PAYLOAD_BYTES). */
if (p->config.wal_segment_bytes < 4096) {
ERROR("persist: wal_segment_bytes=%u too low (minimum 4096) — setting to 4096",
p->config.wal_segment_bytes);
p->config.wal_segment_bytes = 4096;
}
/* Initialise atomic checkpoint-trigger counters */
atomic_store(&p->wal_record_count, 0);
atomic_store(&p->checkpoint_in_progress, 0);
@ -790,7 +798,7 @@ int persist_wal_async(persist_t* p, uint32_t type,
repl_broadcast(&p->repl, &hdr, payload, len, prev_log_term);
/* Rotate segment when full */
if (p->seg_offset >= WAL_SEGMENT_SIZE_BYTES) {
if (p->seg_offset >= p->config.wal_segment_bytes) {
if (rotate_segment(p) != 0) {
pthread_mutex_unlock(&p->lock);
return -EIO;
@ -1204,7 +1212,7 @@ int persist_wal_write_replicated(persist_t* p, uint64_t lsn, uint32_t type,
}
/* Rotate segment when full (same threshold as the primary append). */
if (p->seg_offset >= WAL_SEGMENT_SIZE_BYTES) {
if (p->seg_offset >= p->config.wal_segment_bytes) {
if (rotate_segment(p) != 0) {
pthread_mutex_unlock(&p->lock);
return -EIO;

1
src/include/config.h
View file

@ -93,6 +93,7 @@ typedef uint64_t jobid_t;
#define DEFAULT_SYNC_TIMEOUT_MS 500
#define DEFAULT_HB_INTERVAL_MS 3000
#define DEFAULT_MAX_WAL_RECORDS 10000
#define DEFAULT_WAL_SEGMENT_BYTES (64UL * 1024 * 1024) /* 64 MB per segment */
/* ======================================================
* raft-drive-async constants (task 1.1)

1
src/include/myconf.h
View file

@ -38,6 +38,7 @@ typedef struct {
uint32_t sync_timeout_ms; /* --sync-timeout-ms; 0 = use persist default */
uint32_t wal_sync_ms; /* --wal-sync-ms; 0 = use persist default */
uint32_t max_wal_records; /* --max-wal-records; 0 = use persist default */
uint32_t wal_segment_bytes; /* --wal-segment-bytes; 0 = use persist default */
} myconf_t;
extern cluster_config_t* get_default_conf(void*);

10
src/include/netio.h
View file

@ -115,13 +115,23 @@ typedef struct endpoint_t {
int heap_alloc; /* 1 iff allocated by endpoint_get (unref may free) */
} endpoint_t;
/* bug-1298 family: msg_handler runs SYNCHRONOUSLY on the io_worker thread (not
* via the vtask dispatcher, so the vtask_enq/-1 ref pair does not cover this
* call). A handler whose own logic drops ep's last reference (e.g. a stale
* vote reply driving raft_cb_become_follower -> endpoint_unref on the very ep
* currently being read) frees ep out from under the trailing pkt=NULL write
* below, a heap-use-after-free. Hold a ref for the duration of the call, same
* pattern as the vtask dispatch path, so the handler's own unref cannot drop
* the count to zero until this macro is done touching ep. */
#define INVOKE_MSG_HANDLER(ep) \
do { \
TRACE("%s msg handler %p", __func__, (ep)->msg_handler); \
if ((ep)->msg_handler != NULL) { \
endpoint_ref(ep); \
(ep)->msg_handler(ep); \
/* passing ownership of packet to upper layer */ \
(ep)->fsm_in.pkt = NULL; \
endpoint_unref(ep); \
} \
} while (0)

21
src/include/persist.h
View file

@ -100,6 +100,7 @@ typedef struct {
uint32_t worker_retain_s; /* retain unavailable workers N seconds (default 86400) */
uint32_t sync_timeout_ms; /* sync replication ACK timeout ms (default 50) */
uint32_t max_wal_records; /* checkpoint trigger: records since last snap (default 10000) */
uint32_t wal_segment_bytes; /* physical WAL segment file size (default 64 MB) */
} persist_config_t;
static inline persist_config_t persist_config_defaults(void)
@ -114,6 +115,7 @@ static inline persist_config_t persist_config_defaults(void)
c.worker_retain_s = DEFAULT_WORKER_RETAIN_S;
c.sync_timeout_ms = DEFAULT_SYNC_TIMEOUT_MS;
c.max_wal_records = DEFAULT_MAX_WAL_RECORDS;
c.wal_segment_bytes = DEFAULT_WAL_SEGMENT_BYTES;
return c;
}
@ -154,6 +156,25 @@ static inline void persist_config_set_max_wal_records(persist_config_t *c,
c->max_wal_records = override_n;
}
/*
* persist_config_set_wal_segment_bytes — apply the --wal-segment-bytes
* override (raft-ha-core-safety task 5.10a / ha-entity-snapshots task 7.1:
* the physical WAL segment size, not the record-count checkpoint threshold,
* gates when trim_wal_segments() has anything to delete — a segment is
* never rotated out until it fills, so forcing a real snapshot-triggered
* empty-rejoin in the soak requires a small deterministic segment size
* rather than waiting on the 64 MB production default).
*
* Same convention as persist_config_set_max_wal_records: override == 0
* means "flag not passed", leaves the default segment size untouched.
*/
static inline void persist_config_set_wal_segment_bytes(persist_config_t *c,
uint32_t override_bytes)
{
if (override_bytes > 0)
c->wal_segment_bytes = override_bytes;
}
/* ============================================================
* Key→index mapping for checkpoint serialization (Part 3.8)
* ============================================================ */