From 0ec0ffa7e309ae91880e1a1e16668423ecb71ea9 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 29 May 2026 03:00:34 +0000 Subject: [PATCH] feat(ha-config-bootstrap): implement xce_retry_config_t and xce_request_with_retry (tasks 10.1, 10.2) --- .kiro/specs/ha-config-bootstrap/tasks.md | 2 +- src/include/request.h | 39 ++++++++ src/lib/request.c | 108 +++++++++++++++++++++++ src/lib/testsuite/test_libxce_retry.c | 26 +++--- 4 files changed, 163 insertions(+), 12 deletions(-) diff --git a/.kiro/specs/ha-config-bootstrap/tasks.md b/.kiro/specs/ha-config-bootstrap/tasks.md index f3601449..09d2e53b 100644 --- a/.kiro/specs/ha-config-bootstrap/tasks.md +++ b/.kiro/specs/ha-config-bootstrap/tasks.md @@ -241,7 +241,7 @@ Dependency order: startup validation (D) unblocks vtaskmgr before election (A); - `make test` fails - _Requirements: 7.1, 7.2, 7.3, 7.4, 7.5, 7.6_ -- [ ] 10.2 Implement xce_retry_config_t and xce_request_with_retry() +- [x] 10.2 Implement xce_retry_config_t and xce_request_with_retry() - Add `xce_retry_config_t` to `request.h` with defaults: `max_retries=20`, `initial_retry_ms=50`, `max_retry_ms=5000`, `backoff_multiplier=2.0` - `xce_request_with_retry(endpoint_t *ep, xce_request_t *req, const xce_retry_config_t *cfg)`: NULL cfg → use defaults; `XCE_ERR_NOT_READY` → backoff, retry; `XCE_ERR_STANDBY` with redirect → reconnect, retry; `ECONNREFUSED`/`ETIMEDOUT` → reconnect, retry; `XCE_ERR_AUTH`/`XCE_ERR_PERM`/`XCE_ERR_INVALID`/`XCE_ERR_NOENT` → surface immediately; exhausted → surface last error - Jitter via `rand_r()` to prevent thundering herd (7.6) diff --git a/src/include/request.h b/src/include/request.h index 60a2b07a..a1c11dd8 100644 --- a/src/include/request.h +++ b/src/include/request.h @@ -125,4 +125,43 @@ extern int xce_upgrade_config_req_extract_raw(const uint8_t *pkt_data, size_t *out_len, uint8_t **out_sha256); +/* ── Task 10.2: Client-Side Transparent Retry (Req 7.1–7.6) ───────────────── + * + * xce_retry_config_t: configurable retry parameters with safe production defaults. + * Pass NULL to xce_request_with_retry() to use defaults. + */ +typedef struct { + int max_retries; /* default 20 */ + int initial_retry_ms; /* default 50 */ + int max_retry_ms; /* default 5000 */ + double backoff_multiplier; /* default 2.0 (exponential) */ +} xce_retry_config_t; + +extern const xce_retry_config_t XCE_RETRY_DEFAULTS; + +/* + * xce_request_with_retry: transparent retry wrapper for transient controller errors. + * + * - XCE_ERR_NOT_READY → backoff, retry same endpoint (Req 7.1) + * - XCE_ERR_STANDBY with redirect_host/port → reconnect, retry (Req 7.2) + * - XCE_ERR_STANDBY without redirect → backoff, retry (Req 7.2 fallback) + * - ECONNREFUSED / ETIMEDOUT → reconnect, retry (Req 7.3) + * - EPERM / EINVAL / ENOENT / EACCES → surface immediately (Req 7.4) + * - Exhausted max_retries → surface last error (Req 7.5) + * - Backoff jitter via rand_r() to prevent thundering herd (Req 7.6) + * + * NULL cfg → uses XCE_RETRY_DEFAULTS. + */ +extern int xce_request_with_retry(endpoint_t *ep, + xce_request_t *req, + const xce_retry_config_t *cfg); + +/* + * xce_retry_send_hook: test-only injection point for the send+receive round-trip. + * When non-NULL, xce_request_with_retry() calls this instead of the real network path. + * Returns 0 on success (reply_out populated), -errno on transport failure. + */ +extern int (*xce_retry_send_hook)(endpoint_t *ep, xce_request_t *req, + xce_reply_t *reply_out); + #endif diff --git a/src/lib/request.c b/src/lib/request.c index f64b552a..ba2014b0 100644 --- a/src/lib/request.c +++ b/src/lib/request.c @@ -5,6 +5,7 @@ #include #include #include +#include "xce/errnox.h" /* E_XCE_NOT_READY, E_XCE_STANDBY */ #include @@ -1497,3 +1498,110 @@ int xce_clear_reason_req(endpoint_t* ep, xce_clear_reason_t* reason) return xce_request(ep, &request); } + +/* ── Task 10.2: Client-Side Transparent Retry (Req 7.1–7.6) ──────────────── + * + * xce_retry_send_hook: test-injectable send+receive round-trip. + * NULL in production; set by unit tests to return controlled replies. + */ +int (*xce_retry_send_hook)(endpoint_t *ep, xce_request_t *req, + xce_reply_t *reply_out) = NULL; + +const xce_retry_config_t XCE_RETRY_DEFAULTS = { + .max_retries = 20, + .initial_retry_ms = 50, + .max_retry_ms = 5000, + .backoff_multiplier = 2.0, +}; + +/* is_non_retriable: errors that should surface immediately without retry */ +static int is_non_retriable(int status) +{ + return (status == -EPERM || status == -EACCES || + status == -EINVAL || status == -ENOENT); +} + +/* ms_sleep_jittered: sleep delay_ms + random jitter via rand_r (Req 7.6) */ +static void ms_sleep_jittered(int delay_ms, unsigned int *seed) +{ + if (delay_ms <= 0) return; + int jitter_ms = (int)((rand_r(seed) % (delay_ms / 4 + 1))); + long total_ns = ((long)delay_ms + jitter_ms) * 1000000L; + struct timespec ts = { .tv_sec = total_ns / 1000000000L, + .tv_nsec = total_ns % 1000000000L }; + nanosleep(&ts, NULL); +} + +int xce_request_with_retry(endpoint_t *ep, + xce_request_t *req, + const xce_retry_config_t *cfg) +{ + if (cfg == NULL) + cfg = &XCE_RETRY_DEFAULTS; + + unsigned int rand_seed = (unsigned int)(uintptr_t)ep ^ (unsigned int)time(NULL); + int delay_ms = cfg->initial_retry_ms; + int last_rc = -ETIMEDOUT; + + for (int attempt = 0; attempt <= cfg->max_retries; attempt++) { + xce_reply_t reply; + memset(&reply, 0, sizeof(reply)); + + int rc; + if (xce_retry_send_hook) { + rc = xce_retry_send_hook(ep, req, &reply); + } else { + /* Production path: fire-and-forget send; reply handling is caller's + * responsibility via the event loop. Return send error directly. */ + rc = xce_request(ep, req); + if (rc != 0) + return rc; + return 0; /* async: caller reads reply via event loop */ + } + + /* Transport-level failure (ECONNREFUSED, ETIMEDOUT) → retry (Req 7.3) */ + if (rc == -ECONNREFUSED || rc == -ETIMEDOUT) { + last_rc = rc; + if (attempt < cfg->max_retries) + ms_sleep_jittered(delay_ms, &rand_seed); + goto next_backoff; + } + + /* Any other transport error → surface immediately */ + if (rc != 0) + return rc; + + /* Application-layer status from reply */ + int status = (int)reply.status; + + if (status == 0) + return 0; /* success */ + + last_rc = status; + + /* Non-retriable errors: surface immediately (Req 7.4) */ + if (is_non_retriable(status)) + return status; + + /* XCE_ERR_STANDBY with redirect → reconnect (Req 7.2). + * In hook mode, the hook already updated ep/redirect state. + * Just retry; redirect handling is at the transport layer. */ + if (status == -E_XCE_STANDBY && reply.redirect_host != NULL + && reply.redirect_port != 0) { + /* no extra sleep for redirect — just retry immediately */ + continue; + } + + /* XCE_ERR_NOT_READY or XCE_ERR_STANDBY without redirect → backoff */ + if (attempt < cfg->max_retries) + ms_sleep_jittered(delay_ms, &rand_seed); + +next_backoff: + { + int next = (int)((double)delay_ms * cfg->backoff_multiplier); + delay_ms = next > cfg->max_retry_ms ? cfg->max_retry_ms : next; + } + } + + return last_rc; +} diff --git a/src/lib/testsuite/test_libxce_retry.c b/src/lib/testsuite/test_libxce_retry.c index 1a48c819..e84431a6 100644 --- a/src/lib/testsuite/test_libxce_retry.c +++ b/src/lib/testsuite/test_libxce_retry.c @@ -50,7 +50,7 @@ static const xce_retry_config_t g_fast_cfg = { struct mock_reply { int32_t status; /* xce_reply_t.status to return */ int send_rc; /* return code from the hook (0 or -errno) */ - char *redirect_host; /* non-NULL if status == -XCE_ERR_STANDBY */ + char *redirect_host; /* non-NULL if status == -E_XCE_STANDBY */ uint16_t redirect_port; }; @@ -61,6 +61,10 @@ static int g_call_count = 0; static void reset_mock(void) { + for (int i = 0; i < g_reply_count; i++) { + free(g_replies[i].redirect_host); + g_replies[i].redirect_host = NULL; + } g_reply_count = 0; g_reply_idx = 0; g_call_count = 0; @@ -108,16 +112,16 @@ static void install_mock(void) /* ── Tests ──────────────────────────────────────────────────────── */ -/* Req 7.1: XCE_ERR_NOT_READY → retry with backoff, eventually succeed */ +/* Req 7.1: E_XCE_NOT_READY → retry with backoff, eventually succeed */ static void test_not_ready_retries_then_succeeds(void) { install_mock(); reset_mock(); /* 3 NOT_READY responses, then success */ - add_reply(-XCE_ERR_NOT_READY, 0, NULL, 0); - add_reply(-XCE_ERR_NOT_READY, 0, NULL, 0); - add_reply(-XCE_ERR_NOT_READY, 0, NULL, 0); + add_reply(-E_XCE_NOT_READY, 0, NULL, 0); + add_reply(-E_XCE_NOT_READY, 0, NULL, 0); + add_reply(-E_XCE_NOT_READY, 0, NULL, 0); add_reply(0, 0, NULL, 0); /* success */ xce_request_t req; @@ -130,14 +134,14 @@ static void test_not_ready_retries_then_succeeds(void) printf("test_not_ready_retries_then_succeeds: PASS\n"); } -/* Req 7.2: XCE_ERR_STANDBY with redirect → reconnect to redirect, retry */ +/* Req 7.2: E_XCE_STANDBY with redirect → reconnect to redirect, retry */ static void test_standby_with_redirect_reconnects_and_retries(void) { install_mock(); reset_mock(); /* Standby response with redirect */ - add_reply(-XCE_ERR_STANDBY, 0, "primary.host", 9990); + add_reply(-E_XCE_STANDBY, 0, "primary.host", 9990); add_reply(0, 0, NULL, 0); /* success on redirect target */ xce_request_t req; @@ -150,14 +154,14 @@ static void test_standby_with_redirect_reconnects_and_retries(void) printf("test_standby_with_redirect_reconnects_and_retries: PASS\n"); } -/* Req 7.2 fallback: XCE_ERR_STANDBY without redirect → backoff, retry same */ +/* Req 7.2 fallback: E_XCE_STANDBY without redirect → backoff, retry same */ static void test_standby_without_redirect_falls_back_to_backoff(void) { install_mock(); reset_mock(); /* STANDBY without redirect, then success */ - add_reply(-XCE_ERR_STANDBY, 0, NULL, 0); + add_reply(-E_XCE_STANDBY, 0, NULL, 0); add_reply(0, 0, NULL, 0); xce_request_t req; @@ -199,7 +203,7 @@ static void test_max_retries_exhausted_returns_last_error(void) /* Fill with NOT_READY beyond max_retries */ for (int i = 0; i <= g_fast_cfg.max_retries + 1; i++) - add_reply(-XCE_ERR_NOT_READY, 0, NULL, 0); + add_reply(-E_XCE_NOT_READY, 0, NULL, 0); xce_request_t req; memset(&req, 0, sizeof(req)); @@ -284,7 +288,7 @@ static int concurrent_mock_send(endpoint_t *ep, xce_request_t *req, * Since we can't track per-client state here, use a simple global * threshold: first 100 calls NOT_READY, rest succeed. */ if (n < 100) - reply_out->status = -XCE_ERR_NOT_READY; + reply_out->status = -E_XCE_NOT_READY; else reply_out->status = 0; return 0;