feat(raft-apply-decoupling): task 7.1 apply-ordering conformance assertions
Extend raft_sim.c's deterministic TLA+-conformance harness with two fault-injectable checks: no reaction before commit, and idempotent re-apply of an already-committed entry. Wired as dedicated tests only, not into raft_sim_check_all's fixed four-invariant contract (owned by a separate spec). RED/GREEN independently verified by adversarial review; zero regression to existing scenario functions.
This commit is contained in:
parent
2de410c877
commit
5f98d1b0ef
4 changed files with 347 additions and 5 deletions
|
|
@ -158,7 +158,7 @@
|
|||
- _Depends: 6.1, 4.2_
|
||||
|
||||
- [ ] 7. Conformance, regression, and existing-behavior preservation
|
||||
- [ ] 7.1 Extend the deterministic conformance harness with apply-ordering assertions
|
||||
- [x] 7.1 Extend the deterministic conformance harness with apply-ordering assertions
|
||||
- Add an assertion that no entry's state mutation or reactions occur before that entry commits
|
||||
- Add an assertion that re-applying an already-committed entry is idempotent
|
||||
- Observable: the conformance harness run reports both new assertions passing alongside its existing safety checks
|
||||
|
|
@ -188,6 +188,9 @@
|
|||
|
||||
## Implementation Notes
|
||||
|
||||
- **7.1**: correctly identified `src/lib/raft/raft_sim.c` (pre-existing `#ifdef XCE_TESTING` TLA+-conformance simulator, driven by `src/lib/testsuite/test_raft.c` in the `xcelib_test` Unity binary — a different binary/library boundary than every other task in this spec, which use `controller_unit_test`) as the intended "deterministic conformance harness," confirmed verbatim against design.md's Testing Strategy line 442 ("Extend `raft_sim.c`'s conformance harness with 'no reaction before commit' and 'idempotent re-apply' assertions layered on its existing apply-after-`on_commit_advanced` loop"). Added two fault-injectable checker functions (`raft_sim_check_apply_after_commit`, `raft_sim_check_idempotent_reapply`) plus `raft_sim_replay_commit`, gated by two new flags (`RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT`, `RAFT_SIM_BREAK_REAPPLY_GUARD`) and a new `apply_reaction_count[]` field on `raft_sim_node_t`. Wired into `sim_persist_entry`/`sim_conf_on_commit_advanced`, both no-ops (byte-identical to prior behavior, confirmed by reviewer) when the flags are unset — zero regression risk to the 5 pre-existing scenario functions. RED phase independently reproduced by reviewer for both checks (`Expected 39338 Was 0`, `Expected 2 Was 1`); GREEN 287/0/0 confirmed twice (implementer + reviewer), ASan-clean.
|
||||
**Scoping decision, independently upheld by reviewer**: the two new checks run only via their own 2 dedicated tests, NOT wired into `raft_sim_check_all`/`sim_scen_check` (the umbrella the 5 pre-existing scenario functions use). Reviewer traced this to `raft-sim-conformance` spec's Requirement 3 Criterion 5 (fixed four-invariant, fixed-order, halt-on-first-failure contract) — an already-shipped contract owned by a *different* spec that 7.1 has no charter to modify. Also: these checks operate on the harness's synthetic `applied[]`/`apply_reaction_count[]` model, not real production `apply_committed_entry()`/`persist_apply_pump_run()` — real production coverage of this ordering already exists via tasks 6.1-6.4's `test_apply_pump_integration.c`. Confirmed legitimate scoping, not a corner-cut.
|
||||
**Residual, non-blocking documentation nitpick**: the `applied[k-1] != 0` "has-been-applied" sentinel is not structurally guaranteed non-zero the way log-entry term 0 is (raft.tla guarantees terms start at 1; arbitrary client payloads have no such guarantee) — the header comment's "same zero-means-absent convention as log-entry terms" analogy slightly overclaims precedent (the pre-existing `raft_sim_check_state_machine_safety` bounds by `commit_index` directly, doesn't use a zero-sentinel at all). Currently harmless because the only two call sites use hardcoded non-zero literals (`0x99AA`, `0x77BB`). **Fast-follow flag for whoever later widens this harness to PRNG-drawn payload values**: `apply_reaction_count[]` cannot substitute as the "ever-applied" indicator for `check_apply_after_commit`, since the `BREAK_APPLY_BEFORE_COMMIT` fault only touches `applied[]` — a real generalization needs a third parallel "ever-written" flag array, not a reuse of either existing array.
|
||||
- **1.1**: `commit_submit_async()` reuses the exact `wait_admit_locked`/`wait_release_locked`/`inflight_waits` admission-cap discipline as `COMMIT_MODE_WAIT` — task 1.2's `commit_resolve()` MUST call `wait_release_locked()` when resolving an async-registered entry, or the shared admission cap leaks. Continuation registry is `calloc(n_vcpus, ...)`, sized so `n_continuations >= wait_cap(ce)` always holds; `lsn == 0` is the free-slot sentinel.
|
||||
- **1.1 review**: bare `controller_unit_test` (no `--run` filter) segfaults deterministically in this environment (SSL_CTX_free/iomgr_fini crash, `test_ha_vep_teardown.c`) — reproduced identically on the pre-task base commit, confirmed pre-existing and unrelated. Logged as bug-1406. Also `test_commit_advance_retry_clears_push_before_commit_fence` (`test_config_repl.c`) fails independently of this feature, also reproduced on base — logged as bug-1407, flagged because it's adjacent to `commit_advance_locked`, which task 1.2 modifies. **Always validate with a filtered `--run <group>` invocation or the tightly-coupled commit-engine cluster, not the bare unfiltered binary, until 1406/1407 are separately fixed.**
|
||||
- **1.2**: implemented `commit_resolve()` + wired it into all 6 commit.c-internal paths that can advance `committed_lsn`/`writable_lsn` (the COMMITTED/majority-ack outcome). Chose `VTASK_LIVE_BLOCK` for dispatch (never admission-gated, unlike `VTASK_REQ_BLOCK` which can fast-reject — unacceptable for "exactly-once, never unresolved"). **Reviewer caught a real task-graph gap, not a defect in 1.2 itself**: converting `COMMIT_MODE_WAIT` away from a blocking wait also removes the *only* mechanism that ever detected a commit timing out (the caller's own `pthread_cond_timedwait` deadline) — nothing yet arms an equivalent timeout for async-registered continuations, and nothing yet wires demotion/log-truncation detection to call `commit_resolve(FAILED, ...)`. Without this, a pending async command during quorum loss would hang forever instead of failing — worse than today's behavior. Split out as new tasks **1.3** (timeout arming) and **1.4** (demotion/truncation wiring), inserted before the renumbered **1.5** (was 1.3, concurrency test — now exercises the real detection paths from 1.3/1.4 instead of calling `commit_resolve()` directly). Task 3.2 now also depends on 1.3/1.4, since its own "failure/timeout outcome replies" observable is meaningless without them.
|
||||
|
|
|
|||
|
|
@ -467,9 +467,24 @@ void raft_snapshot(const raft_state_t *s, raft_snapshot_t *snap);
|
|||
* supplied term (emulates G1: entries carry term 0 so on_get_entry_term cannot
|
||||
* distinguish same-index/different-term entries).
|
||||
* - RAFT_SIM_BREAK_TRUNCATE: on_truncate_from is a no-op (emulates G3: a divergent
|
||||
* suffix is never deleted). */
|
||||
* suffix is never deleted).
|
||||
* - RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT (raft-apply-decoupling task 7.1):
|
||||
* on_persist_entry ALSO eagerly decodes and writes the entry's value into
|
||||
* applied[] at persist time, before the entry ever commits — emulating an
|
||||
* apply-before-commit regression (the defect class raft-apply-decoupling
|
||||
* fixes). Lets the new raft_sim_check_apply_after_commit oracle be proven
|
||||
* sensitive, not merely vacuously passing.
|
||||
* - RAFT_SIM_BREAK_REAPPLY_GUARD (raft-apply-decoupling task 7.1):
|
||||
* on_commit_advanced (conformance variant) reprocesses the FULL committed
|
||||
* prefix on every call instead of only the newly-committed suffix, so a
|
||||
* forced replay (raft_sim_replay_commit) re-fires an already-applied
|
||||
* entry's reaction — emulating a duplicate/retried commit notification
|
||||
* that is not idempotent. Lets the new raft_sim_check_idempotent_reapply
|
||||
* oracle be proven sensitive. */
|
||||
#define RAFT_SIM_BREAK_TERM_STAMP 0x1u
|
||||
#define RAFT_SIM_BREAK_TRUNCATE 0x2u
|
||||
#define RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT 0x4u
|
||||
#define RAFT_SIM_BREAK_REAPPLY_GUARD 0x8u
|
||||
|
||||
/* Buffered RPC entry: one pending vote-request to route.
|
||||
* last_log_term/last_log_index capture the CANDIDATE's advertised last-log tuple
|
||||
|
|
@ -614,6 +629,15 @@ typedef struct {
|
|||
* satisfies the design's `log_len` requirement for the in-memory log length;
|
||||
* no separate log_len field is added (it would duplicate log_count). */
|
||||
uint64_t applied[RAFT_SIM_MAX_LOG];
|
||||
/* apply_reaction_count[k-1] counts how many times committed index k's
|
||||
* apply-time reaction has fired (raft-apply-decoupling task 7.1). Under
|
||||
* correct behavior this is always 0 or 1 per index — a value > 1 means the
|
||||
* SAME committed entry's mutation/reaction ran more than once (a genuine
|
||||
* duplicate application), which raft_sim_check_idempotent_reapply asserts
|
||||
* never happens (Req 10.3). Incremented alongside applied[] inside
|
||||
* sim_conf_on_commit_advanced only; the legacy (non-conformance) adapter
|
||||
* never touches it, so it stays all-zero for raft_sim_init clusters. */
|
||||
int apply_reaction_count[RAFT_SIM_MAX_LOG];
|
||||
/* Persisted-(term, votedFor) shadow surviving a Restart, mirroring raft.tla's
|
||||
* `Restart` (keeps currentTerm/votedFor/log, resets volatile state). The
|
||||
* conformance restart action restores from this shadow rather than re-reading
|
||||
|
|
@ -855,6 +879,41 @@ int raft_sim_check_leader_completeness(const raft_sim_cluster_t *c);
|
|||
*/
|
||||
int raft_sim_check_state_machine_safety(const raft_sim_cluster_t *c);
|
||||
|
||||
/* ---- additive apply-ordering assertions (raft-apply-decoupling task 7.1) ----
|
||||
*
|
||||
* The two checks below are layered ADDITIONS to the conformance harness's existing
|
||||
* apply-after-on_commit_advanced loop (sim_conf_on_commit_advanced): they verify
|
||||
* Req 10.1 and 10.3 against the harness's own apply model (applied[] /
|
||||
* apply_reaction_count[]). They are deliberately NOT members of raft_sim_invariant_t
|
||||
* and are NOT evaluated by raft_sim_check_all — that enum/function's four-invariant
|
||||
* FIXED order is the raft.tla safety-property contract (Req 3.5) and is left
|
||||
* untouched. Callers assert on these return values directly, same 1=holds/0=violated
|
||||
* convention as the four checks above. */
|
||||
|
||||
/*
|
||||
* raft_sim_check_apply_after_commit (Req 10.1): assert that no entry's state
|
||||
* mutation/reaction — the harness's apply model, node->applied[] — occurs before
|
||||
* that entry commits (node->commit_index reaching its index). applied[] uses the
|
||||
* same zero-means-absent convention already established for log-entry terms
|
||||
* elsewhere in this file: an untouched slot reads exactly 0, and every value this
|
||||
* harness's tests/scheduler ever apply (PRNG draws or hand-authored client values)
|
||||
* is non-zero, so "applied[k-1] != 0" reliably means "index k has been applied".
|
||||
* Returns 1 when, for every node and every index k, applied[k-1] != 0 implies
|
||||
* k <= commit_index; 0 on the first violation. Pure read-only over c.
|
||||
*/
|
||||
int raft_sim_check_apply_after_commit(const raft_sim_cluster_t *c);
|
||||
|
||||
/*
|
||||
* raft_sim_check_idempotent_reapply (Req 10.3): assert that re-applying an
|
||||
* already-committed entry — whether a genuine duplicate on_commit_advanced fire or
|
||||
* a forced raft_sim_replay_commit retry — never re-executes that entry's apply-time
|
||||
* mutation/reaction. Backed by node->apply_reaction_count[k-1], incremented once
|
||||
* per genuine apply of committed index k. Returns 1 when, for every node and every
|
||||
* index k, apply_reaction_count[k-1] <= 1; 0 on the first index whose reaction
|
||||
* fired more than once. Pure read-only over c.
|
||||
*/
|
||||
int raft_sim_check_idempotent_reapply(const raft_sim_cluster_t *c);
|
||||
|
||||
/*
|
||||
* raft_sim_invariant_t: identifier for the four Raft safety invariants checked by
|
||||
* the conformance harness, in the FIXED evaluation order used by
|
||||
|
|
@ -952,6 +1011,20 @@ void raft_sim_export_trace(const raft_sim_cluster_t *c, FILE *fp);
|
|||
*/
|
||||
void raft_sim_advance_commit(raft_sim_cluster_t *c, int node_idx);
|
||||
|
||||
/*
|
||||
* raft_sim_replay_commit (raft-apply-decoupling task 7.1; Req 10.3): re-invoke
|
||||
* node_idx's OWN registered on_commit_advanced callback with its CURRENT
|
||||
* commit_index — simulating a replay or retry of an already-processed commit
|
||||
* notification. Bypasses the raft library's own fire-once-per-strict-advance
|
||||
* contract (raft_advance_commit never re-fires on a held index) so the callback's
|
||||
* OWN idempotency can be exercised directly, independent of whether the library
|
||||
* would ever naturally re-invoke it. Works for clusters from either
|
||||
* raft_sim_init or raft_sim_conformance_init, since both wire on_commit_advanced
|
||||
* through node->cbs. No-op if node_idx is out of range or the node has no
|
||||
* registered callback.
|
||||
*/
|
||||
void raft_sim_replay_commit(raft_sim_cluster_t *c, int node_idx);
|
||||
|
||||
/*
|
||||
* raft_sim_restart_node: model a process restart of node_idx (Req 8.3).
|
||||
*
|
||||
|
|
|
|||
|
|
@ -37,6 +37,11 @@ static void sim_noop_start_election_after_delay(void *ctx, uint32_t delay_ms)
|
|||
|
||||
/* ---------- wired callbacks ---------- */
|
||||
|
||||
/* Forward declaration: sim_conf_decode_value is defined further below (conformance
|
||||
* harness adapter section) but sim_persist_entry's RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT
|
||||
* fault branch (raft-apply-decoupling task 7.1) needs it earlier in the file. */
|
||||
static uint64_t sim_conf_decode_value(const raft_sim_entry_t *slot);
|
||||
|
||||
/*
|
||||
* sim_send_vote_request: buffer a pending RPC into the cluster queue.
|
||||
*
|
||||
|
|
@ -125,6 +130,15 @@ static int sim_persist_entry(void *ctx, uint64_t index, uint64_t term,
|
|||
memcpy(slot->bytes, entry, n);
|
||||
slot->len = n;
|
||||
|
||||
/* Deliberately-broken-build fault switch (raft-apply-decoupling task 7.1): when
|
||||
* RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT is set, eagerly decode and write this
|
||||
* entry's value into applied[] right here at persist time — BEFORE it has ever
|
||||
* committed — emulating an apply-before-commit regression (the defect class
|
||||
* raft-apply-decoupling fixes). Purely additive — DEFAULT OFF, so legacy and
|
||||
* conformance runs that never set the flag are byte-identical to before. */
|
||||
if (c && (c->fault_inject & RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT))
|
||||
node->applied[index - 1] = sim_conf_decode_value(slot);
|
||||
|
||||
if ((int)index > node->log_count)
|
||||
node->log_count = (int)index;
|
||||
|
||||
|
|
@ -314,12 +328,29 @@ static uint64_t sim_conf_decode_value(const raft_sim_entry_t *slot)
|
|||
static void sim_conf_on_commit_advanced(void *ctx, uint64_t commit_index)
|
||||
{
|
||||
raft_sim_node_t *node = (raft_sim_node_t *)ctx;
|
||||
raft_sim_cluster_t *c = (raft_sim_cluster_t *)node->ctx;
|
||||
|
||||
/* Deliberately-broken-build fault switch (raft-apply-decoupling task 7.1): when
|
||||
* RAFT_SIM_BREAK_REAPPLY_GUARD is set, widen the apply range to the FULL
|
||||
* committed prefix (1..commit_index) on EVERY call instead of only the
|
||||
* newly-committed suffix (prev+1..commit_index) — emulating a regression where
|
||||
* a retried/duplicate commit notification re-applies entries that were already
|
||||
* applied. Purely additive — DEFAULT OFF, so a normal call (including a forced
|
||||
* raft_sim_replay_commit retry) remains exactly as before: a no-op when
|
||||
* commit_index has not strictly advanced. */
|
||||
int reapply_guard_broken = c && (c->fault_inject & RAFT_SIM_BREAK_REAPPLY_GUARD);
|
||||
|
||||
uint64_t prev = node->commit_index;
|
||||
if (commit_index > prev) {
|
||||
for (uint64_t k = prev + 1; k <= commit_index; k++) {
|
||||
if (k >= 1 && k <= (uint64_t)node->log_count && k <= RAFT_SIM_MAX_LOG)
|
||||
if (commit_index > prev || reapply_guard_broken) {
|
||||
uint64_t from = reapply_guard_broken ? 1 : prev + 1;
|
||||
for (uint64_t k = from; k <= commit_index; k++) {
|
||||
if (k >= 1 && k <= (uint64_t)node->log_count && k <= RAFT_SIM_MAX_LOG) {
|
||||
node->applied[k - 1] = sim_conf_decode_value(&node->log[k - 1]);
|
||||
/* Reaction-fire mirror (Req 10.3): counts genuine applies of
|
||||
* committed index k, so raft_sim_check_idempotent_reapply can
|
||||
* assert it never exceeds 1. */
|
||||
node->apply_reaction_count[k - 1]++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -797,6 +828,22 @@ void raft_sim_advance_commit(raft_sim_cluster_t *c, int node_idx)
|
|||
(void)raft_advance_commit(&c->nodes[node_idx].state);
|
||||
}
|
||||
|
||||
/*
|
||||
* raft_sim_replay_commit (raft-apply-decoupling task 7.1; Req 10.3): re-invoke
|
||||
* node_idx's own registered on_commit_advanced callback with its CURRENT
|
||||
* commit_index, simulating a replay/retry of an already-processed commit
|
||||
* notification. See raft.h for the full contract.
|
||||
*/
|
||||
void raft_sim_replay_commit(raft_sim_cluster_t *c, int node_idx)
|
||||
{
|
||||
if (!c || node_idx < 0 || node_idx >= c->n)
|
||||
return;
|
||||
|
||||
raft_sim_node_t *node = &c->nodes[node_idx];
|
||||
if (node->cbs.on_commit_advanced)
|
||||
node->cbs.on_commit_advanced(node, node->state.commit_index);
|
||||
}
|
||||
|
||||
/* Two log entries are identical iff their term, length, and payload bytes
|
||||
* (over the recorded length) all match. */
|
||||
static int sim_entry_identical(const raft_sim_entry_t *a,
|
||||
|
|
@ -929,6 +976,45 @@ int raft_sim_check_state_machine_safety(const raft_sim_cluster_t *c)
|
|||
return 1; /* holds */
|
||||
}
|
||||
|
||||
int raft_sim_check_apply_after_commit(const raft_sim_cluster_t *c)
|
||||
{
|
||||
/* Additive check (raft-apply-decoupling task 7.1, Req 10.1) — NOT one of the
|
||||
* four raft.tla invariants and NOT evaluated by raft_sim_check_all; see raft.h.
|
||||
* applied[k-1] != 0 is treated as "index k has been applied" (the same
|
||||
* zero-means-absent convention this file already uses for log-entry terms);
|
||||
* every value this harness ever applies is non-zero in practice. */
|
||||
for (int i = 0; i < c->n; i++) {
|
||||
const raft_sim_node_t *node = &c->nodes[i];
|
||||
int limit = node->log_count;
|
||||
if (limit > RAFT_SIM_MAX_LOG)
|
||||
limit = RAFT_SIM_MAX_LOG;
|
||||
|
||||
for (int k = 1; k <= limit; k++) {
|
||||
if (node->applied[k - 1] != 0 && (uint64_t)k > node->commit_index)
|
||||
return 0; /* applied ahead of commit — Req 10.1 violated */
|
||||
}
|
||||
}
|
||||
return 1; /* holds */
|
||||
}
|
||||
|
||||
int raft_sim_check_idempotent_reapply(const raft_sim_cluster_t *c)
|
||||
{
|
||||
/* Additive check (raft-apply-decoupling task 7.1, Req 10.3) — NOT one of the
|
||||
* four raft.tla invariants and NOT evaluated by raft_sim_check_all; see raft.h. */
|
||||
for (int i = 0; i < c->n; i++) {
|
||||
const raft_sim_node_t *node = &c->nodes[i];
|
||||
int limit = node->log_count;
|
||||
if (limit > RAFT_SIM_MAX_LOG)
|
||||
limit = RAFT_SIM_MAX_LOG;
|
||||
|
||||
for (int k = 1; k <= limit; k++) {
|
||||
if (node->apply_reaction_count[k - 1] > 1)
|
||||
return 0; /* re-applied — Req 10.3 violated */
|
||||
}
|
||||
}
|
||||
return 1; /* holds */
|
||||
}
|
||||
|
||||
raft_sim_invariant_t raft_sim_check_all(const raft_sim_cluster_t *c)
|
||||
{
|
||||
/* Evaluate the four invariants in the FIXED order required by Req 3.5 and
|
||||
|
|
|
|||
|
|
@ -8832,6 +8832,184 @@ static void test_sim_7_2_negative_both_directions(void)
|
|||
* (broken_rc < 0, Log Matching) AND the correct build passes (correct_rc == 0). */
|
||||
}
|
||||
|
||||
/*
|
||||
* test_sim_apply_decoupling_apply_after_commit_ordering
|
||||
* (raft-apply-decoupling task 7.1; Req 10.1)
|
||||
*
|
||||
* New conformance-harness assertion, layered on the existing apply-after-
|
||||
* on_commit_advanced loop: no entry's state mutation/reaction (the harness's apply
|
||||
* model, node->applied[]) may occur before that entry commits (node->commit_index
|
||||
* reaching its index). Proven in BOTH directions within one test, mirroring the
|
||||
* task 7.1/7.2 fault-injection idiom used above for the four raft.tla invariants:
|
||||
*
|
||||
* (1) CORRECT (fault OFF, the shipped sim_conf_on_commit_advanced adapter):
|
||||
* a persisted-but-not-yet-committed entry is NOT applied; once replicated
|
||||
* to a quorum and committed, it IS applied — raft_sim_check_apply_after_commit
|
||||
* holds throughout.
|
||||
* (2) BROKEN (RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT set): the SAME persist step
|
||||
* eagerly marks the entry applied while commit_index is still 0 — the SAME
|
||||
* check now FAILS, proving the assertion has real teeth, not a vacuous pass.
|
||||
*/
|
||||
static void test_sim_apply_decoupling_apply_after_commit_ordering(void)
|
||||
{
|
||||
/* ---- (1) CORRECT: fault OFF — apply never precedes commit ---- */
|
||||
{
|
||||
raft_sim_cluster_t c;
|
||||
memset(&c, 0, sizeof(c));
|
||||
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 0xA0715ULL));
|
||||
|
||||
raft_start_election(&c.nodes[0].state);
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[0].state.role);
|
||||
|
||||
uint64_t value = 0x99AAULL;
|
||||
uint64_t out_index = 0;
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_log_append(&c.nodes[0].state, &value, sizeof(value), &out_index));
|
||||
TEST_ASSERT_EQUAL_UINT64(1, out_index);
|
||||
|
||||
/* Persisted but not yet replicated/committed: nothing applied yet, and the
|
||||
* ordering check holds (vacuously — nothing to violate it with). */
|
||||
TEST_ASSERT_EQUAL_UINT64(0, c.nodes[0].applied[0]);
|
||||
TEST_ASSERT_EQUAL_UINT64(0, c.nodes[0].commit_index);
|
||||
TEST_ASSERT_EQUAL_INT(1, raft_sim_check_apply_after_commit(&c));
|
||||
|
||||
/* Replicate to one follower (2 of 3 = quorum) and commit. */
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_send_append_entries(&c.nodes[0].state, 1, 0));
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
raft_sim_advance_commit(&c, 0);
|
||||
TEST_ASSERT_EQUAL_UINT64(1, c.nodes[0].commit_index);
|
||||
|
||||
/* Now applied, and ordering still holds. */
|
||||
TEST_ASSERT_EQUAL_UINT64(value, c.nodes[0].applied[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, raft_sim_check_apply_after_commit(&c));
|
||||
}
|
||||
|
||||
/* ---- (2) BROKEN: RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT — persisted-but-uncommitted
|
||||
* entry is nonetheless marked applied; the SAME check now fails. ---- */
|
||||
{
|
||||
raft_sim_cluster_t c;
|
||||
memset(&c, 0, sizeof(c));
|
||||
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 0xA0715ULL));
|
||||
raft_sim_set_fault_inject(&c, RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT);
|
||||
TEST_ASSERT_EQUAL_UINT(RAFT_SIM_BREAK_APPLY_BEFORE_COMMIT, c.fault_inject);
|
||||
|
||||
raft_start_election(&c.nodes[0].state);
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[0].state.role);
|
||||
|
||||
uint64_t value = 0x99AAULL;
|
||||
uint64_t out_index = 0;
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_log_append(&c.nodes[0].state, &value, sizeof(value), &out_index));
|
||||
TEST_ASSERT_EQUAL_UINT64(1, out_index);
|
||||
|
||||
/* Never replicated, never committed — commit_index is still 0 ... */
|
||||
TEST_ASSERT_EQUAL_UINT64(0, c.nodes[0].commit_index);
|
||||
/* ... yet the corrupted adapter already marked it applied. */
|
||||
TEST_ASSERT_EQUAL_UINT64(value, c.nodes[0].applied[0]);
|
||||
|
||||
/* The oracle has teeth: this deliberately-broken build FAILS the SAME check
|
||||
* that passed throughout the correct-code block above. */
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, raft_sim_check_apply_after_commit(&c),
|
||||
"BROKEN config (apply-before-commit) did not trip the ordering check — "
|
||||
"the assertion lacks teeth");
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* test_sim_apply_decoupling_idempotent_reapply
|
||||
* (raft-apply-decoupling task 7.1; Req 10.3)
|
||||
*
|
||||
* New conformance-harness assertion: re-applying an already-committed entry
|
||||
* (whether a genuine duplicate on_commit_advanced fire or a forced
|
||||
* raft_sim_replay_commit retry) is idempotent — its apply-time reaction fires
|
||||
* exactly once per index, never twice. Proven in BOTH directions:
|
||||
*
|
||||
* (1) CORRECT (fault OFF): commit one entry, then force a REPLAY of the SAME
|
||||
* commit event via raft_sim_replay_commit — applied[]/commit_index stay
|
||||
* byte-identical and raft_sim_check_idempotent_reapply holds.
|
||||
* (2) BROKEN (RAFT_SIM_BREAK_REAPPLY_GUARD set): the SAME replay re-executes
|
||||
* the apply-time reaction a second time — the SAME check now FAILS.
|
||||
*/
|
||||
static void test_sim_apply_decoupling_idempotent_reapply(void)
|
||||
{
|
||||
/* ---- (1) CORRECT: fault OFF — replay of an already-applied commit is a no-op ---- */
|
||||
{
|
||||
raft_sim_cluster_t c;
|
||||
memset(&c, 0, sizeof(c));
|
||||
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 0xB0715ULL));
|
||||
|
||||
raft_start_election(&c.nodes[0].state);
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[0].state.role);
|
||||
|
||||
uint64_t value = 0x77BBULL;
|
||||
uint64_t out_index = 0;
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_log_append(&c.nodes[0].state, &value, sizeof(value), &out_index));
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_send_append_entries(&c.nodes[0].state, 1, 0));
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
raft_sim_advance_commit(&c, 0);
|
||||
TEST_ASSERT_EQUAL_UINT64(1, c.nodes[0].commit_index);
|
||||
TEST_ASSERT_EQUAL_UINT64(value, c.nodes[0].applied[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, c.nodes[0].apply_reaction_count[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, raft_sim_check_idempotent_reapply(&c));
|
||||
|
||||
/* Force a REPLAY of the already-processed commit (simulated retry/duplicate
|
||||
* notification), bypassing the library's own fire-once gate. */
|
||||
raft_sim_replay_commit(&c, 0);
|
||||
|
||||
/* No residual mutation: applied[]/commit_index unchanged, reaction still 1. */
|
||||
TEST_ASSERT_EQUAL_UINT64(1, c.nodes[0].commit_index);
|
||||
TEST_ASSERT_EQUAL_UINT64(value, c.nodes[0].applied[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, c.nodes[0].apply_reaction_count[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, raft_sim_check_idempotent_reapply(&c));
|
||||
}
|
||||
|
||||
/* ---- (2) BROKEN: RAFT_SIM_BREAK_REAPPLY_GUARD — replay re-fires the reaction ---- */
|
||||
{
|
||||
raft_sim_cluster_t c;
|
||||
memset(&c, 0, sizeof(c));
|
||||
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 0xB0715ULL));
|
||||
|
||||
raft_start_election(&c.nodes[0].state);
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_LEADER, c.nodes[0].state.role);
|
||||
|
||||
uint64_t value = 0x77BBULL;
|
||||
uint64_t out_index = 0;
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_log_append(&c.nodes[0].state, &value, sizeof(value), &out_index));
|
||||
TEST_ASSERT_EQUAL_INT(RAFT_OK,
|
||||
raft_send_append_entries(&c.nodes[0].state, 1, 0));
|
||||
for (int g = 0; g < 64 && c.bag_count > 0; g++)
|
||||
raft_sim_bag_deliver(&c, 0);
|
||||
raft_sim_advance_commit(&c, 0);
|
||||
TEST_ASSERT_EQUAL_UINT64(1, c.nodes[0].commit_index);
|
||||
TEST_ASSERT_EQUAL_INT(1, c.nodes[0].apply_reaction_count[0]);
|
||||
TEST_ASSERT_EQUAL_INT(1, raft_sim_check_idempotent_reapply(&c));
|
||||
|
||||
/* NOW arm the fault and replay — the corrupted adapter re-fires the reaction. */
|
||||
raft_sim_set_fault_inject(&c, RAFT_SIM_BREAK_REAPPLY_GUARD);
|
||||
TEST_ASSERT_EQUAL_UINT(RAFT_SIM_BREAK_REAPPLY_GUARD, c.fault_inject);
|
||||
raft_sim_replay_commit(&c, 0);
|
||||
|
||||
TEST_ASSERT_EQUAL_INT(2, c.nodes[0].apply_reaction_count[0]);
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, raft_sim_check_idempotent_reapply(&c),
|
||||
"BROKEN config (reapply guard disabled) did not trip the idempotency "
|
||||
"check — the assertion lacks teeth");
|
||||
}
|
||||
}
|
||||
|
||||
#define RAFT_SIM_SWEEP_M 32 /* configured minimum number of randomized seeds (R6.2) */
|
||||
#define RAFT_SIM_SWEEP_STEPS 400 /* per-run step cap — bounded for the suite budget (R6.4) */
|
||||
|
||||
|
|
@ -9129,6 +9307,8 @@ void test_raft(void)
|
|||
RUN_TEST(test_sim_5_5_stale_rejoin);
|
||||
RUN_TEST(test_sim_7_1_fault_switch_mechanism);
|
||||
RUN_TEST(test_sim_7_2_negative_both_directions);
|
||||
RUN_TEST(test_sim_apply_decoupling_apply_after_commit_ordering);
|
||||
RUN_TEST(test_sim_apply_decoupling_idempotent_reapply);
|
||||
RUN_TEST(test_sim_6_1_seed_sweep);
|
||||
|
||||
/* Group A: basic election correctness (task 5.1) */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue