diff --git a/.kiro/specs/libxce-failover/tasks.md b/.kiro/specs/libxce-failover/tasks.md index 27cc0ae..1a5e32c 100644 --- a/.kiro/specs/libxce-failover/tasks.md +++ b/.kiro/specs/libxce-failover/tasks.md @@ -77,7 +77,7 @@ - _Requirements: 1.1, 5.3_ - _Depends: 3.1_ - _Boundary: XcecliRetryLoop_ -- [ ] 5.3 (P) Populate the primary address in controller redirect replies +- [x] 5.3 (P) Populate the primary address in controller redirect replies - On the standby redirect path, fill the reply's redirect host and port from the known primary address; keep returning the no-primary status when the primary is unknown - Audit for any other redirect-emitting sites and apply the same population - Add a controller unit test: the redirect path populates the address, and an unknown primary still yields the no-primary status @@ -109,3 +109,4 @@ - 4.1: public `xcecli_reconnect_cb` typedef + `xcecli_set_reconnect_handler` (client.h/client.c, stores cb/arg in failover); socket-free `xcecli_invoke_rejoin` wired into xcecli_retry_next AFTER rotate+recover, BEFORE resume. hook<0 = failed attempt → bounded tail-recursion (consumes attempt budget, capped at max_retries). typedef MOVED from client_internal.h to client.h. API additive. - 5.1: `XCECLI_FAILOVER_CALL(ac,is_write,EXPR)` macro (client_internal.h) = lock(failover.lock) → retry init → do{rc=EXPR}while(xcecli_retry_next) → unlock → yield rc. Wrapped 18 reply-bearing ops; writes=true for jobsubmit/jobdispatch/jobctrl/queuectrl/hostctrl/update_resvals/update_jusage/runtask/join/set_pend_reason/clear_reason, reads=false otherwise. R1.3: zero signature changes. DESIGN CORRECTION: `xce_nop` is fire-and-forget (no cbuf_get, xceapi.c:30) — moved nop reply-read→fire-and-forget in design OpCategory; nop wraps in 5.2 not 5.1. - 5.2: `XCECLI_FF_CALL` macro (single-shot, NO retry loop/backoff/budget): lock → if known-dead (xcecli_ff_needs_reconnect = ctx.state==XCE_CTX_ERR) one rotate+recover → one send → unlock. Applied at the 3 wire chokepoints xcecli_notify/heartbeat/nop (notify_* delegate to xcecli_notify). wakeup/try_wakeup are LOCAL-only (cbuf_tryput own wbuf, no wire send) so correctly NOT wrapped. +- 5.3: `fill_redirect_reply(xce_reply_t*, const ha_state_t*)` (non-static, join.c) sets redirect_host=ha.primary_host + redirect_port=(uint16_t)ha.primary_port; called ONLY on the E_XCE_REDIRECT branch before xce_reply. Audit: join.c is the SOLE E_XCE_REDIRECT emitter. NOMASTER path unchanged (8.2). host alias (no copy) safe: ha lives cluster-lifetime, xce_reply serializes synchronously. New test_join_redirect.c registered in controller_test.c + meson.build. diff --git a/src/controller/join.c b/src/controller/join.c index 6c83b54..8b9e1ac 100644 --- a/src/controller/join.c +++ b/src/controller/join.c @@ -16,6 +16,21 @@ static int validate_host(cluster_t* c, const char* host, const char* key) return 1; } +/* + * fill_redirect_reply: populate an E_XCE_REDIRECT reply with the primary's + * host and port from ha state so the libxce client can use it as a one-hop + * hint (libxce-failover Req 8.1). The redirect_host pointer aliases the + * ha_state_t.primary_host buffer (capnp serialises it when non-NULL — see + * request.c); the caller must keep *ha live until xce_reply() runs, which it + * does (ha is embedded in cluster_t). Pure aside from the field writes — no + * locking, no allocation. + */ +void fill_redirect_reply(xce_reply_t* reply, const ha_state_t* ha) +{ + reply->redirect_host = (char*)ha->primary_host; + reply->redirect_port = (uint16_t)ha->primary_port; +} + static int handle_client_join(cluster_t* c, endpoint_t* ep, xce_request_t* request) { @@ -264,6 +279,7 @@ static int handle_join_(cluster_t* c, endpoint_t* ep, xce_request_t* request) rc = -E_XCE_REDIRECT; reply.status = rc; reply.data_which = RequestReply_data_nodata; + fill_redirect_reply(&reply, &c->ha); xce_reply(ep, c->myhost, &reply); return rc; } diff --git a/src/controller/meson.build b/src/controller/meson.build index c2502e6..7c4c04e 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -94,6 +94,7 @@ controller_unit_test_src = [ 'testsuite/test_ha_role_lifecycle.c', 'testsuite/test_ha_cluster.c', 'testsuite/test_ha_redirect.c', + 'testsuite/test_join_redirect.c', 'testsuite/test_p1_integration.c', 'testsuite/test_repl_unit.c', 'testsuite/test_repl_apply.c', diff --git a/src/controller/testsuite/controller_test.c b/src/controller/testsuite/controller_test.c index 89cb781..c0e0004 100644 --- a/src/controller/testsuite/controller_test.c +++ b/src/controller/testsuite/controller_test.c @@ -23,6 +23,7 @@ extern void test_ha_role(void); extern void test_ha_role_lifecycle(void); extern void test_ha_cluster(void); extern void test_ha_redirect(void); +extern void test_join_redirect(void); extern void test_p1_integration(void); extern void test_repl_unit(void); extern void test_repl_apply(void); @@ -150,6 +151,7 @@ int main(int argc, char* argv[]) TEST(test_ha_role_lifecycle); TEST(test_ha_cluster); TEST(test_ha_redirect); + TEST(test_join_redirect); TEST(test_p1_integration); TEST(test_repl_unit); TEST(test_repl_apply); diff --git a/src/controller/testsuite/test_join_redirect.c b/src/controller/testsuite/test_join_redirect.c new file mode 100644 index 0000000..7cd278b --- /dev/null +++ b/src/controller/testsuite/test_join_redirect.c @@ -0,0 +1,103 @@ +/* + * test_join_redirect.c — Task 5.3 (libxce-failover) + * + * ControllerRedirectAddress: the controller's E_XCE_REDIRECT reply must carry + * the primary's host and port so the libxce client can use it as a one-hop hint + * (Req 8.1). When no primary is known, the path must yield E_XCE_NOMASTER + * rather than an addressless redirect (Req 8.2). + * + * handle_join() is not directly unit-testable (needs a live endpoint + capnp + * transport), so join.c factors fill_redirect_reply(reply, ha) — the exact + * helper the production E_XCE_REDIRECT branch uses to populate the reply from + * ha state. These tests drive that helper. + */ + +#include +#include +#include + +#include + +#include "ha.h" /* ha_state_t, ha_init, ha_should_redirect, xce_reply_t (via typemap.h) */ +#include "xce/errnox.h" /* E_XCE_REDIRECT, E_XCE_NOMASTER */ + +/* + * Production helper defined in join.c and exercised on the E_XCE_REDIRECT path. + * Declared here (rather than in a controller header) because cluster.h pulls in + * netio.h/OpenSSL, which ha.h deliberately avoids; join.c is linked into this + * test binary, so an extern prototype is sufficient. + */ +extern void fill_redirect_reply(xce_reply_t *reply, const ha_state_t *ha); + +/* ── 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 ──────────────────────────────────────────────────── */ + +/* + * Req 8.1: when a primary is known, the redirect reply carries the primary's + * host and port verbatim from ha state. + */ +static void test_redirect_reply_carries_primary_address(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + xce_reply_t reply; + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 1)); + /* Default role after init is STANDBY; set a known primary. */ + strncpy(ha.primary_host, "10.0.1.7", sizeof(ha.primary_host) - 1); + ha.primary_port = 7001; + + /* Sanity: this is genuinely a redirect situation. */ + TEST_ASSERT_EQUAL_INT(1, ha_should_redirect(&ha)); + + memset(&reply, 0, sizeof(reply)); + fill_redirect_reply(&reply, &ha); + + TEST_ASSERT_NOT_NULL(reply.redirect_host); + TEST_ASSERT_EQUAL_STRING("10.0.1.7", reply.redirect_host); + TEST_ASSERT_EQUAL_UINT16(7001, reply.redirect_port); + + ha_fini(&ha); + printf("test_redirect_reply_carries_primary_address: PASS\n"); +} + +/* + * Req 8.2: when no primary is known, this is NOT a redirect situation + * (ha_should_redirect == 0, ha_no_primary_known == 1), so the join path + * returns E_XCE_NOMASTER rather than an addressless redirect. + */ +static void test_no_primary_known_yields_nomaster_not_redirect(void) +{ + ha_state_t ha; + ha_config_t cfg = valid_cfg(); + + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 1)); + /* primary_host 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_no_primary_known_yields_nomaster_not_redirect: PASS\n"); +} + +/* ── suite entry point ─────────────────────────────────────── */ + +void test_join_redirect(void) +{ + RUN_TEST(test_redirect_reply_carries_primary_address); + RUN_TEST(test_no_primary_known_yields_nomaster_not_redirect); +}