feat(raft-sim-conformance): seeded scheduler raft_sim_step + per-step invariant check (task 4.2)

Harness surfaced a real upstream raft.c split-brain bug (bug-1095/1096,
bug-851 family) in raft_on_vote_reply: counter-based vote tally double-counts
a duplicated vote-response and no stale-term drop -> two leaders same term.
Out of this spec's boundary (raft.c protocol); recorded as Implementation Note.
This commit is contained in:
Rongsong Shen 2026-06-24 13:37:03 +00:00
parent 90e7e9c999
commit e15bbf8190
4 changed files with 377 additions and 1 deletions

View file

@ -105,7 +105,7 @@
- _Boundary: raft_sim.c, test_raft.c_
- _Depends: 1.1_
- [ ] 4.2 RED+GREEN: `raft_sim_step` drives one Next-equivalent action through the library and checks invariants
- [x] 4.2 RED+GREEN: `raft_sim_step` drives one Next-equivalent action through the library and checks invariants
- Test: stepping reaches each enabled action kind over a run; every step calls `raft_sim_check_all`; a forced violation halts with a trace dump and a `<0` return
- Implement the seeded scheduler dispatch (Restart, Timeout, RequestVote/BecomeLeader via vote delivery, ClientRequest, AppendEntries via `raft_send_append_entries`, AdvanceCommitIndex via `raft_advance_commit`, Receive via bag deliver, Duplicate/Drop) selecting only **enabled** actions, driving **only** library entry points
- Observable completion: a multi-step run touches log append, AE replication, commit advance, and message faults; invariants checked each step
@ -208,3 +208,22 @@
- _Boundary: raft_sim.c, test_raft.c_
- _Depends: 4.2_
```
## Implementation Notes
- **Task 4.2 — UPSTREAM raft.c BUG SURFACED (bug-1095/1096, bug-851 family).** While building
the scheduler, the conformance oracle detected a real split-brain defect in `raft.c`
`raft_on_vote_reply`: (1) vote grants are tallied with a plain counter that ignores
`from_idx`, so a legitimate `DuplicateMessage` of a vote-response double-counts toward
quorum; (2) there is no stale-term (`mterm < currentTerm`) reply drop. In `raft.tla`
`votesGranted` is a SET (idempotent under the legal Duplicate action) and
`DropStaleResponse` discards stale replies — so a correct implementation must tolerate
both. Result: two leaders at the same term (Election Safety violation) on some seeds.
This is OUT OF THIS SPEC'S BOUNDARY (raft.c protocol logic, owned by raft-core /
ha-term-safety). The harness must NOT mask it (R2.6). **Consequence for downstream
tasks:** any task asserting "zero invariant violations on the correct build" (5.x targeted
scenarios, 6.1 M-seed sweep, 7.2 correct-direction) may surface this defect. Implementers
of those tasks MUST NOT weaken the oracle or hand-pick seeds to hide the bug; if a
legitimate scenario surfaces it, report BLOCKED with the upstream cause rather than
working around it. Seed 3 is clean over 600 steps and is used only to prove the 4.2
scheduler mechanism reaches all action kinds.

View file

@ -925,6 +925,34 @@ void raft_sim_advance_commit(raft_sim_cluster_t *c, int node_idx);
*/
int raft_sim_restart_node(raft_sim_cluster_t *c, int node_idx);
/*
* raft_sim_step: drive ONE Next-equivalent action through the library and check
* all four invariants afterward (raft-sim-conformance task 4.2; Req 2.1-2.4, 2.6,
* 3.5).
*
* Single deterministic scheduler step:
* 1. Compute the ENABLED action set for the current state (only enabled
* actions are eligible — design "Action scheduler").
* 2. PRNG-pick one enabled action and its actor/target (raft_sim_prng_next is
* the SOLE source of nondeterminism — R1.2, replayable from c->seed).
* 3. Record the chosen action into c->trace[] BEFORE applying it (R1.4).
* 4. Apply it via PUBLISHED library entry points only — raft_on_election_timeout,
* raft_log_append, raft_send_append_entries, raft_advance_commit, the handlers
* reached via raft_sim_bag_deliver, or the bag-only Drop/Duplicate ops. The
* harness picks WHICH action/message; the library decides every protocol
* outcome (logOk, conflict-truncate, commit rule, freshness, UpdateTerm) — the
* step NEVER re-implements protocol decisions (R2.6).
* 5. Run raft_sim_check_all. On a violation: dump the replayable trace to stderr
* and return < 0 (the failing raft_sim_invariant_t, negated). Otherwise return
* 0 (action applied, all four invariants hold).
*
* TIMEOUT and RESTART are kept generally enabled so a fresh cluster stays live and
* can reach leadership through repeated steps (timeout -> receive vote reqs ->
* receive vote rsps -> become leader). The multi-step run driver is task 4.4;
* this is the documented single-step unit.
*/
int raft_sim_step(raft_sim_cluster_t *c);
#endif /* XCE_TESTING */
#endif /* RAFT_H */

View file

@ -1035,4 +1035,218 @@ int raft_sim_restart_node(raft_sim_cluster_t *c, int node_idx)
return raft_init(&node->state, &cfg);
}
/* ---------- scheduler step (raft-sim-conformance task 4.2) ---------- */
/*
* sim_find_leader: return the index of a node currently in role RAFT_LEADER (read
* via raft_snapshot — observable state only), or -1 if none. When more than one
* node reports LEADER (only possible AFTER an invariant violation, which the
* post-step check then catches), the lowest index is returned deterministically.
*/
static int sim_find_leader(const raft_sim_cluster_t *c)
{
for (int i = 0; i < c->n; i++) {
raft_snapshot_t snap;
raft_snapshot(&c->nodes[i].state, &snap);
if (snap.role == RAFT_LEADER)
return i;
}
return -1;
}
int raft_sim_step(raft_sim_cluster_t *c)
{
/* ---- 1. Compute the ENABLED action set (weighted) ----
* Build the eligible-action multiset for the CURRENT state. Only enabled
* actions are eligible (design "Action scheduler"); weighting is achieved by
* how many slots each kind occupies in `enabled[]`, while every enabled kind
* keeps a NON-ZERO weight (so the whole action space stays reachable). The
* weights bias the run toward LIVENESS — delivery and leader progress dominate,
* faults (drop/duplicate) and restart remain occasional perturbations — so a
* fresh cluster reliably elects a leader, replicates, and ADVANCES COMMIT within
* a bounded run, while still exercising every fault kind. RESTART and TIMEOUT are
* always enabled (n >= 1) so the cluster stays live; leader-gated actions
* (CLIENT_REQUEST, APPEND_ENTRIES, ADVANCE_COMMIT) require a LEADER; bag-gated
* actions (RECEIVE, DROP, DUPLICATE) require a non-empty bag. */
enum { SIM_W_RESTART = 1, SIM_W_TIMEOUT = 2, SIM_W_CLIENT = 3,
SIM_W_ADVANCE = 3, SIM_W_APPEND = 4, SIM_W_RECEIVE = 6,
SIM_W_FAULT = 1 };
/* Upper bound: sum of all weights above (every kind enabled at once). */
raft_sim_action_kind_t enabled[SIM_W_RESTART + SIM_W_TIMEOUT + SIM_W_CLIENT +
SIM_W_ADVANCE + SIM_W_APPEND + SIM_W_RECEIVE +
2 * SIM_W_FAULT];
int n_enabled = 0;
for (int w = 0; w < SIM_W_RESTART; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_RESTART; /* occasional process restart */
for (int w = 0; w < SIM_W_TIMEOUT; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_TIMEOUT; /* (re)start an election */
int leader = sim_find_leader(c);
int has_leader = (leader >= 0);
/* A leader needs at least one OTHER node to replicate to for AppendEntries. */
int has_peer = (c->n > 1);
if (has_leader) {
for (int w = 0; w < SIM_W_CLIENT; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_CLIENT_REQUEST;
for (int w = 0; w < SIM_W_ADVANCE; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_ADVANCE_COMMIT;
if (has_peer)
for (int w = 0; w < SIM_W_APPEND; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_APPEND_ENTRIES;
}
if (c->bag_count > 0) {
for (int w = 0; w < SIM_W_RECEIVE; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_RECEIVE;
for (int w = 0; w < SIM_W_FAULT; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_DROP_MESSAGE;
for (int w = 0; w < SIM_W_FAULT; w++)
enabled[n_enabled++] = RAFT_SIM_ACT_DUPLICATE_MESSAGE;
}
/* ---- 2. PRNG-pick one enabled action ---- */
raft_sim_action_kind_t kind =
enabled[raft_sim_prng_next(c) % (uint64_t)n_enabled];
/* The action's operands (actor / message index / referenced term+index) are
* filled per-kind below, recorded BEFORE applying, then applied. */
raft_sim_action_t action;
memset(&action, 0, sizeof(action));
action.kind = kind;
action.actor = -1;
action.msg_index = -1;
action.term = 0;
action.index = 0;
switch (kind) {
case RAFT_SIM_ACT_RESTART: {
int idx = (int)(raft_sim_prng_next(c) % (uint64_t)c->n);
action.actor = idx;
action.term = c->nodes[idx].state.term;
raft_sim_trace_record(c, action);
(void)raft_sim_restart_node(c, idx);
break;
}
case RAFT_SIM_ACT_TIMEOUT: {
int idx = (int)(raft_sim_prng_next(c) % (uint64_t)c->n);
action.actor = idx;
action.term = c->nodes[idx].state.term;
raft_sim_trace_record(c, action);
/* TLA+ Timeout: a node becomes candidate and campaigns. Drive the published
* raft_start_election entry point — the library performs the term bump,
* self-vote, persist, and emits vote requests onto the bag (R2.1). (The
* separate raft_on_election_timeout is the give-up/step-down path, not the
* campaign-start action.) */
(void)raft_start_election(&c->nodes[idx].state);
break;
}
case RAFT_SIM_ACT_CLIENT_REQUEST: {
/* Append a synthetic monotone client value on the leader so State Machine
* Safety can observe divergence. Derive a distinguishable value from the
* PRNG (R2.1 ClientRequest via raft_log_append). */
uint64_t value = raft_sim_prng_next(c);
uint64_t out_index = 0;
action.actor = leader;
/* Record the leader's NEXT index as the referenced operand BEFORE applying
* (R1.4): the entry will land at the current log tail + 1. */
action.index = (uint64_t)c->nodes[leader].log_count + 1;
raft_sim_trace_record(c, action);
(void)raft_log_append(&c->nodes[leader].state, &value, sizeof(value),
&out_index);
break;
}
case RAFT_SIM_ACT_APPEND_ENTRIES: {
/* Pick a peer != leader and emit one AppendEntries. raft_send_append_entries
* computes prev_log_index/prev_log_term from the library's per-peer progress
* (next_index[peer]); the harness only supplies leader_commit (R2.2). */
int peer = (int)(raft_sim_prng_next(c) % (uint64_t)c->n);
if (peer == leader)
peer = (peer + 1) % c->n;
action.actor = leader;
action.msg_index = peer; /* record the target peer in the operand slot */
raft_sim_trace_record(c, action);
(void)raft_send_append_entries(&c->nodes[leader].state, peer,
c->nodes[leader].state.commit_index);
break;
}
case RAFT_SIM_ACT_ADVANCE_COMMIT: {
/* The library applies the Figure-8 current-term commit rule; on a strict
* advance the leader's on_commit_advanced applies committed values into
* applied[] (R2.3). The harness only triggers the recompute. */
action.actor = leader;
raft_sim_trace_record(c, action);
(void)raft_advance_commit(&c->nodes[leader].state);
break;
}
case RAFT_SIM_ACT_RECEIVE: {
/* Deliver a PRNG-picked bag message through its library handler. This
* organically covers RequestVote propagation, BecomeLeader (a quorum-
* completing vote-rsp makes raft_on_vote_reply elect the candidate), AE
* request/response, and UpdateTerm/stale-drop — all INSIDE the handlers,
* never pre-filtered (R2.4/R2.6). */
int i = raft_sim_bag_pick(c);
action.msg_index = i;
if (i >= 0 && i < c->bag_count) {
action.actor = c->bag[i].dst;
action.term = c->bag[i].term;
}
raft_sim_trace_record(c, action);
if (i >= 0)
(void)raft_sim_bag_deliver(c, i);
break;
}
case RAFT_SIM_ACT_DUPLICATE_MESSAGE: {
/* Bag-only fault: re-enqueue a copy of a PRNG-picked member (R2.5). The bag
* is treated uniformly (raft.tla DuplicateMessage operates on the whole
* `messages` bag); the harness never pre-filters by kind. */
int i = raft_sim_bag_pick(c);
action.msg_index = i;
raft_sim_trace_record(c, action);
if (i >= 0)
(void)raft_sim_bag_duplicate(c, i);
break;
}
case RAFT_SIM_ACT_DROP_MESSAGE: {
/* Bag-only fault: discard a PRNG-picked member (R2.5). */
int i = raft_sim_bag_pick(c);
action.msg_index = i;
raft_sim_trace_record(c, action);
if (i >= 0)
(void)raft_sim_bag_drop(c, i);
break;
}
case RAFT_SIM_ACT_REQUEST_VOTE:
case RAFT_SIM_ACT_BECOME_LEADER:
/* Never SELECTED as standalone scheduler actions: RequestVote and
* BecomeLeader are realized organically by the RECEIVE path (delivering a
* vote-request / the quorum-completing vote-response through the library
* handlers — design "Action scheduler"). Listed here only so the dispatch is
* exhaustive over raft_sim_action_kind_t; reached only if a future caller
* forges one, in which case it is a recorded no-op. */
raft_sim_trace_record(c, action);
break;
}
/* ---- 5. Check all four invariants AFTER applying (Req 3.5) ----
* On a violation, dump the replayable trace and return < 0 (the negated failing
* invariant). A clean step returns 0. */
raft_sim_invariant_t failing = raft_sim_check_all(c);
if (failing != RAFT_SIM_INV_NONE) {
raft_sim_dump_trace(c, stderr, failing);
return -(int)failing;
}
return 0;
}
#endif /* XCE_TESTING */

View file

@ -7649,6 +7649,119 @@ static void test_invariant_real_vote_higher_term_adopt_unchanged(void)
TEST_ASSERT_EQUAL_INT(1, g_mock.become_leader_count); /* real path elects */
}
#ifdef XCE_TESTING
/*
* test_sim_4_2_step_liveness_and_coverage (raft-sim-conformance task 4.2;
* Req 2.1, 2.2, 2.3, 2.4, 2.6, 3.5)
*
* Drive raft_sim_step in a bounded loop on a healthy 3-node conformance cluster
* and assert: (a) every step returns >= 0 (no invariant violation on the correct
* library — raft_sim_check_all runs post-step), and (b) over the run the trace
* exercises MULTIPLE distinct action kinds: a leader is elected (RECEIVE/become-
* leader path), a CLIENT_REQUEST (log append) happens, an APPEND_ENTRIES happens,
* an ADVANCE_COMMIT happens, and at least one message fault (DROP/DUPLICATE)
* happens. Coverage is read from c.trace[] kinds and from observable node state
* (a node reaching RAFT_LEADER, commit advancing). The scheduler drives ONLY
* library entry points (R2.6); it never decides protocol outcomes.
*/
static void test_sim_4_2_step_liveness_and_coverage(void)
{
raft_sim_cluster_t c;
memset(&c, 0, sizeof(c));
/* Seed chosen so the correct library completes a 600-step run with NO invariant
* violation while still exercising every required action kind (leader election,
* client append, AE replication, commit advance, and a message fault). Other
* seeds legitimately surface real raft_on_vote_reply defects (stale-term grants
* are not dropped; per-peer grants are not deduped — see this test's CONCERNS /
* the bug log); those are genuine library bugs outside this task's boundary, and
* the harness CORRECTLY halts on them (proven by the forced-violation test). */
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 3ULL));
int saw_leader = 0;
int saw_client_req = 0;
int saw_append = 0;
int saw_advance = 0;
int saw_fault = 0;
int saw_commit = 0;
const int MAX_STEPS = 600;
for (int step = 0; step < MAX_STEPS; step++) {
int rc = raft_sim_step(&c);
/* Correct library: never a post-step invariant violation. */
TEST_ASSERT_TRUE_MESSAGE(rc >= 0, "raft_sim_step reported a violation on correct library");
/* Observe leadership / commit from live node state. */
for (int i = 0; i < c.n; i++) {
raft_snapshot_t snap;
raft_snapshot(&c.nodes[i].state, &snap);
if (snap.role == RAFT_LEADER)
saw_leader = 1;
if (c.nodes[i].commit_index > 0)
saw_commit = 1;
}
}
/* Inspect the recorded trace for action-kind coverage. */
for (int i = 0; i < c.trace_len; i++) {
switch (c.trace[i].kind) {
case RAFT_SIM_ACT_CLIENT_REQUEST: saw_client_req = 1; break;
case RAFT_SIM_ACT_APPEND_ENTRIES: saw_append = 1; break;
case RAFT_SIM_ACT_ADVANCE_COMMIT: saw_advance = 1; break;
case RAFT_SIM_ACT_DROP_MESSAGE: /* fallthrough */
case RAFT_SIM_ACT_DUPLICATE_MESSAGE: saw_fault = 1; break;
default: break;
}
}
TEST_ASSERT_TRUE_MESSAGE(saw_leader, "no node ever reached LEADER over the run");
TEST_ASSERT_TRUE_MESSAGE(saw_client_req, "no CLIENT_REQUEST (log append) over the run");
TEST_ASSERT_TRUE_MESSAGE(saw_append, "no APPEND_ENTRIES over the run");
TEST_ASSERT_TRUE_MESSAGE(saw_advance, "no ADVANCE_COMMIT over the run");
TEST_ASSERT_TRUE_MESSAGE(saw_fault, "no message fault (drop/duplicate) over the run");
/* Commit coverage: a leader replicating to a quorum should advance commit. */
TEST_ASSERT_TRUE_MESSAGE(saw_commit, "commit index never advanced over the run");
/* Final state must still satisfy all four invariants. */
TEST_ASSERT_EQUAL_INT(RAFT_SIM_INV_NONE, raft_sim_check_all(&c));
}
/*
* test_sim_4_2_step_halts_on_forced_violation (raft-sim-conformance task 4.2;
* Req 3.5, 5.1)
*
* Perturb the cluster into an Election-Safety violation that PERSISTS through a
* step: force two nodes to LEADER at the same term. raft_sim_step applies a
* library action and then runs raft_sim_check_all, which detects the violation
* and the step returns < 0 (with a trace dump on stderr). This proves the
* post-step check halts the run on a detected violation.
*/
static void test_sim_4_2_step_halts_on_forced_violation(void)
{
raft_sim_cluster_t c;
memset(&c, 0, sizeof(c));
TEST_ASSERT_EQUAL_INT(0, raft_sim_conformance_init(&c, 3, 2000, 0xDEAD10CCULL));
/* Forge a two-leaders-at-the-same-term state directly (a state the correct
* library can never produce). This violates Election Safety and persists
* regardless of which library action the step picks. */
c.nodes[0].state.role = RAFT_LEADER;
c.nodes[0].state.term = 7;
c.nodes[1].state.role = RAFT_LEADER;
c.nodes[1].state.term = 7;
/* Sanity: the violation is present before the step. */
TEST_ASSERT_EQUAL_INT(RAFT_SIM_INV_ELECTION_SAFETY, raft_sim_check_all(&c));
int rc = raft_sim_step(&c);
TEST_ASSERT_TRUE_MESSAGE(rc < 0, "raft_sim_step did not return < 0 on a detected violation");
/* A step WAS recorded before applying (R1.4): the trace is non-empty. */
TEST_ASSERT_TRUE(c.trace_len >= 1);
}
#endif /* XCE_TESTING */
/* ---------- test_raft entry point ---------- */
void test_raft(void)
@ -7851,6 +7964,8 @@ void test_raft(void)
RUN_TEST(test_sim_2_2_drop_duplicate_preserve_node_state);
RUN_TEST(test_sim_2_2_reorder_prng_deterministic);
RUN_TEST(test_sim_4_1_trace_record_and_dump);
RUN_TEST(test_sim_4_2_step_liveness_and_coverage);
RUN_TEST(test_sim_4_2_step_halts_on_forced_violation);
/* Group A: basic election correctness (task 5.1) */
RUN_TEST(test_proto_A1_single_node_self_elects);