forked from lei.chen/myxce
test(ha-commit-engine): full regression green + legacy-path audit (task 6.1)
Fixed the 3 bug-411 baseline failures honestly: stale timeout expectation updated to the reject-on-timeout contract (engine-wired fixture, killing power retained); two healthy-primary gate fixtures populated with standby_endpoints (gate logic untouched). Census: 46 groups / 467 tests / 0 failures standalone under default ASAN. Audit (reviewer-confirmed, recorded in tasks.md): no production walAck path reaches the deprecated acked_lsn>0 counting or n_active==0 short-circuit except the documented LSN-0 first entry. Leak inventory for 6 groups documented as follow-up.
This commit is contained in:
parent
f0d6074d4e
commit
95a10cedf3
6 changed files with 785 additions and 295 deletions
|
|
@ -73,7 +73,7 @@
|
|||
- _Requirements: 1.1, 1.2, 1.3, 2.1, 4.4, 6.3_
|
||||
|
||||
- [ ] 6. Validation: full regression and dishonest-path audit
|
||||
- [ ] 6.1 Run the full controller regression and audit the legacy paths
|
||||
- [x] 6.1 Run the full controller regression and audit the legacy paths
|
||||
- Build and run the complete controller testsuite with ASAN (`/xce-build controller`); fix any regression introduced by the short-circuit removal or the `committed_lsn` ownership move (existing persist/repl/ha suites)
|
||||
- Audit: grep confirms no production (engine-wired) walAck path reaches the `acked_lsn > 0` counting or the `n_active == 0 → success` branch; both survive only under the deprecated `ce == NULL` test fallback
|
||||
- Observable completion: full testsuite green under ASAN; audit results recorded in the task completion notes
|
||||
|
|
@ -98,3 +98,4 @@
|
|||
- (5.2) Remaining RED going into 5.3: test 33 (Expected -110 Was 0 — persist passes entry_lsn=0 → legacy fallback) and test 34 (Expected 0 Was -19 — -ENODEV special case). These plus test 32's honest-delegation green are 5.3's completion evidence. [RESOLVED at 5.3: both green]
|
||||
- (5.3) For 6.1 audit: (a) baseline failure #1 (test_committed_lsn_does_not_advance_on_timeout) PHYSICALLY lives in test_p2_integration.c:277 — Unity misattributes it across groups (bug-411 artifact); its root cause is a stale expectation (old rc==0+sync_rc contract). (b) Legacy cluster_init() wires the engine with n_peers==0 (self-quorum) — confirmed test-only (zero production callers; production = myapp_run two-phase), but ce->n_peers is never re-derived if ha->n_peers is reassigned post-init (ha.c:3484 RDHUP re-election) — latent gap for future dynamic membership; consider an assert or re-derivation at the first-boot election callback. (c) Pre-existing teardown race (not 5.3's): persist_fini runs before vtaskmgr_fini in cluster_fini.
|
||||
- (5.3) cluster_t lives in src/include/xce/cluster_def.h (not cluster.h); engine embedded there. Init point: cluster_init_phase2 after vtaskmgr guard, idempotence-guarded by c->commit.p==NULL (phase2 can run twice). n_vcpus = c->vtaskmgr.n_vcpus (resolved).
|
||||
- (6.1) AUDIT RECORD (reviewer-confirmed): delegation guard repl.c:435-437 (is_wal_ack && ce && entry_lsn!=0 → commit_submit); legacy acked_lsn>0 counting (repl.c:469) and n_active==0→success (repl.c:482-484) strictly below it, DEPRECATED-marked, walAck-reachable only when ce==NULL || entry_lsn==0; persist-side mirror guard persist.c:1037 with DEPRECATED legacy branch (1044-1064); production always wires ce at cluster_init_phase2 (cluster.c:1134-1148) — sole production legacy exposure = documented LSN-0 first entry (once per cluster lifetime; eliminated by ha-single-leader-role no-op); configAck never delegates (is_wal_ack false; barrier-rollout hardens that track). Census: 46 groups / 467 tests / 0 failures (standalone default-ASAN per group; meson harness uses pre-existing detect_leaks=0 from commit 8468a83). Baseline fixes: stale timeout expectation → reject-on-timeout contract (engine-wired fixture); 2 gate fixtures honestly populated with standby_endpoints. LEAK FOLLOW-UP (out of spec scope): 6 groups leak production-side allocations under default ASAN — worker_join 33,648B/357; rdhup_integration 1,776B/24 (ha.c:3457 elect_ctx deliberate, ha.c:2293/2299 delaymgr objects); failover 232B/2; start_election_async 272B/1; liveness 216B/5; connection_reuse 48B/2. Stale comment persist.c:1151-1154 (pre-engine contract text) — one-line follow-up.
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -953,3 +953,11 @@
|
|||
| 00:25 | Adversarial review of ha-commit-engine task 5.3 (init wiring + sync-append switch): APPROVED — full census exactly 3 baseline fails, RED 32/34→34/34 verified via temp worktree at 6600fd5 | cluster.c, persist.c, cluster_def.h, test_p2_integration.c | approved | ~45k |
|
||||
| 17:15 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~20 |
|
||||
| 17:16 | Edited .kiro/specs/ha-commit-engine/tasks.md | modified race() | ~342 |
|
||||
| 17:23 | Edited src/controller/testsuite/test_p2_integration.c | modified test_committed_lsn_does_not_advance_on_timeout() | ~858 |
|
||||
| 17:23 | Edited src/controller/testsuite/test_ha_repl_ack.c | modified test_repl_ack_after_promote_gate_allows_jobsub() | ~446 |
|
||||
| 17:23 | Edited src/controller/testsuite/test_ha_rdhup_integration.c | modified test_rdhup_primary_dispatch_gate_opens() | ~404 |
|
||||
| 17:24 | Edited src/controller/testsuite/test_ha_rdhup_integration.c | 7→10 lines | ~112 |
|
||||
| 17:24 | Edited src/controller/testsuite/test_ha_repl_ack.c | 4→5 lines | ~27 |
|
||||
| 00:36 | task 6.1 validation: full controller suite 46 groups / 467 tests 0 failures; 3 bug-411 baseline tests fixed (test files only); legacy-path audit clean; 6 pre-existing leak groups inventoried | test_p2_integration.c, test_ha_repl_ack.c, test_ha_rdhup_integration.c | success | ~60k |
|
||||
| 17:51 | Edited .kiro/specs/ha-commit-engine/tasks.md | inline fix | ~19 |
|
||||
| 17:51 | Edited .kiro/specs/ha-commit-engine/tasks.md | modified RECORD() | ~433 |
|
||||
|
|
|
|||
|
|
@ -386,6 +386,8 @@ static void test_rdhup_committing_fires_replication_hooks(void)
|
|||
* ═════════════════════════════════════════════════════════════════════════ */
|
||||
static void test_rdhup_primary_dispatch_gate_opens(void)
|
||||
{
|
||||
static int dummy_ep; /* stands in for the ACKing standby's endpoint */
|
||||
|
||||
install_stubs();
|
||||
|
||||
cluster_t c;
|
||||
|
|
@ -397,6 +399,19 @@ static void test_rdhup_primary_dispatch_gate_opens(void)
|
|||
c.ha.phase2_done = 1;
|
||||
c.ha.n_peers = 1;
|
||||
|
||||
/* FIXTURE FIX (ha-commit-engine task 6.1, bug-411 baseline failure):
|
||||
* ha_on_primary_ctrl_fd_rdhup re-derives ha->n_peers from
|
||||
* myconf->peers_cfg.candidates_count (= 2 here), so the CTRL_PRIMARY
|
||||
* dispatch gate requires write quorum: >= 1 connected standby. The
|
||||
* ack below models a standby that connected and ACKed; in production
|
||||
* it lives in ha->standby_endpoints (raft_cb_become_leader /
|
||||
* on_standby_connected). Register it so quorum legitimately holds on
|
||||
* this healthy primary — the gate itself must NOT be weakened. */
|
||||
c.ha.standby_endpoints = calloc(1, sizeof(zarray_t));
|
||||
TEST_ASSERT_NOT_NULL(c.ha.standby_endpoints);
|
||||
zarray_init(c.ha.standby_endpoints, 4, NULL);
|
||||
zarray_store(c.ha.standby_endpoints, &dummy_ep);
|
||||
|
||||
g_rdhup_state.c = &c;
|
||||
ha_rdhup_election_hook = rdhup_election_drives_evaluate;
|
||||
|
||||
|
|
@ -423,6 +438,9 @@ static void test_rdhup_primary_dispatch_gate_opens(void)
|
|||
"OPCode_jobdispatch must be allowed by dispatch gate in CTRL_PRIMARY (Req 5.3)");
|
||||
|
||||
clear_stubs();
|
||||
zarray_fini(c.ha.standby_endpoints);
|
||||
free(c.ha.standby_endpoints);
|
||||
c.ha.standby_endpoints = NULL;
|
||||
ha_fini(&c.ha);
|
||||
printf("test_rdhup_primary_dispatch_gate_opens: PASS (gate accepts job ops)\n");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
#define _GNU_SOURCE
|
||||
#include <stdatomic.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
|
||||
#include <unity/unity.h>
|
||||
|
|
@ -199,9 +200,23 @@ static void test_repl_ack_null_cluster_increments_count(void)
|
|||
|
||||
static void test_repl_ack_after_promote_gate_allows_jobsub(void)
|
||||
{
|
||||
cluster_t c;
|
||||
static int dummy_ep; /* stands in for the ACKing standby's endpoint */
|
||||
cluster_t c;
|
||||
|
||||
init_cluster_committing(&c, 2);
|
||||
|
||||
/* FIXTURE FIX (ha-commit-engine task 6.1, bug-411 baseline failure):
|
||||
* the CTRL_PRIMARY dispatch gate now requires write quorum — for
|
||||
* n_peers=2 that is >= 1 connected standby. The ack below models a
|
||||
* standby that connected and ACKed config; in production that standby
|
||||
* lives in ha->standby_endpoints (raft_cb_become_leader /
|
||||
* on_standby_connected). Register it so quorum legitimately holds on
|
||||
* this healthy primary — the gate itself must NOT be weakened. */
|
||||
c.ha.standby_endpoints = calloc(1, sizeof(zarray_t));
|
||||
TEST_ASSERT_NOT_NULL(c.ha.standby_endpoints);
|
||||
zarray_init(c.ha.standby_endpoints, 4, NULL);
|
||||
zarray_store(c.ha.standby_endpoints, &dummy_ep);
|
||||
|
||||
/* Trigger promotion via ack */
|
||||
ha_repl_ack_received(&c.ha, &c);
|
||||
|
||||
|
|
@ -215,6 +230,9 @@ static void test_repl_ack_after_promote_gate_allows_jobsub(void)
|
|||
TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc,
|
||||
"CTRL_PRIMARY dispatch gate must allow OPCode_jobsub (Req 8.3)");
|
||||
|
||||
zarray_fini(c.ha.standby_endpoints);
|
||||
free(c.ha.standby_endpoints);
|
||||
c.ha.standby_endpoints = NULL;
|
||||
ha_fini(&c.ha);
|
||||
printf("test_repl_ack_after_promote_gate_allows_jobsub: PASS\n");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -267,23 +267,41 @@ static void test_single_node_committed_lsn_advances(void)
|
|||
}
|
||||
|
||||
/* ------------------------------------------------------------------ */
|
||||
/* Test 8: Req 4.2 — committed_lsn must NOT advance when sync times out.
|
||||
/* Test 8: Req 4.2 (P2) / ha-commit-engine Req 2.1, 4.4 — committed_lsn
|
||||
* must NOT advance when sync times out, and the write is REJECTED.
|
||||
*
|
||||
* Set up persist with two active standbys that never ACK.
|
||||
* Use a very short sync_timeout_ms so the wait times out quickly.
|
||||
* Verify: persist_wal_sync_append returns 0 (WAL local success),
|
||||
* sync_rc == -ETIMEDOUT, and committed_lsn stays at 0.
|
||||
* UPDATED by ha-commit-engine task 6.1: this test previously pinned the
|
||||
* OLD accept-on-timeout contract (rc==0 with sync_rc==-ETIMEDOUT as a
|
||||
* mere advisory). persist_wal_sync_append now rejects on timeout
|
||||
* (returns -ETIMEDOUT) so no unconfirmed write reaches the caller as
|
||||
* success (Req 4.4). The fixture is also upgraded to the engine-wired
|
||||
* production path: cluster_init_phase2 always wires the commit engine,
|
||||
* so the honest place to pin "committed_lsn unmoved on timeout" is the
|
||||
* engine-owned path, not the deprecated ce==NULL legacy counting.
|
||||
*
|
||||
* Set up: 3-member cluster (self + two standbys that never ACK), engine
|
||||
* wired exactly as cluster_init_phase2 does. A priming async append
|
||||
* makes the sync entry land at LSN 1 (LSN 0 routes to the legacy branch
|
||||
* — the accepted once-per-cluster-lifetime interim behaviour).
|
||||
* Verify: rc == -ETIMEDOUT (write rejected), sync_rc == -ETIMEDOUT,
|
||||
* and committed_lsn stays at 0 (engine never advanced it).
|
||||
*/
|
||||
static void test_committed_lsn_does_not_advance_on_timeout(void)
|
||||
{
|
||||
commit_engine_t ce;
|
||||
|
||||
persist_t* p = make_p2_persist("clsn_to");
|
||||
TEST_ASSERT_NOT_NULL(p);
|
||||
|
||||
/* Add two standbys — they will never ACK, so wait times out */
|
||||
/* Add two standbys — they will never ACK, so the wait times out */
|
||||
static int ep_a, ep_b;
|
||||
repl_add_standby(&p->repl, (void *)&ep_a, "sb_a", 7010);
|
||||
repl_add_standby(&p->repl, (void *)&ep_b, "sb_b", 7011);
|
||||
|
||||
/* Wire the engine exactly as cluster_init_phase2 does for n_peers=3 */
|
||||
TEST_ASSERT_EQUAL_INT(0, commit_engine_init(&ce, p, 3, 8));
|
||||
repl_set_commit_engine(&p->repl, &ce);
|
||||
|
||||
/* Use a very short timeout so the test finishes quickly */
|
||||
p->config.sync_timeout_ms = 40;
|
||||
|
||||
|
|
@ -293,19 +311,24 @@ static void test_committed_lsn_does_not_advance_on_timeout(void)
|
|||
|
||||
/* Install drop hook so repl_broadcast doesn't dereference dummy eps */
|
||||
repl_broadcast_send_hook = p2_drop_send_hook;
|
||||
/* Priming append: sync entry below lands at LSN 1 (engine-wired path) */
|
||||
TEST_ASSERT_EQUAL_INT(0, persist_wal_async(p, WAL_JOB_UPDATE, "p", 1));
|
||||
int sync_rc = 0;
|
||||
int rc = persist_wal_sync_append(p, WAL_JOB_SUBMIT, "job", 3, &sync_rc);
|
||||
repl_broadcast_send_hook = NULL;
|
||||
|
||||
/* WAL write must succeed locally */
|
||||
TEST_ASSERT_EQUAL_INT(0, rc);
|
||||
/* Sync must report timeout (standbys registered but never ACK) */
|
||||
/* No per-entry majority within the deadline → the write is REJECTED
|
||||
* (ha-commit-engine Req 4.4: timeout propagates to the caller) */
|
||||
TEST_ASSERT_EQUAL_INT_MESSAGE(-ETIMEDOUT, rc,
|
||||
"sync append without majority ACK must be rejected with -ETIMEDOUT");
|
||||
TEST_ASSERT_EQUAL_INT(-ETIMEDOUT, sync_rc);
|
||||
/* committed_lsn must not have advanced (Req 4.2) */
|
||||
/* committed_lsn must not have advanced (Req 4.2; engine Req 2.1) */
|
||||
uint64_t committed_after =
|
||||
atomic_load_explicit(&p->committed_lsn, memory_order_acquire);
|
||||
TEST_ASSERT_EQUAL_UINT64(0, committed_after);
|
||||
TEST_ASSERT_EQUAL_UINT64_MESSAGE(0, committed_after,
|
||||
"committed_lsn must stay unmoved when the entry never reaches majority");
|
||||
|
||||
commit_engine_fini(&ce);
|
||||
teardown_p2_persist(p);
|
||||
printf("test_committed_lsn_does_not_advance_on_timeout: PASS\n");
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue