diff --git a/src/controller/ha.c b/src/controller/ha.c index 1b17687b..1f407057 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -187,8 +187,18 @@ static int raft_cb_send_vote_request(void *ctx, int peer_idx, uint64_t term) return 1; } - /* Track endpoint for promotion/close after election result */ - ectx->votes[votes_idx].ep = ep; + /* Track endpoint for promotion/close after election result. + * Also record the candidate pointer: raft_cb_become_leader uses + * votes[i].peer->hostname / ->port to register the surviving vote + * endpoint with the WAL replicator (repl_add_standby). Without this, + * the promotion registration block (guarded by votes[i].peer != NULL) + * is skipped — the standbys never enter c->persist.repl.standbys[], + * commit/quorum see zero standbys, and every write fails NOT_READY. + * `peer` points into c->myconf->peers_cfg.candidates[] (persistent; + * outlives the election — repl_add_standby copies host/port). + * (fix-promotion-standby-registration, Req 1.1) */ + ectx->votes[votes_idx].ep = ep; + ectx->votes[votes_idx].peer = peer; return 0; } @@ -3194,6 +3204,24 @@ void raft_cb_become_follower_with_cluster_for_test(cluster_t *c, uint64_t term) c->ha.raft_c = c; raft_cb_become_follower((void *)&c->ha, term, -1); } + +/* + * raft_cb_become_leader_with_cluster_for_test (fix-promotion-standby-registration): + * drive the leader promotion path with a LIVE cluster pointer so the + * cluster-dependent standby-registration branch (repl_add_standby for each + * surviving vote endpoint whose votes[i].peer is set) is exercised against a + * real persist/repl/commit/delaymgr fixture. Sets ha->raft_c = c so the + * become-leader callback sees a real cluster (unlike the NULL-cluster shim + * raft_cb_become_leader_for_test above, which suppresses the c-dependent + * registration block entirely). raft_cb_become_leader is file-static, so this + * seam must live here in ha.c. Symmetric with + * raft_cb_become_follower_with_cluster_for_test. + */ +void raft_cb_become_leader_with_cluster_for_test(cluster_t *c, uint64_t term) +{ + c->ha.raft_c = c; + raft_cb_become_leader((void *)&c->ha, term); +} #endif /* XCE_TESTING */ /* ── Task 2.4: vote_ep_err_handler ───────────────────────────────────────── diff --git a/src/controller/ha.h b/src/controller/ha.h index 10e2a824..4b621ad6 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -1008,6 +1008,19 @@ extern void (*ha_fatal_hook)(const char *msg); */ void raft_cb_become_follower_with_cluster_for_test(cluster_t *c, uint64_t term); +/* + * raft_cb_become_leader_with_cluster_for_test: drive the leader promotion path + * with a LIVE cluster pointer (fix-promotion-standby-registration). Unlike + * raft_cb_become_leader_for_test (which forces raft_c = NULL and so skips the + * c-dependent standby-registration branch entirely), this shim sets + * ha->raft_c = c first so the promotion-time repl_add_standby loop — which + * registers each surviving vote endpoint (votes[i].ep != NULL) carrying a + * peer pointer (votes[i].peer != NULL) into the WAL replicator — is exercised + * against a real persist/repl/commit fixture. Symmetric with + * raft_cb_become_follower_with_cluster_for_test. + */ +void raft_cb_become_leader_with_cluster_for_test(cluster_t *c, uint64_t term); + /* * ha_walack_continuation_for_test: test-only entry point for the ACK-driven * catch-up continuation logic (ha-convergence-recovery task 4.x, Req 5.1, 5.2, diff --git a/src/controller/testsuite/test_convergence_verification.c b/src/controller/testsuite/test_convergence_verification.c index c82678e0..1c61507d 100644 --- a/src/controller/testsuite/test_convergence_verification.c +++ b/src/controller/testsuite/test_convergence_verification.c @@ -1038,6 +1038,155 @@ static void test_pin_repl_complete_catchup_reincludes_in_broadcast(void) printf("test_pin_repl_complete_catchup_reincludes_in_broadcast: checked\n"); } +/* ── fix-promotion-standby-registration ───────────────────────────────────── + * + * Root cause pinned here: raft_cb_send_vote_request must set votes[i].peer + * (the candidate pointer) alongside votes[i].ep, so that the promotion path + * (raft_cb_become_leader) can call repl_add_standby for each surviving vote + * endpoint. Before the fix, votes[i].peer was NEVER set, so the promotion + * registration block (guarded by `votes[i].peer != NULL`) was skipped, the + * standbys were never added to c->persist.repl.standbys[], commit/quorum saw + * zero standbys, and every write failed with E_XCE_NOT_READY. + * + * These tests drive the REAL promotion path (raft_cb_become_leader via the + * with-cluster seam) over a real persist/repl/commit fixture, then assert the + * surviving vote endpoints are registered in the WAL replicator. + * + * The promotion tail is safe against this fixture: + * - ha_register_persist_hooks: documented safe on this cluster_t (ha.c). + * - ha_start_primary_heartbeat: no-ops because delaymgr is not started. + * - ha_arm_own_term_gate: uses the real persist + commit engine (broadcast + * drops through the stubbed repl_broadcast_send_hook). + */ + +/* Build a vote endpoint entry on the election ctx: set BOTH .ep and .peer + * (the post-fix vote-send contract). Uses g_cv_eps[] for the ep identity (so + * repl_active_standby_count / broadcast slot-matching stay consistent with the + * rest of this suite) and a caller-owned xce_candidate_t for .peer. */ +static void cv_set_vote(ha_elect_ctx_t *ctx, int idx, xce_candidate_t *peer, + const char *host, int port) +{ + snprintf(peer->hostname, sizeof(peer->hostname), "%s", host); + peer->port = port; + ctx->votes[idx].ep = (struct endpoint_t *)&g_cv_eps[idx]; + ctx->votes[idx].peer = peer; +} + +/* (fix) Promotion with surviving vote endpoints carrying .peer registers every + * one of them in the WAL replicator → repl_active_standby_count == n. + * + * RED before the fix: with the production vote-send leaving votes[i].peer NULL, + * an election ctx built the SAME way the real path builds it would have + * .peer == NULL, the promotion block would skip repl_add_standby, and the count + * would be 0. This test sets .peer (simulating the fixed vote-send) and pins + * that the promotion path then registers all surviving standbys. */ +static void test_promotion_registers_standbys_when_peer_set(void) +{ + /* n_standbys=0: the replicator starts EMPTY; only the promotion path may + * populate it, so the count is attributable solely to registration. */ + cv_setup("promreg", /*n_peers=*/2, /*n_standbys=*/0, + /*appended_lsn=*/0, /*n_vcpus=*/8); + + /* Precondition: nothing registered yet. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, + repl_active_standby_count(&g_cv.persist.repl), + "fixture precondition: replicator must start empty (n_standbys=0)"); + + /* Build an election ctx with 2 surviving vote endpoints, each carrying a + * peer pointer (the post-fix vote-send contract). */ + ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx) + 2 * sizeof(ctx->votes[0])); + TEST_ASSERT_NOT_NULL(ctx); + ctx->n_peers = 2; + pthread_mutex_init(&ctx->lock, NULL); + sem_init(&ctx->done, 0, 0); + + xce_candidate_t peer0 = {0}, peer1 = {0}; + cv_set_vote(ctx, 0, &peer0, "sb0.example", 7700); + cv_set_vote(ctx, 1, &peer1, "sb1.example", 7701); + + /* standby_endpoints array must exist (promotion stores eps there too). */ + g_cv.ha.standby_endpoints = calloc(1, sizeof(zarray_t)); + TEST_ASSERT_NOT_NULL(g_cv.ha.standby_endpoints); + zarray_init(g_cv.ha.standby_endpoints, 4, NULL); + + atomic_store_explicit(&g_cv.ha.elect_ctx, (void *)ctx, memory_order_release); + + /* Act: real promotion with a live cluster. */ + raft_cb_become_leader_with_cluster_for_test(&g_cv, /*term=*/1); + + /* Assert (Req 1.1): both surviving vote endpoints are now registered in the + * WAL replicator, so commit/quorum can advance committed_lsn. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(2, + repl_active_standby_count(&g_cv.persist.repl), + "promotion must register every surviving vote endpoint (votes[i].peer " + "set) in the WAL replicator — repl_active_standby_count must equal the " + "number of vote endpoints (fix-promotion-standby-registration Req 1.1)"); + + /* Cleanup */ + atomic_store_explicit(&g_cv.ha.elect_ctx, NULL, memory_order_release); + if (g_cv.ha.standby_endpoints) { + zarray_fini(g_cv.ha.standby_endpoints); + free(g_cv.ha.standby_endpoints); + g_cv.ha.standby_endpoints = NULL; + } + pthread_mutex_destroy(&ctx->lock); + sem_destroy(&ctx->done); + free(ctx); + cv_teardown(); + printf("test_promotion_registers_standbys_when_peer_set: checked\n"); +} + +/* RED-pin: with votes[i].peer == NULL (the PRE-fix vote-send behavior), the + * promotion block must NOT register the endpoints — the count stays 0. This + * pins the exact failure the fix addresses: the registration is conditional on + * .peer being set, so leaving it NULL (as the buggy vote-send did) reproduces + * the zero-standby / no-quorum / NOT_READY condition. GREEN: the production + * vote-send now always sets .peer, so the real path takes the registered + * branch above. */ +static void test_promotion_skips_registration_when_peer_null(void) +{ + cv_setup("promnull", /*n_peers=*/2, /*n_standbys=*/0, + /*appended_lsn=*/0, /*n_vcpus=*/8); + + ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx) + 2 * sizeof(ctx->votes[0])); + TEST_ASSERT_NOT_NULL(ctx); + ctx->n_peers = 2; + pthread_mutex_init(&ctx->lock, NULL); + sem_init(&ctx->done, 0, 0); + + /* eps present (election connected), but peer left NULL (the BUG). */ + ctx->votes[0].ep = (struct endpoint_t *)&g_cv_eps[0]; + ctx->votes[0].peer = NULL; + ctx->votes[1].ep = (struct endpoint_t *)&g_cv_eps[1]; + ctx->votes[1].peer = NULL; + + g_cv.ha.standby_endpoints = calloc(1, sizeof(zarray_t)); + TEST_ASSERT_NOT_NULL(g_cv.ha.standby_endpoints); + zarray_init(g_cv.ha.standby_endpoints, 4, NULL); + + atomic_store_explicit(&g_cv.ha.elect_ctx, (void *)ctx, memory_order_release); + + raft_cb_become_leader_with_cluster_for_test(&g_cv, /*term=*/1); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, + repl_active_standby_count(&g_cv.persist.repl), + "with votes[i].peer NULL (pre-fix vote-send), promotion must NOT register " + "standbys — this reproduces the zero-quorum / NOT_READY bug " + "(fix-promotion-standby-registration RED pin)"); + + atomic_store_explicit(&g_cv.ha.elect_ctx, NULL, memory_order_release); + if (g_cv.ha.standby_endpoints) { + zarray_fini(g_cv.ha.standby_endpoints); + free(g_cv.ha.standby_endpoints); + g_cv.ha.standby_endpoints = NULL; + } + pthread_mutex_destroy(&ctx->lock); + sem_destroy(&ctx->done); + free(ctx); + cv_teardown(); + printf("test_promotion_skips_registration_when_peer_null: checked\n"); +} + #endif /* XCE_TESTING */ /* ── Suite entry point ──────────────────────────────────────────────────── */ @@ -1068,6 +1217,12 @@ void test_convergence_verification(void) RUN_TEST(test_empty_standby_fresh_cluster_streams_not_snapshot); RUN_TEST(test_pin_dispatch_gate_degraded_allowlist); RUN_TEST(test_pin_repl_complete_catchup_reincludes_in_broadcast); + + printf("\n>> Promotion standby registration " + "(fix-promotion-standby-registration: votes[].peer → repl_add_standby)\n\n"); + + RUN_TEST(test_promotion_registers_standbys_when_peer_set); + RUN_TEST(test_promotion_skips_registration_when_peer_null); #else printf("test_convergence_verification: SKIP (non-test build)\n"); #endif