feat(libxce-failover): foundation — per-ctx failover state, redirect-hint fields, remove orphaned retry primitive (task 1.1)

This commit is contained in:
Rongsong Shen 2026-06-17 00:05:41 +00:00
parent 8c11655e53
commit 387ccfa8dc
17 changed files with 2122 additions and 562 deletions

View file

@ -0,0 +1,150 @@
# Brief: libxce-failover
## Problem
Client applications using libxce do **not** survive controller primary failover. When the
primary controller is killed and a new one is elected, in-flight and subsequent libxce calls
fail with an opaque `Invalid argument` (EINVAL) instead of either (a) retrying transparently
against the new primary or (b) reporting a clear "cluster is not ready" during the election
window.
Reproduction (3-node HA cluster, `set masters localhost:9991,localhost:9992,localhost:9993`):
1. Primary elected; `jobsub` / `joblist` work.
2. Kill the primary (`ctrl-1`).
3. Before a new primary is elected, `joblist` should report **cluster not ready**.
4. After a new primary is elected, all commands should transparently route to the new
primary with no error.
Observed instead: `joblist` keeps returning stale cached output, then `xce_joblist: Invalid
argument` indefinitely — the client never fails over.
## Current State
- The requirement already exists: **`ha-config-bootstrap` Requirement 7 — "Client-Side
Transparent Retry for Transient Controller Errors"** (acceptance criteria 7.1–7.6), and its
tasks 10 / 10.1 / 10.2 are marked complete.
- Only the **standalone primitive** was delivered:
- `xce_request_with_retry()` is implemented at `src/lib/request.c:1956` and unit-tested in
`src/lib/testsuite/test_libxce_retry.c` (NOT_READY backoff, STANDBY+redirect reconnect,
STANDBY-without-redirect fallback, non-retriable surface-immediately, max-retries,
50-client concurrency). `xce_retry_config_t` defaults: `max_retries=20`,
`initial_retry_ms=50`, `max_retry_ms=5000`, `backoff_multiplier=2.0`.
- **Gap 1 — wiring:** every production API path bypasses the wrapper. `xce_joblist` →
`xce_get_req` (`src/lib/request.c:1373`) → plain `xce_request()` (`request.c:1385`). The
same is true for ~18 other `xce_*_req` wrappers — none call `xce_request_with_retry()`.
- **Gap 2 — multi-master discovery:** the retry primitive operates on a single `endpoint_t`
and only follows an explicit `redirect_host/redirect_port`. A *killed* primary cannot
redirect, so the client must **rotate through its configured master list** (9991/9992/9993)
to discover the new primary. That orchestration does not exist; the client connects to
`masters[0]` once in `xcecli_start()` (`src/lib/client.c:122`) and never re-selects.
- Connections are long-lived and cached in `api_ctx_t->ep`; there is no dead-primary
detection or automatic reconnect at the API level.
## Desired Outcome
Failover is transparent to client applications:
- Short failovers are hidden: libxce retries across the configured masters with backoff and
succeeds against the new primary once elected.
- If the election outlasts the retry budget, the call returns a clear `E_XCE_NOT_READY`
("cluster not ready") rather than `EINVAL`.
- Both read and write requests fail over (writes rely on existing server-side commit-barrier +
idempotency guards to dedup re-sends).
- No per-daemon ad-hoc retry loops — the behavior lives entirely in libxce.
## Approach
**Central chokepoint (Approach A).** Make failover the default behavior at the single point
all wrappers already funnel through:
1. Give the request layer access to the cluster/master list + a "current primary" index
(design choice: a back-reference on `endpoint_t`, or a small `xce_cluster_t` the endpoint
references — resolved in design phase).
2. Upgrade `xce_request_with_retry()` to, in addition to its existing redirect/backoff cases:
- rotate through the master list on connection failure (`ECONNREFUSED`/`ETIMEDOUT`/RDHUP) or
`STANDBY`-without-redirect, reconnecting to each candidate to find the new primary;
- apply the bounded backoff budget and, on expiry, surface `E_XCE_NOT_READY`.
3. Route the production `xce_*_req` wrappers through the retry/failover path (e.g. make
`xce_request()` the failover wrapper over an inner `xce_request_raw()`), wiring every API
at once.
Chosen because it is the only approach that covers the entire client surface at one
integration point, reuses the already-tested primitive, treats reads and writes uniformly, and
keeps the requirement out of caller code — matching "transparent to the client application."
## Scope
- **In:**
- Wire all production libxce request paths through transparent retry/failover.
- Multi-master rotation to discover the new primary after the connected one dies.
- Bounded-budget semantics: transparent retry, then `E_XCE_NOT_READY` ("cluster not ready").
- Failover for both read and write requests (rely on server idempotency for writes).
- Tests: extend `test_libxce_retry.c` for master-list rotation + budget→NOT_READY; a 3-node
live failover integration test (kill primary, assert NOT_READY window then transparent
recovery).
- **Out:**
- Server-side election, WAL, commit barrier, idempotency guards (already owned by the HA
server specs; this spec consumes them).
- Changing the `xce_test` REPL behavior beyond what surfaces the new error semantics.
- Per-request idempotency/retry opt-in flags (writes retry by default per decision).
## Boundary Candidates
- Master-list + current-primary selection state and rotation policy.
- The retry/failover wrapper (backoff budget, error classification, NOT_READY surfacing).
- Integration seam: how production `xce_*_req` wrappers reach the failover path.
- Error-surface mapping: connection/EINVAL vs. `E_XCE_NOT_READY` vs. `E_XCE_STANDBY` vs.
non-retriable (auth/perm/invalid/noent).
## Out of Boundary
- Any controller/server-side change (election timing, redirect content, idempotency).
- New transport/probe opcodes unless design shows rotation cannot work with existing replies.
## Upstream / Downstream
- **Upstream:** `ha-config-bootstrap` (Req 7 retry primitive + `xce_retry_config_t` + error
codes `E_XCE_NOT_READY` / `E_XCE_STANDBY` + `redirect_host/port` reply fields);
`ha-commit-barrier-rollout` (server idempotency guards that make write retries safe);
`ha-single-leader-role` / election specs (define when a standby/primary answers).
- **Downstream:** worker, scheduler, and apiserver clients — all gain transparent failover and
can drop any ad-hoc retry handling; the `/xce-deploy-ha` live test harness.
## Existing Spec Touchpoints
- **Extends:** consumes and completes the intent of `ha-config-bootstrap` Req 7 (the primitive
was delivered but never integrated); this spec owns the integration + multi-master failover.
- **Adjacent:** `ha-repl-hardening`, `ha-convergence-recovery` (server-side failover behavior);
`worker-rejoin-resync` (worker-side rejoin, distinct from client-library failover).
## Protocol Correction (2026-06-16, requirements phase)
The discovery notes above assumed the controller signals non-primary via `E_XCE_STANDBY` with
`redirect_host/redirect_port` fields. Code review found the real taxonomy (`errnox.h:22-25`):
`E_XCE_REDIRECT` (standby knows a primary exists; `join.c:260-268`), `E_XCE_NOMASTER` (standby,
no primary known yet; `join.c:270-278`), `E_XCE_NOT_READY` (INIT/ELECTING), `E_XCE_STANDBY`
(primary-only op on a standby). The controller's redirect path sends `E_XCE_REDIRECT` with
`data_which = nodata` — **status only, no address** — so the existing `xce_request_with_retry()`
condition (`E_XCE_STANDBY` + populated redirect fields, `request.c:2010`) never matches a real
controller. Decisions (confirmed): classify on the actual statuses — `E_XCE_REDIRECT` →
fast-rotate the master list to find the existing primary (use an address if the reply carries
one, else rotate); `E_XCE_NOMASTER`/`E_XCE_NOT_READY` → backoff → cluster-not-ready;
`E_XCE_STANDBY` → rotate. The existing retry unit tests (keyed on the non-emitted shape) are
corrected to the real statuses. Tracked as bug-608.
**Scope expansion (confirmed):** the redirect address is absent today only because `join.c`
never fills the reply's `redirect_host/redirect_port` fields — those fields already exist in the
wire schema (`controller.capnp:568-569`) and the reply struct (`typemap.h:225-226`), and the
controller already knows the address (`join.c:263`). One small controller change to populate them
is now **in scope** (Requirement 8), making redirect-based failover deterministic (direct connect
to the primary, O(1)); client-side master-list rotation remains as a defensive fallback for an
older controller that omits the address.
## Constraints
- C library (libxce); follow project error-handling (`errnox`, early-return) and async-I/O
rules — `xce_request`/`xce_reply` non-blocking; never block the iomgr thread.
- TDD mandatory: failing tests in `src/lib/testsuite/` before implementation.
- Touches HA/failover behavior → full Kiro workflow (requirements → design → tasks), not the
quick path, per CLAUDE.md.
- Reuse the existing `xce_request_with_retry()` primitive and its config struct; do not fork a
second retry implementation.