forked from lei.chen/myxce
fix(ha-single-leader-role,ha-repl-hardening): promotion must register standbys with the WAL replicator
Live symptom: every write (jobsub) on a healthy 3-node cluster returned E_XCE_NOT_READY (10013) — the primary had no write quorum and committed_lsn was stuck at 0, even though both standbys were connected, heartbeating, streamed, and the primary RECEIVED their WAL ACKs. Root cause: raft_cb_become_leader registers a surviving vote endpoint with repl_add_standby only when ectx->votes[i].peer != NULL, but raft_cb_send_vote_request set votes[i].ep and never votes[i].peer. So at promotion the guard failed, the standbys were added to ha->standby_endpoints (streaming) but NOT to the replicator array c->persist.repl.standbys[] (commit/quorum). Consequently repl_on_ack could not find the ack's ep (slot_idx=-1) -> commit_on_ack never ran -> committed_lsn never advanced; majority_candidate/repl_active_standby_count saw 0 standbys -> no write quorum -> the dispatch gate rejected every write with -XCE_ERR_NOT_READY. (Streaming/persistence worked, which masked it: reslist/workerlist succeed, only writes fail.) Fix: raft_cb_send_vote_request now also sets ectx->votes[votes_idx].peer = peer (the &candidates[peer_idx] pointer already computed for the connect) at the same success site as .ep. peer points into the persistent myconf candidates[]; repl_add_standby copies host/port, so no lifetime dependency. Promotion now registers each surviving vote ep -> standbys enter the replicator -> WAL ACKs advance committed_lsn -> own-term gate opens -> write quorum established. TDD: drive the real raft_cb_become_leader over a real persist/repl/commit fixture and assert repl_active_standby_count post-promotion == N when votes[].peer is set, == 0 when NULL (the bug). Full controller suite green under ASAN; election/commit/ repl/wire untouched apart from the one-line .peer assignment + a test seam.
This commit is contained in:
parent
4c5b27cb70
commit
c4fd3e1c8f
3 changed files with 198 additions and 2 deletions
|
|
@ -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 ─────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue