feat(ha-term-safety): add WAL_CONFIG_MARKER entry type + reference payload codec (task 4.1)

This commit is contained in:
Rongsong Shen 2026-06-22 09:37:23 +00:00
parent f1783f7952
commit f447ad4183
5 changed files with 215 additions and 1 deletions

View file

@ -86,7 +86,7 @@
## 4. Configuration ordering as a unified `(term,lsn)` marker (R7)
- [ ] 4.1 Add the `WAL_CONFIG_MARKER` entry type and reference payload
- [x] 4.1 Add the `WAL_CONFIG_MARKER` entry type and reference payload
- Add the new WAL entry type and its marker payload carrying only `{version_tag, sha256}` (no config bytes); no on-disk header layout change
- Observable completion: a config marker can be encoded/decoded with its version tag and 32-byte hash and carries no configuration bytes
- _Requirements: 7.2, 7.4_
@ -182,3 +182,6 @@
- **bug-960 (CRITICAL, caught by review — logged in buglog.json):** `ha_raft_on_repl_ack` first applied the votes-index→absolute remap UNCONDITIONALLY, but the walAck call site fires for BOTH promoted-vote eps (`vctx->ctx != NULL`, relative peer_idx) AND heartbeat standby eps (`vctx->ctx == NULL`, ALREADY-absolute peer_idx — see authoritative ha.c ~6172-6189 / ~5808). Over-shift → OOB `match_index[]` write (raft.c:802 has NO bounds check) in a 3-node cluster / wrong-peer FALSE-QUORUM data-loss in larger. FIX: `ha_repl_ack_abs_peer_idx(ha, ep_is_vote, peer_idx)` resolver (vote→remap, heartbeat→passthrough) resolved at the call site via `vctx->ctx != NULL`; `ha_raft_on_repl_ack` now takes ABSOLUTE idx + fail-loud `[0,n_peers)` self-excluding bounds check (logs+returns, never crashes/self-terminates). Unit tests had MASKED it (only exercised votes_idx under my_priority==0). LESSON: any peer-index passed to a library `match_index[]`/`next_index[]` write MUST be resolved per endpoint-type (vote-relative vs heartbeat-absolute) — never blind-remap; test BOTH endpoint types at my_priority 0 AND non-0.
- **DO-NOT-REPEAT (cerebrum):** an implementer ran `clang-format -i` (whole-file reformat) then `git checkout HEAD -- <files>` to undo it, which WIPED the uncommitted task diff (HEAD was at the prior task's commit). It reconstructed the diff from context and re-review confirmed fidelity, but: never `git checkout HEAD --` to undo a reformat on a file holding uncommitted work; reformat only the changed hunks.
- **STILL DEFERRED to 5.2/5.4 (carried 3.2→3.3):** live steady-state send is still TERM-0 `repl_broadcast`; the library send loop (`on_send_append_entries`) is not yet the live sender, so the ACK→commit path is proven end-to-end in UNIT TESTS but the first LIVE `on_commit_advanced(>=1)` requires retiring term-0 `repl_broadcast` + driving the library send. Live chaotic convergence (and this retirement) is validated/owned by 5.2 (clean failover) + 5.4 (chaotic regression). Phase 3 (drive library entry points) is structurally complete; phase 4 (config marker) + phase 5 (verification) remain.
- **4.1 done.** `WAL_CONFIG_MARKER = 50` (wal_types.h, between WAL_NOOP=40/WAL_CHECKPOINT=100) + `wal_config_marker_t {uint64_t version_tag; uint8_t sha256[32];}` + `WAL_CONFIG_MARKER_PAYLOAD_LEN (8u+32u)`. Reference-only codec `config_marker_encode/decode` (config_repl.c/h): explicit BIG-ENDIAN version_tag + raw 32-byte sha256 memcpy; NO config-bytes parameter on any path (R7.4); `-EINVAL`/`-ENOSPC` guards, no OOB on truncated decode/short encode. 4 tests (test_config_repl.c). 40-byte hdr static_assert intact; src/lib/raft untouched. Reviewer APPROVED.
- **For 4.2 (use, don't rediscover):** (1) the marker version_tag is consistent with the EXISTING config tag: `config_version_tag = be64toh(first 8 bytes of sha256)` (config_repl.c ~171-174/382-386), so a marker's BE version_tag bytes == sha256[0..7]. (2) `wal_classify` (persist.c ~2188) has `default: assert(0)` under XCE_TESTING and `replay_wal` (persist.c ~1499) has `default: break;` — 4.2 MUST add WAL_CONFIG_MARKER classification (and update the hand-maintained `test_classify_is_total_over_wal_entry_type` array in test_commit_barrier_rollout.c ~263-281) BEFORE any live path appends a type-50 entry, or the assert fires. The marker rides the `raft_entry_blob_t {type=WAL_CONFIG_MARKER, payload_len=40}` envelope through `ha_primary_log_append`→raft_log_append→on_persist_entry.

View file

@ -37,6 +37,57 @@
#include "xce/cluster_def.h"
#include "wal_types.h" /* wal_entry_hdr_t, WAL_* constants */
/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ───────────────
* Fixed, padding-independent serialization of a config-change marker:
* 8-byte big-endian version_tag followed by the raw 32-byte SHA-256. NO
* configuration bytes are ever encoded (R7.2/R7.4) — the bytes travel
* out-of-band on the config-push transport. The explicit byte layout (rather
* than a struct memcpy) keeps the form stable across compiler padding and host
* endianness, since this payload replicates to standbys via AppendEntries. */
int config_marker_encode(uint64_t version_tag, const uint8_t sha256[32],
uint8_t *buf, size_t bufsz, size_t *out_len)
{
if (!sha256 || !buf || !out_len)
return -EINVAL;
if (bufsz < WAL_CONFIG_MARKER_PAYLOAD_LEN)
return -ENOSPC;
/* version_tag: 8 bytes, big-endian (matches config_repl wire convention). */
buf[0] = (uint8_t)(version_tag >> 56);
buf[1] = (uint8_t)(version_tag >> 48);
buf[2] = (uint8_t)(version_tag >> 40);
buf[3] = (uint8_t)(version_tag >> 32);
buf[4] = (uint8_t)(version_tag >> 24);
buf[5] = (uint8_t)(version_tag >> 16);
buf[6] = (uint8_t)(version_tag >> 8);
buf[7] = (uint8_t)(version_tag);
/* sha256: raw 32 bytes — the configuration *reference*, never the bytes. */
memcpy(buf + 8, sha256, 32);
*out_len = WAL_CONFIG_MARKER_PAYLOAD_LEN;
return 0;
}
int config_marker_decode(const uint8_t *buf, size_t len,
uint64_t *version_tag, uint8_t sha256_out[32])
{
if (!buf || len < WAL_CONFIG_MARKER_PAYLOAD_LEN)
return -EINVAL;
if (version_tag) {
*version_tag = ((uint64_t)buf[0] << 56) | ((uint64_t)buf[1] << 48) |
((uint64_t)buf[2] << 40) | ((uint64_t)buf[3] << 32) |
((uint64_t)buf[4] << 24) | ((uint64_t)buf[5] << 16) |
((uint64_t)buf[6] << 8) | ((uint64_t)buf[7]);
}
if (sha256_out)
memcpy(sha256_out, buf + 8, 32);
return 0;
}
/* SSL context for xce_probe_config_tag.
* Stored as void* to avoid SSL_CTX type at this point in the translation unit. */
static void *s_probe_ssl_ctx = NULL;

View file

@ -9,6 +9,7 @@
#include "clusterconfig.h"
#include "netio.h"
#include "request.h"
#include "wal_types.h" /* WAL_CONFIG_MARKER, WAL_CONFIG_MARKER_PAYLOAD_LEN */
/* Max config blob size — reject before allocating */
#define CONF_MAX_SIZE (1 * 1024 * 1024) /* 1 MiB */
@ -29,6 +30,49 @@
/* Certificate expiry warning threshold (days) */
#define CERT_WARN_DAYS 30
/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ───────────────
*
* A config-change marker payload is a *reference* to a configuration — its
* version tag and full SHA-256 content hash — and carries NO configuration
* bytes (R7.2/R7.4). The bytes always travel out-of-band on the config-push
* transport; they are never placed in a WAL payload. This codec produces the
* fixed, padding-independent on-wire form (big-endian version_tag + raw 32-byte
* sha256) that rides inside the raft_entry_blob_t envelope as the payload of a
* WAL_CONFIG_MARKER entry.
*/
/*
* config_marker_encode: serialize a {version_tag, sha256} config-change marker
* into a fixed WAL_CONFIG_MARKER_PAYLOAD_LEN-byte buffer. NO config bytes are
* ever written. version_tag is emitted big-endian; sha256 is the raw 32 bytes.
*
* buf — output buffer (must be >= WAL_CONFIG_MARKER_PAYLOAD_LEN)
* bufsz — capacity of buf
* out_len — receives the number of bytes written (always
* WAL_CONFIG_MARKER_PAYLOAD_LEN on success); untouched on failure
*
* Returns 0 on success; -EINVAL if sha256/buf/out_len is NULL;
* -ENOSPC if bufsz < WAL_CONFIG_MARKER_PAYLOAD_LEN.
*/
int config_marker_encode(uint64_t version_tag, const uint8_t sha256[32],
uint8_t *buf, size_t bufsz, size_t *out_len);
/*
* config_marker_decode: deserialize a WAL_CONFIG_MARKER payload produced by
* config_marker_encode. Reads exactly WAL_CONFIG_MARKER_PAYLOAD_LEN bytes; a
* shorter buffer is rejected without any out-of-bounds read.
*
* buf — input buffer
* len — bytes available in buf
* version_tag — receives the host-order version tag (may be NULL to skip)
* sha256_out — receives the raw 32-byte content hash (may be NULL to skip)
*
* Returns 0 on success; -EINVAL if buf is NULL or len <
* WAL_CONFIG_MARKER_PAYLOAD_LEN.
*/
int config_marker_decode(const uint8_t *buf, size_t len,
uint64_t *version_tag, uint8_t sha256_out[32]);
/* ── Phase 1: Storage ────────────────────────────────────────── */
/*

View file

@ -504,6 +504,92 @@ static void test_conf_save_config_path_consistency(void)
printf("test_conf_save_config_path_consistency: PASS\n");
}
/* ── WAL_CONFIG_MARKER codec (ha-term-safety task 4.1) ───────────────
* The config-change marker payload carries ONLY {version_tag, sha256} —
* a reference, never the configuration bytes (R7.2/R7.4). These tests
* exercise the fixed-size encode/decode codec that rides inside the
* raft_entry_blob_t envelope with type == WAL_CONFIG_MARKER. */
/* Round-trip: encode {version_tag, sha256} then decode → identical, and the
* encoded form is exactly the fixed marker size (tag + 32-byte hash) with NO
* configuration bytes appended. */
static void test_config_marker_encode_decode_roundtrip(void)
{
const uint64_t version_tag = 0x0123456789abcdefULL;
uint8_t sha256[32];
for (int i = 0; i < 32; i++)
sha256[i] = (uint8_t)(0xA0 + i);
uint8_t buf[64];
size_t enc_len = 0;
int rc = config_marker_encode(version_tag, sha256, buf, sizeof(buf), &enc_len);
TEST_ASSERT_EQUAL_INT(0, rc);
/* Encoded form carries NO config bytes: exactly 8 (tag) + 32 (sha256). */
TEST_ASSERT_EQUAL_UINT(WAL_CONFIG_MARKER_PAYLOAD_LEN, enc_len);
TEST_ASSERT_EQUAL_UINT(8u + 32u, enc_len);
uint64_t got_tag = 0;
uint8_t got_sha[32];
memset(got_sha, 0, sizeof(got_sha));
rc = config_marker_decode(buf, enc_len, &got_tag, got_sha);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_EQUAL_HEX64(version_tag, got_tag);
TEST_ASSERT_EQUAL_HEX8_ARRAY(sha256, got_sha, 32);
printf("test_config_marker_encode_decode_roundtrip: PASS\n");
}
/* The WAL entry type value must be exactly 50 and must not collide with the
* neighbouring WAL_NOOP (40) or WAL_CHECKPOINT (100) types (design.md). */
static void test_config_marker_enum_value_is_50(void)
{
TEST_ASSERT_EQUAL_INT(50, WAL_CONFIG_MARKER);
TEST_ASSERT_NOT_EQUAL(WAL_NOOP, WAL_CONFIG_MARKER);
TEST_ASSERT_NOT_EQUAL(WAL_CHECKPOINT, WAL_CONFIG_MARKER);
/* Strictly between WAL_NOOP and WAL_CHECKPOINT. */
TEST_ASSERT_TRUE(WAL_CONFIG_MARKER > WAL_NOOP);
TEST_ASSERT_TRUE(WAL_CONFIG_MARKER < WAL_CHECKPOINT);
printf("test_config_marker_enum_value_is_50: PASS\n");
}
/* A truncated (short) buffer must fail cleanly with -EINVAL, never read past
* the end of the supplied buffer. */
static void test_config_marker_decode_truncated_fails(void)
{
uint8_t sha256[32];
memset(sha256, 0x5c, sizeof(sha256));
uint8_t buf[WAL_CONFIG_MARKER_PAYLOAD_LEN];
size_t enc_len = 0;
TEST_ASSERT_EQUAL_INT(0, config_marker_encode(0xdeadbeefcafef00dULL, sha256,
buf, sizeof(buf), &enc_len));
uint64_t tag = 0;
uint8_t sha_out[32];
/* One byte short of a full marker payload → reject, no OOB read. */
int rc = config_marker_decode(buf, enc_len - 1, &tag, sha_out);
TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
/* Zero-length buffer → reject. */
rc = config_marker_decode(buf, 0, &tag, sha_out);
TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
printf("test_config_marker_decode_truncated_fails: PASS\n");
}
/* A too-small output buffer must fail with -ENOSPC and not overflow. */
static void test_config_marker_encode_short_buffer_fails(void)
{
uint8_t sha256[32];
memset(sha256, 0x11, sizeof(sha256));
uint8_t small[WAL_CONFIG_MARKER_PAYLOAD_LEN - 1];
size_t enc_len = 123; /* sentinel — must be left untouched on failure */
int rc = config_marker_encode(7, sha256, small, sizeof(small), &enc_len);
TEST_ASSERT_EQUAL_INT(-ENOSPC, rc);
printf("test_config_marker_encode_short_buffer_fails: PASS\n");
}
/* ── wait_all_with_timeout (XCE_TESTING shim) ───── */
#ifdef XCE_TESTING
@ -2260,6 +2346,11 @@ void test_config_repl(void)
RUN_TEST(test_sanitise_cn_control_chars);
RUN_TEST(test_sanitise_cn_truncated);
RUN_TEST(test_conf_save_config_path_consistency);
/* Task 4.1: WAL_CONFIG_MARKER type + reference-only codec */
RUN_TEST(test_config_marker_enum_value_is_50);
RUN_TEST(test_config_marker_encode_decode_roundtrip);
RUN_TEST(test_config_marker_decode_truncated_fails);
RUN_TEST(test_config_marker_encode_short_buffer_fails);
#ifdef XCE_TESTING
RUN_TEST(test_load_primary_config_missing_returns_error);
RUN_TEST(test_wait_all_with_timeout_slow_threads);

View file

@ -76,5 +76,30 @@ typedef enum {
WAL_QUEUE_CTRL = 20,
WAL_WORKER_CTRL = 30,
WAL_NOOP = 40, /* Internal: empty payload, replay no-op; appended only at promotion */
WAL_CONFIG_MARKER = 50, /* Config-change marker: {version_tag, sha256} reference only —
* NEVER the config bytes (R7.2/R7.4). Ordered + committed through
* the same (term,lsn) log as job state (ha-term-safety). */
WAL_CHECKPOINT = 100, /* Local WAL marker only — never replicated */
} wal_entry_type;
/* ── WAL_CONFIG_MARKER payload (ha-term-safety task 4.1) ────────────────────
* The marker payload is a *reference* to a configuration — its version tag and
* full SHA-256 content hash — and carries NO configuration bytes (R7.2/R7.4).
* The configuration bytes always travel out-of-band on the existing config-push
* transport (OPCode_replConfigPush), keyed to {version_tag, sha256}; they are
* NEVER placed in a WAL payload.
*
* This struct rides INSIDE the raft_entry_blob_t envelope with
* type == WAL_CONFIG_MARKER; on persist it lands as the WAL entry payload. The
* 40-byte wal_entry_hdr_t is unchanged (R1.7) — this is a payload, not a header.
* Use config_marker_encode / config_marker_decode (config_repl.c) for a fixed,
* padding-independent on-wire form. */
typedef struct {
uint64_t version_tag; /* first 8 bytes of the config SHA-256, host order */
uint8_t sha256[32]; /* full SHA-256 of the config bytes (the reference) */
} wal_config_marker_t;
/* Fixed serialized size of a WAL_CONFIG_MARKER payload: 8-byte version_tag +
* 32-byte sha256, with NO config bytes ever appended. This is the exact byte
* count config_marker_encode emits and config_marker_decode requires. */
#define WAL_CONFIG_MARKER_PAYLOAD_LEN (8u + 32u)