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

feat(config-upgrade-snapshot): embed active configuration bytes into every checkpoint

Browse source 
write_snapshot now appends a flat [config_bytes_len][raw bytes]
section after the host frames, reading the locally-held active config
straight off disk (read_active_config_bytes, persist.c) rather than
calling into config_repl.c -- mirrors scan_latest_committed_config_ref's
existing precedent of avoiding a persist.c -> config_repl.c
function-call dependency. Populated on every checkpoint, not only
marker-triggered ones (Req 2.4). No local config yet (early boot)
degrades to an explicit empty section rather than failing the
checkpoint; genuine corruption still fails loud. Task 2.3 of 19.

Requirements: 2.1, 2.3, 2.4, 5.1
This commit is contained in:
Rongsong Shen 2026-07-24 11:56:12 +00:00
parent 70ff3e5b80
commit 387ea35c9f
3 changed files with 494 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

187
src/controller/persist.c
View file

@ -2063,6 +2063,143 @@ static int collect_host_frames(persist_t* p, xlist_t* frames)
return rc;
}
/* Bound on the config bytes this file will embed in a checkpoint -- mirrors
* config_repl.h's CONF_MAX_SIZE (1 MiB) exactly. Duplicated as its own local
* constant rather than including config_repl.h, per the zero-config_repl.c-
* dependency decision documented on read_active_config_bytes() below. */
#define PERSIST_CONFIG_BYTES_MAX_SIZE (1u * 1024u * 1024u)
/* read_active_config_bytes (config-upgrade-snapshot task 2.3, Req 2.1, 2.3,
* 2.4, 5.1): read the locally-held active configuration's raw bytes straight
* off disk, for embedding in every checkpoint's trailing config-bytes
* section.
*
* DELIBERATE DESIGN CHOICE -- read this before changing the call site below:
* this function duplicates config_repl.c's `conf_read_current_filename` +
* `conf_read_raw` path-resolution-and-read logic INLINE, rather than calling
* those functions (config_repl.h is not included by this file at all). This
* mirrors scan_latest_committed_config_ref()'s own precedent just above,
* which independently decodes WAL_CONFIG_MARKER payloads inline "so persist
* stays free of a config_repl layering dependency" instead of adding a
* persist.c -> config_repl.c function-call edge -- and design.md's
* "Snapshot Entity/Config Payload" + File Structure Plan sections direct the
* SAME treatment here verbatim ("no new config_repl.c function is added...
* this deliberately mirrors scan_latest_committed_config_ref's own existing
* precedent of avoiding a persist.c -> config_repl.c function-call
* dependency, rather than introducing the layering violation a new accessor
* would create").
*
* A future reader who spots ha.c's two `extern int conf_read_raw(...)` call
* sites (config push to standbys) and wonders why this file doesn't do the
* same: ha.c is the intentional CONSUMER of config-bytes sourcing
* (design.md's Architecture boundary map: "config replication (config_repl.c)
* owns config-bytes sourcing"), so a ha.c -> config_repl.c call is expected,
* in-boundary integration. persist.c owns materialization/on-disk-format and
* is deliberately kept free of any dependency on config_repl.c's domain --
* the same boundary this file already draws for the WAL-marker case above.
* This is a considered choice, not an oversight: duplicating this ~30-line
* read-with-validation routine once was judged cheaper than adding this
* file's first cross-domain coupling edge into config_repl.c.
*
* Returns 0 with *out_bytes (caller must free()) / *out_len set on success.
* Returns -ENOENT when no local config bytes exist yet: no cluster/myconf
* back-pointer (e.g. a bare unit-test persist_t), no workdir/name configured,
* no conf/current pointer file yet (this node has not received its first
* config push), or the pointer names a versioned file that is not present.
* This is a BENIGN, expected state (early boot / not-yet-provisioned), never
* corruption -- callers must treat -ENOENT as "no config yet", not failure.
* Any OTHER negative errno (-EINVAL malformed pointer/oversized file, -EIO
* read failure, -ENOMEM, -EFBIG) indicates genuine on-disk corruption or a
* real I/O failure and must propagate as a hard failure, never be silently
* downgraded to an empty section. */
static int read_active_config_bytes(persist_t* p, uint8_t** out_bytes,
size_t* out_len)
{
cluster_t* c = p->cluster;
if (c == NULL || c->myconf == NULL ||
c->myconf->workdir == NULL || c->myconf->name == NULL)
return -ENOENT;
/* effective_dir = workdir/name -- must match conf_save_config's own
* path construction (config_repl.c:781) exactly. */
char conf_dir[4096];
snprintf(conf_dir, sizeof(conf_dir), "%s/%s", c->myconf->workdir,
c->myconf->name);
/* Step 1: resolve the "current" versioned filename -- mirrors
* conf_read_current_filename's validation exactly (non-empty, no path
* separator, "cluster.bin." prefix + 12 lowercase hex chars) so a
* corrupted pointer file is caught here just as loudly as it would be
* there. */
char head_path[4160];
snprintf(head_path, sizeof(head_path), "%s/conf/current", conf_dir);
FILE* hf = fopen(head_path, "r");
if (hf == NULL)
return -ENOENT; /* no config pushed to this node yet */
struct stat hst;
if (fstat(fileno(hf), &hst) == 0 && hst.st_size == 0) {
fclose(hf);
return -ENOENT; /* interrupted-write pointer file -- treat as absent */
}
char fname[256];
if (fgets(fname, (int)sizeof(fname), hf) == NULL) {
fclose(hf);
return -EIO;
}
fclose(hf);
fname[strcspn(fname, "\n")] = '\0';
if (strchr(fname, '/') != NULL)
return -EINVAL; /* corrupted pointer file -- embedded path separator */
if (strncmp(fname, "cluster.bin.", 12) != 0)
return -EINVAL;
const char* hex = fname + 12;
if (strlen(hex) != 12 || strspn(hex, "0123456789abcdef") != 12)
return -EINVAL;
/* Step 2: read the versioned config file's raw bytes. */
char cfg_path[4416];
snprintf(cfg_path, sizeof(cfg_path), "%s/conf/%s", conf_dir, fname);
FILE* cf = fopen(cfg_path, "rb");
if (cf == NULL)
return -ENOENT; /* pointer references a file that is not there --
* treat as not-yet-ready rather than corruption */
if (fseek(cf, 0, SEEK_END) != 0) { fclose(cf); return -EIO; }
long sz = ftell(cf);
if (sz < 0) { fclose(cf); return -EIO; }
if (fseek(cf, 0, SEEK_SET) != 0) { fclose(cf); return -EIO; }
if (sz == 0) {
fclose(cf);
return -EIO; /* zero-byte versioned config file: genuine corruption */
}
if ((size_t)sz > PERSIST_CONFIG_BYTES_MAX_SIZE) {
fclose(cf);
return -EFBIG;
}
uint8_t* buf = (uint8_t*)malloc((size_t)sz);
if (buf == NULL) {
fclose(cf);
return -ENOMEM;
}
if (fread(buf, 1, (size_t)sz, cf) != (size_t)sz) {
free(buf);
fclose(cf);
return -EIO;
}
fclose(cf);
*out_bytes = buf;
*out_len = (size_t)sz;
return 0;
}
/* Write an entity-bearing checkpoint: legacy header (unchanged) + entity
* payload (ha-entity-snapshots task 2.2/3.2, Req 1.1-1.4, 3.1-3.3). Any
* failure at any step -- collect, encode, write, sync -- leaves NO file at
@ -2197,6 +2334,56 @@ static int write_snapshot(persist_t* p)
}
}
/* Config-bytes section (config-upgrade-snapshot task 2.3, Req 2.1, 2.3,
* 2.4, 5.1): raw active-configuration bytes, appended LAST. This is a
* single flat length-prefixed blob for the whole file -- NOT a
* [count][frame...] shape like the job/queue/host sections above, since
* there is exactly one active configuration, never a set of them.
* Populated on EVERY checkpoint (marker-triggered or periodic alike,
* never gated on which triggered this call) so a member joining or
* catching up after the marker-triggered upgrade snapshot has rotated
* away still gets configuration and materialized state together from
* whichever checkpoint it installs (Req 2.4). A pure function of
* whatever bytes currently sit at the resolved config path -- no
* timestamp, no ordering dependence -- so reading the same unchanged
* file twice yields byte-identical output (Req 5.1 determinism). */
uint8_t* cfg_bytes = NULL;
size_t cfg_len = 0;
if (rc == 0) {
int cfg_rc = read_active_config_bytes(p, &cfg_bytes, &cfg_len);
if (cfg_rc == -ENOENT) {
/* No local config bytes yet -- no cluster/myconf back-pointer
* (bare unit-test persist_t) or this node has not received its
* first config push. Mirrors the "no live cluster back-pointer"
* empty-is-not-failure discipline collect_job/queue/host_frames
* already apply above: write an explicit zero-length section
* rather than failing the whole checkpoint over a benign,
* expected early-boot / not-yet-provisioned state. */
cfg_bytes = NULL;
cfg_len = 0;
} else if (cfg_rc != 0) {
/* Any other error means genuinely corrupt/unreadable on-disk
* config state, not benign absence -- fail the checkpoint like
* any other collect/restore failure (design.md Error Handling:
* "Host placeholder creation failure ... treated as any other
* collect/restore failure today -- discard partial state, fail
* the checkpoint/restore attempt, retriable on the next
* cycle"). */
rc = cfg_rc;
}
}
if (rc == 0) {
uint32_t cfg_bytes_len = (uint32_t)cfg_len;
if (write(fd, &cfg_bytes_len, sizeof(cfg_bytes_len)) !=
(ssize_t)sizeof(cfg_bytes_len))
rc = -EIO;
}
if (rc == 0 && cfg_len > 0 &&
write(fd, cfg_bytes, cfg_len) != (ssize_t)cfg_len)
rc = -EIO;
free(cfg_bytes);
if (rc == 0 && fdatasync(fd) != 0)
rc = -errno;