From c7e11a4e9efa6481f05a269b19a5edbf591e7205 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 17 Jun 2026 02:56:48 +0000 Subject: [PATCH] =?UTF-8?q?feat(libxce-failover):=20endpoint=20rotator=20?= =?UTF-8?q?=E2=80=94=20hint-first=20target=20selection=20+=20master=20rota?= =?UTF-8?q?tion;=20keep=20api=5Fctx=5Ft=20decls=20private=20(task=202.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .kiro/specs/libxce-failover/tasks.md | 3 +- src/include/xce/client_failover.h | 6 + src/lib/client_failover.c | 105 +++++++++++++++- src/lib/client_internal.h | 20 +++ src/lib/testsuite/test_libxce_retry.c | 171 ++++++++++++++++++++++++++ 5 files changed, 300 insertions(+), 5 deletions(-) diff --git a/.kiro/specs/libxce-failover/tasks.md b/.kiro/specs/libxce-failover/tasks.md index 840c304..53b7bb3 100644 --- a/.kiro/specs/libxce-failover/tasks.md +++ b/.kiro/specs/libxce-failover/tasks.md @@ -22,7 +22,7 @@ - Done: classification unit tests pass for every status and for both duplicate-replay cases - _Requirements: 2.5, 4.1, 4.2, 4.3, 4.5, 5.2_ - _Boundary: StatusClassifier_ -- [ ] 2.2 Implement endpoint reconnect and master rotation +- [x] 2.2 Implement endpoint reconnect and master rotation - On rotate, try the captured redirect-hint address first when one is set (then clear it); otherwise advance the master cursor to the next configured master and reconnect - Close the stale connection first and reuse the same endpoint object so reply routing stays intact; a failed hint simply falls through to the next master - Done: rotation unit tests show hint-first then next-master fallback, reusing a single endpoint object, with correctness independent of the hint @@ -102,3 +102,4 @@ ## Implementation Notes - 1.1: design said `XCE_MAXHOST_LEN`; real constant is `XCE_MAXHOSTNAME_LEN` (config.h, =256) — design corrected. `api_ctx_t` now in private `client_internal.h`; `xcecli_reconnect_cb` typedef lives there until task 4.1 adds the public setter. Build: `cd src && nix develop --command bash tools/xce-build.sh lib`. - 1.2 (ADAPTED): RED-against-undefined-symbols would break linking of the shared lib test binary across tasks, so 1.2 builds a GREEN reusable harness (`src/lib/testsuite/failover_harness.h`): fake multi-master config+cursor, an 11-row controller-status taxonomy table (resolves to real errnox.h codes), and an N-pthread runner. Component RED→GREEN tests live in 2.1/2.2/2.3/3.1/3.2. The harness defines `fo_expected_action_t` (FO_ACT_*) mirroring the design `xce_fo_action_t` — 2.1 asserts the real classifier against these rows. +- 2.2: EndpointRotator `xce_failover_next_target` (pure, socket-free: hint-first/consume, else cursor advance+wrap over conf->masters[]) + thin `xce_failover_rotate` (close-old/reconnect same endpoint). Live connect path is E2E (6.2). LAYERING: functions taking the private `api_ctx_t` are declared in `src/lib/client_internal.h`, NOT the public `xce/client_failover.h` (which keeps only the pure int-typed classifier) — per src/lib/CLAUDE.md (public headers in src/include/xce/ must not pull private state). diff --git a/src/include/xce/client_failover.h b/src/include/xce/client_failover.h index db24a39..e43583f 100644 --- a/src/include/xce/client_failover.h +++ b/src/include/xce/client_failover.h @@ -46,4 +46,10 @@ typedef enum { */ xce_fo_action_t xce_failover_classify(int status, bool is_write, int attempt); +/* + * EndpointRotator (xce_failover_next_target / xce_failover_rotate) operates on + * the private api_ctx_t and is therefore declared in the internal header + * src/lib/client_internal.h, not in this public surface. + */ + #endif /* XCE_CLIENT_FAILOVER_H */ diff --git a/src/lib/client_failover.c b/src/lib/client_failover.c index 0e23477..b5e7982 100644 --- a/src/lib/client_failover.c +++ b/src/lib/client_failover.c @@ -1,15 +1,26 @@ /* - * client_failover.c — StatusClassifier for the libxce-failover spec (task 2.1). + * client_failover.c — StatusClassifier (task 2.1) + EndpointRotator (task 2.2) + * for the libxce-failover spec. * - * Pure mapping from a reply status / transport rc to a failover action. No I/O, - * no globals. See design.md "Components and Interfaces → StatusClassifier" and - * requirements 2.5, 4.1, 4.2, 4.3, 4.5, 5.2. + * StatusClassifier: pure mapping from a reply status / transport rc to a + * failover action. No I/O, no globals (requirements 2.5, 4.1-4.3, 4.5, 5.2). + * + * EndpointRotator: choose and connect the next failover target. The target + * SELECTION (xce_failover_next_target) is a pure, socket-free helper so it is + * unit-testable; xce_failover_rotate() is a thin wrapper that closes the stale + * connection and reconnects the same endpoint object (requirements 2.1, 2.2, + * 2.3, 4.4, 8.4). See design.md "Components and Interfaces → EndpointRotator". */ #define _GNU_SOURCE #include "xce/client_failover.h" #include +#include +#include +#include "client_internal.h" /* api_ctx_t + EndpointRotator declarations */ +#include "config.h" /* XCE_MAXHOSTNAME_LEN */ +#include "netio.h" /* endpoint_init / endpoint_connect / endpoint_fini */ #include "xce/errnox.h" /* @@ -58,3 +69,89 @@ xce_fo_action_t xce_failover_classify(int status, bool is_write, int attempt) * status — is non-retriable and surfaced to the caller (4.5). */ return XCE_FO_TERMINAL; } + +/* ════════════════════════════════════════════════════════════════════════ + * EndpointRotator — target selection (pure) + connect wrapper (thin) + * ════════════════════════════════════════════════════════════════════════ */ + +/* True if a redirect-address hint is currently set on the ctx. The hint is a + * NUL-terminated host; an empty host ("") means "no hint" (xcectx.h). */ +static bool fo_has_hint(const api_ctx_t *ac) +{ + return ac->ctx.redirect_hint_host[0] != '\0'; +} + +/* Clear (consume) the redirect-address hint on the ctx. One-shot + advisory. */ +static void fo_clear_hint(api_ctx_t *ac) +{ + ac->ctx.redirect_hint_host[0] = '\0'; + ac->ctx.redirect_hint_port = 0; +} + +int xce_failover_next_target(api_ctx_t *ac, char *host_out, size_t host_sz, + uint16_t *port_out) +{ + if (ac == NULL || host_out == NULL || host_sz == 0 || port_out == NULL) + return -EINVAL; + + /* Hint-first (2.2, 8.3): if the last reply carried a redirect address, try + * it first and CONSUME it. The cursor is intentionally left untouched so a + * failed hint falls through to normal rotation on the next call (4.4) — + * correctness never depends on the hint. */ + if (fo_has_hint(ac)) { + strncpy(host_out, ac->ctx.redirect_hint_host, host_sz - 1); + host_out[host_sz - 1] = '\0'; + *port_out = ac->ctx.redirect_hint_port; + fo_clear_hint(ac); + return 0; + } + + /* No hint: advance the sticky master cursor to the NEXT configured master, + * wrapping around (2.1, 8.4). */ + if (ac->conf == NULL || ac->conf->nmasters <= 0) + return -ENOENT; + + int next = (ac->failover.cur_master + 1) % ac->conf->nmasters; + ac->failover.cur_master = next; + + const char *host = ac->conf->masters[next]; + if (host == NULL) + return -ENOENT; + + strncpy(host_out, host, host_sz - 1); + host_out[host_sz - 1] = '\0'; + *port_out = (uint16_t)ac->conf->master_ports[next]; + return 0; +} + +int xce_failover_rotate(api_ctx_t *ac) +{ + if (ac == NULL) + return -EINVAL; + + char host[XCE_MAXHOSTNAME_LEN] = { 0 }; + uint16_t port = 0; + + int rc = xce_failover_next_target(ac, host, sizeof(host), &port); + if (rc < 0) + return rc; + + /* Close the stale connection FIRST, then reconnect the SAME endpoint + * object so reply routing to ctx->wbuf (via ctx->ep == &ac->ep) is + * preserved. endpoint_fini() closes the fd, frees the SSL session, and + * clears handlers/peer; endpoint_init() recreates the socket fd and + * re-arms the object before endpoint_connect() drives the handshake. + * + * The live socket/handshake path is exercised by the E2E task (6.2); this + * wrapper keeps the I/O surface thin around the pure selector above. */ + endpoint_fini(&ac->ep); + + rc = endpoint_init(&ac->ep, AF_INET, SOCK_STREAM, 0, 0); + if (rc < 0) + return rc; + + endpoint_set_peer(&ac->ep, &ac->ctx.myinfo); + + rc = endpoint_connect(&ac->ep, host, (int)port); + return rc < 0 ? rc : 0; +} diff --git a/src/lib/client_internal.h b/src/lib/client_internal.h index 70bd38e..b43384e 100644 --- a/src/lib/client_internal.h +++ b/src/lib/client_internal.h @@ -2,6 +2,7 @@ #define _CLIENT_INTERNAL_H_ 1 +#include #include #include "client.h" @@ -60,4 +61,23 @@ typedef struct { xce_failover_state_t failover; } api_ctx_t; +/* + * EndpointRotator (task 2.2) — these operate on the private api_ctx_t, so they + * live in this internal header rather than the public xce/client_failover.h. + * + * xce_failover_next_target(): PURE target-selection (no I/O). If a redirect- + * address hint is set on ac->ctx, write it out and CONSUME it (one-shot, + * advisory). Otherwise advance the sticky master cursor (ac->failover.cur_master) + * to the next configured master in ac->conf, wrapping around. Returns 0, + * -EINVAL on a NULL/zero argument, or -ENOENT if no masters and no hint. + * Correctness never depends on the hint. + * + * xce_failover_rotate(): close the stale connection on ac->ep and reconnect the + * SAME endpoint object to the chosen target. Thin wrapper over the selector + + * endpoint_connect(); the live socket path is exercised by the E2E task. + */ +int xce_failover_next_target(api_ctx_t* ac, char* host_out, size_t host_sz, + uint16_t* port_out); +int xce_failover_rotate(api_ctx_t* ac); + #endif diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index 3885225..3f6bcc6 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -26,8 +26,11 @@ #include #include +#include + #include +#include "client_internal.h" /* api_ctx_t, xce_failover_state_t */ #include "failover_harness.h" #include "request.h" #include "xce/client_failover.h" @@ -450,6 +453,170 @@ static void test_classify_unknown_is_terminal(void) printf("test_classify_unknown_is_terminal: PASS\n"); } +/* ════════════════════════════════════════════════════════════════════════ + * Task 2.2 — EndpointRotator target selection (xce_failover_next_target) + * RED→GREEN tests. Pure, socket-free target-selection logic; the + * live connect path is covered by E2E task 6.2. + * ════════════════════════════════════════════════════════════════════════ */ + +/* Build a 3-master client config (distinct host:port) the rotator selects over. + * Strings are static-duration so the api_ctx may borrow them by pointer. */ +static void fo_make_client_masters(xce_client_config_t *conf) +{ + static char *hosts[N_MAX_MASTERS] = { "10.30.0.1", "10.30.0.2", + "10.30.0.3" }; + static int ports[N_MAX_MASTERS] = { 8001, 8002, 8003 }; + + memset(conf, 0, sizeof(*conf)); + conf->nmasters = N_MAX_MASTERS; + for (int i = 0; i < N_MAX_MASTERS; i++) { + conf->masters[i] = hosts[i]; + conf->master_ports[i] = ports[i]; + } +} + +/* Minimal api_ctx for target-selection tests: only conf, the failover cursor, + * and the ctx redirect-hint fields are exercised. No endpoint/socket touched. */ +static void fo_make_api_ctx(api_ctx_t *ac, xce_client_config_t *conf) +{ + memset(ac, 0, sizeof(*ac)); + ac->conf = conf; + ac->failover.cur_master = 0; +} + +static void fo_set_hint(api_ctx_t *ac, const char *host, uint16_t port) +{ + strncpy(ac->ctx.redirect_hint_host, host, XCE_MAXHOSTNAME_LEN - 1); + ac->ctx.redirect_hint_host[XCE_MAXHOSTNAME_LEN - 1] = '\0'; + ac->ctx.redirect_hint_port = port; +} + +/* Req 2.2, 8.4: with a redirect hint set, next_target returns the hint + * host/port and CONSUMES (clears) it; the following call ignores the stale + * hint and advances the master cursor instead. */ +static void test_next_target_hint_first_then_consumed(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + fo_set_hint(&ac, "10.99.0.7", 9100); + + char host[XCE_MAXHOSTNAME_LEN] = { 0 }; + uint16_t port = 0; + + int rc = xce_failover_next_target(&ac, host, sizeof(host), &port); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_STRING("10.99.0.7", host); + TEST_ASSERT_EQUAL_INT(9100, port); + + /* hint consumed: cleared on the ctx, cursor untouched */ + TEST_ASSERT_EQUAL_STRING("", ac.ctx.redirect_hint_host); + TEST_ASSERT_EQUAL_INT(0, ac.failover.cur_master); + + /* next call has no hint → advances cursor to master 1 */ + rc = xce_failover_next_target(&ac, host, sizeof(host), &port); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_STRING("10.30.0.2", host); + TEST_ASSERT_EQUAL_INT(8002, port); + TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master); + + printf("test_next_target_hint_first_then_consumed: PASS\n"); +} + +/* Req 2.1, 8.4: with no hint, successive next_target calls advance through the + * configured masters and WRAP around; the cursor lives on failover.cur_master. */ +static void test_next_target_rotates_and_wraps(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + + char host[XCE_MAXHOSTNAME_LEN] = { 0 }; + uint16_t port = 0; + + /* from cur_master=0, first rotation advances to master 1 */ + TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_STRING("10.30.0.2", host); + TEST_ASSERT_EQUAL_INT(8002, port); + TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master); + + TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_STRING("10.30.0.3", host); + TEST_ASSERT_EQUAL_INT(8003, port); + TEST_ASSERT_EQUAL_INT(2, ac.failover.cur_master); + + /* wrap back to master 0 */ + TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_STRING("10.30.0.1", host); + TEST_ASSERT_EQUAL_INT(8001, port); + TEST_ASSERT_EQUAL_INT(0, ac.failover.cur_master); + + printf("test_next_target_rotates_and_wraps: PASS\n"); +} + +/* Req 4.4, 8.4: after a hint is consumed, the very next call advances to the + * next master — correctness is independent of the (one-shot) hint. */ +static void test_next_target_hint_then_fallback_rotation(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + /* start the cursor at master 1 to prove fallback advances from current */ + ac.failover.cur_master = 1; + fo_set_hint(&ac, "10.99.0.9", 9200); + + char host[XCE_MAXHOSTNAME_LEN] = { 0 }; + uint16_t port = 0; + + /* hint first */ + TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_STRING("10.99.0.9", host); + TEST_ASSERT_EQUAL_INT(9200, port); + TEST_ASSERT_EQUAL_INT(1, ac.failover.cur_master); /* cursor untouched */ + + /* fallback: advances from master 1 → master 2 */ + TEST_ASSERT_EQUAL_INT(0, xce_failover_next_target(&ac, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_STRING("10.30.0.3", host); + TEST_ASSERT_EQUAL_INT(8003, port); + TEST_ASSERT_EQUAL_INT(2, ac.failover.cur_master); + + printf("test_next_target_hint_then_fallback_rotation: PASS\n"); +} + +/* Guard: NULL/invalid arguments are rejected without touching state. */ +static void test_next_target_rejects_bad_args(void) +{ + xce_client_config_t conf; + api_ctx_t ac; + fo_make_client_masters(&conf); + fo_make_api_ctx(&ac, &conf); + + char host[XCE_MAXHOSTNAME_LEN] = { 0 }; + uint16_t port = 0; + + TEST_ASSERT_EQUAL_INT(-EINVAL, + xce_failover_next_target(NULL, host, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + xce_failover_next_target(&ac, NULL, sizeof(host), + &port)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + xce_failover_next_target(&ac, host, 0, &port)); + TEST_ASSERT_EQUAL_INT(-EINVAL, + xce_failover_next_target(&ac, host, sizeof(host), + NULL)); + + printf("test_next_target_rejects_bad_args: PASS\n"); +} + /* ── Suite entry point ──────────────────────────────────────────────── */ void test_libxce_retry(void) @@ -467,5 +634,9 @@ void test_libxce_retry(void) RUN_TEST(test_classify_dup_replay_rule); RUN_TEST(test_classify_zero_is_success); RUN_TEST(test_classify_unknown_is_terminal); + RUN_TEST(test_next_target_hint_first_then_consumed); + RUN_TEST(test_next_target_rotates_and_wraps); + RUN_TEST(test_next_target_hint_then_fallback_rotation); + RUN_TEST(test_next_target_rejects_bad_args); UNITY_END(); }