From f5b0ac0af3b720c9e418a70dc95f3b123c4b587c Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Thu, 7 May 2026 15:11:53 +0000 Subject: [PATCH] feat: ha_should_redirect + join.c HA redirect/nomaster guard (P1-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add redirect helpers to ha.c/ha.h so a standby controller never accepts join requests as if it were the primary: - ha_should_redirect(): returns 1 when role is STANDBY and primary_host/port are known — caller should send E_XCE_REDIRECT. - ha_no_primary_known(): returns 1 when role is STANDBY but primary address is not yet known — caller should send E_XCE_NOMASTER (retry later). - Add E_XCE_NOMASTER and E_XCE_REDIRECT to errnox.h. - Wire both guards into handle_join_() in join.c (before the role switch) so all join types (worker, scheduler, apiserver, client) are protected. - Full redirect response sends status=-E_XCE_REDIRECT with nodata body; the primary host:port are logged so operators can trace misrouted clients. Full Cap'n Proto RedirectInfo response body is deferred to a follow-up (schema union would require regenerating capnp bindings). 5 new TDD tests — 110 total, 0 failures. --- src/controller/ha.c | 32 ++++++ src/controller/ha.h | 17 +++ src/controller/join.c | 27 +++++ src/controller/meson.build | 1 + src/controller/testsuite/controller_test.c | 2 + src/controller/testsuite/test_ha_redirect.c | 110 ++++++++++++++++++++ src/include/xce/errnox.h | 2 + 7 files changed, 191 insertions(+) create mode 100644 src/controller/testsuite/test_ha_redirect.c diff --git a/src/controller/ha.c b/src/controller/ha.c index 3ce90e52..f4da627a 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -186,6 +186,38 @@ void ha_fini(ha_state_t *ha) pthread_mutex_destroy(&ha->lock); } +/* ── P1-6: Redirect helpers ────────────────────────────────── */ + +/* + * ha_should_redirect: returns 1 when this standby knows the primary's + * address and should redirect incoming join requests there. + * Returns 0 for PRIMARY role, or for STANDBY with no known primary yet. + */ +int ha_should_redirect(const ha_state_t *ha) +{ + ctrl_role_t role = + (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); + if (role == CTRL_PRIMARY) + return 0; + if (ha->primary_host[0] == '\0' || ha->primary_port == 0) + return 0; + return 1; +} + +/* + * ha_no_primary_known: returns 1 when this node is a STANDBY but does not + * yet know the primary's address. Callers should reply E_XCE_NOMASTER so + * the client retries rather than treating the error as permanent. + */ +int ha_no_primary_known(const ha_state_t *ha) +{ + ctrl_role_t role = + (ctrl_role_t)atomic_load_explicit(&ha->role, memory_order_acquire); + if (role == CTRL_PRIMARY) + return 0; + return (ha->primary_host[0] == '\0' || ha->primary_port == 0) ? 1 : 0; +} + /* * P1-5: cluster_t now embeds ha_state_t ha, so the three functions below * can dereference c->ha. diff --git a/src/controller/ha.h b/src/controller/ha.h index 70468684..e2d3087e 100644 --- a/src/controller/ha.h +++ b/src/controller/ha.h @@ -141,4 +141,21 @@ void ha_stop_watchdog(ha_state_t *ha); */ void ha_check_heartbeat_nocluster(ha_state_t *ha); +/* ── Redirect helpers used by join.c ──────────────────────── */ + +/* + * ha_should_redirect: returns 1 if this controller should redirect join + * requests to the primary. + * Returns 0 if: (a) role is PRIMARY, or (b) primary_host is empty / + * primary_port is 0 (no primary known yet — client should retry). + */ +int ha_should_redirect(const ha_state_t *ha); + +/* + * ha_no_primary_known: returns 1 if role is STANDBY and primary_host is + * empty or primary_port is 0. Caller should reply E_XCE_NOMASTER so the + * client knows to retry later rather than treating the error as permanent. + */ +int ha_no_primary_known(const ha_state_t *ha); + #endif /* HA_H */ diff --git a/src/controller/join.c b/src/controller/join.c index cbea7f3a..ffd95286 100644 --- a/src/controller/join.c +++ b/src/controller/join.c @@ -5,6 +5,7 @@ #include #include "cluster.h" +#include "ha.h" #include "xce/cluster_def.h" #include "xce/errnox.h" @@ -177,6 +178,32 @@ static int handle_join_(cluster_t* c, endpoint_t* ep, xce_request_t* request) TRACE("%s role %d join from %s", __func__, role, from->name); + /* ── HA guard (P1-6) ──────────────────────────────────────── + * A standby must never accept joins as if it were the primary. + * If the primary address is known, tell the client where to go + * (E_XCE_REDIRECT). If no primary is known yet, tell the client + * to retry later (E_XCE_NOMASTER). + */ + if (ha_should_redirect(&c->ha)) { + WARN("%s standby redirecting role-%d join from %s to %s:%u", + __func__, role, from->name, + c->ha.primary_host, c->ha.primary_port); + rc = -E_XCE_REDIRECT; + reply.status = rc; + reply.data_which = RequestReply_data_nodata; + xce_reply(ep, c->myhost, &reply); + return rc; + } + if (ha_no_primary_known(&c->ha)) { + WARN("%s standby has no primary yet, rejecting role-%d join from %s", + __func__, role, from->name); + rc = -E_XCE_NOMASTER; + reply.status = rc; + reply.data_which = RequestReply_data_nodata; + xce_reply(ep, c->myhost, &reply); + return rc; + } + switch (role) { case HostRole_client: rc = handle_client_join(c, ep, request); diff --git a/src/controller/meson.build b/src/controller/meson.build index 4bf76517..79d4cbf7 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -85,6 +85,7 @@ controller_unit_test_src = [ 'testsuite/test_e2e_capnp.c', 'testsuite/test_ha_role.c', 'testsuite/test_ha_cluster.c', + 'testsuite/test_ha_redirect.c', 'persist.c', 'ha.c', ] diff --git a/src/controller/testsuite/controller_test.c b/src/controller/testsuite/controller_test.c index c72e8e21..a5823877 100644 --- a/src/controller/testsuite/controller_test.c +++ b/src/controller/testsuite/controller_test.c @@ -21,6 +21,7 @@ extern void test_e2e_gc(void); extern void test_e2e_capnp(void); extern void test_ha_role(void); extern void test_ha_cluster(void); +extern void test_ha_redirect(void); void setUp(void) { return; } @@ -111,6 +112,7 @@ int main(int argc, char* argv[]) TEST(test_e2e_capnp); TEST(test_ha_role); TEST(test_ha_cluster); + TEST(test_ha_redirect); return UNITY_END(); } diff --git a/src/controller/testsuite/test_ha_redirect.c b/src/controller/testsuite/test_ha_redirect.c new file mode 100644 index 00000000..aea20008 --- /dev/null +++ b/src/controller/testsuite/test_ha_redirect.c @@ -0,0 +1,110 @@ +#include +#include +#include + +#include + +#include "ha.h" + +/* ── helpers ────────────────────────────────────────────────── */ + +static ha_config_t valid_cfg(void) +{ + ha_config_t cfg = { + .heartbeat_interval_ms = 3000, + .heartbeat_timeout_ms = 10000, + .election_timeout_ms = 2000, + .probe_connect_timeout_ms = 500, + .max_promotion_retries = 5, + }; + return cfg; +} + +/* ── tests ──────────────────────────────────────────────────── */ + +static void test_primary_does_not_redirect(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 0)); + /* Explicitly set PRIMARY role */ + atomic_store_explicit(&ha.role, (int32_t)CTRL_PRIMARY, memory_order_release); + + TEST_ASSERT_EQUAL_INT(0, ha_should_redirect(&ha)); + + ha_fini(&ha); + printf("test_primary_does_not_redirect: PASS\n"); +} + +static void test_standby_redirects_when_primary_known(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 1)); + /* Default role after init is STANDBY; set primary info */ + strncpy(ha.primary_host, "10.0.1.1", sizeof(ha.primary_host) - 1); + ha.primary_port = 7000; + + TEST_ASSERT_EQUAL_INT(1, ha_should_redirect(&ha)); + + ha_fini(&ha); + printf("test_standby_redirects_when_primary_known: PASS\n"); +} + +static void test_standby_no_redirect_when_no_primary_known(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 1)); + /* primary_host is empty after init — no known primary */ + TEST_ASSERT_EQUAL_INT(0, ha_should_redirect(&ha)); + TEST_ASSERT_EQUAL_INT(1, ha_no_primary_known(&ha)); + + ha_fini(&ha); + printf("test_standby_no_redirect_when_no_primary_known: PASS\n"); +} + +static void test_primary_is_not_no_primary_known(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 0)); + atomic_store_explicit(&ha.role, (int32_t)CTRL_PRIMARY, memory_order_release); + + TEST_ASSERT_EQUAL_INT(0, ha_no_primary_known(&ha)); + + ha_fini(&ha); + printf("test_primary_is_not_no_primary_known: PASS\n"); +} + +static void test_standby_no_primary_known_when_port_zero(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 1)); + /* Host set but port = 0 — not a valid redirect target */ + strncpy(ha.primary_host, "10.0.1.1", sizeof(ha.primary_host) - 1); + ha.primary_port = 0; + + TEST_ASSERT_EQUAL_INT(0, ha_should_redirect(&ha)); + TEST_ASSERT_EQUAL_INT(1, ha_no_primary_known(&ha)); + + ha_fini(&ha); + printf("test_standby_no_primary_known_when_port_zero: PASS\n"); +} + +/* ── suite entry point ─────────────────────────────────────── */ + +void test_ha_redirect(void) +{ + RUN_TEST(test_primary_does_not_redirect); + RUN_TEST(test_standby_redirects_when_primary_known); + RUN_TEST(test_standby_no_redirect_when_no_primary_known); + RUN_TEST(test_primary_is_not_no_primary_known); + RUN_TEST(test_standby_no_primary_known_when_port_zero); +} diff --git a/src/include/xce/errnox.h b/src/include/xce/errnox.h index ceecd418..41dd7ad5 100644 --- a/src/include/xce/errnox.h +++ b/src/include/xce/errnox.h @@ -19,6 +19,8 @@ enum { E_XCE_BAD_QUEUE, E_XCE_BAD_GROUP, E_XCE_INTERNAL, + E_XCE_NOMASTER, /* Standby has no known primary — client should retry */ + E_XCE_REDIRECT, /* Standby is redirecting client to primary */ E_XCE_MAX };