myxce/.kiro/specs/component-failover-adoption/research.md

# Research & Design Decisions

## Summary
- **Feature**: `component-failover-adoption`
- **Discovery Scope**: Extension (light discovery — libxce, worker, scheduler, apiserver already exist; this wires three consumers onto an existing library mechanism)
- **Key Findings**:
  - `libxce-failover` (shipped, GO 2026-07-18) already built a **complete, transparent, per-call multi-master retry/rotate mechanism** (`XCECLI_FAILOVER_CALL` / `XCECLI_FAILOVER_CALL_COUNT` / `XCECLI_FF_CALL` in `client_internal.h`, wired into every public `xcecli_*` call in `client.c`, including `xcecli_join` and the initial `xcecli_start` connect). It requires only `xce_client_config_t.nmasters > 1` to activate. **No production code sets `nmasters` above 1.**
  - A **rejoin hook** (`xcecli_set_reconnect_handler`) exists specifically so a consumer can re-send its join after a transparent reconnect. It has **zero production callers** — only its own setter and a unit test reference it.
  - A second, older, **half-built and unused** primitive (`xce_connect_cluster()` / `xce_reconnect_init/step/fini()` in `src/lib/cluster.c`) also exists. Its production connect path (`do_connect()`) is a stub that always returns `NULL` outside unit tests, and it operates on raw `endpoint_t*`, not `xcecli_ctx_t`. No production code calls it. It predates/overlaps `libxce-failover` and is superseded by it.
  - This is a genuine "wire it up" feature, not a "build failover" feature — confirmed by the roadmap: `component-failover-adoption` (roadmap.md:64) was already recorded 2026-06-16 with this exact framing, HARD-blocked on `libxce-failover`, now unblocked.

## Research Log

### Why do worker/scheduler/apiserver each hand-roll a candidate loop instead of using the library mechanism?
- **Context**: Initial verification (this conversation) found worker, scheduler, and apiserver each independently loop over configured candidates, calling `xcecli_ctx_init`/`xcecli_start`/`xcecli_join` fresh for each candidate with `nmasters` hardcoded to `1` per attempt.
- **Sources Consulted**: `src/lib/client.c`, `src/lib/client_internal.h`, `src/worker/src/controller_connector.c`, `src/scheduler/src/scheduler.c`, `src/frontend/apiserver/backend/clib.go`, `roadmap.md`.
- **Findings**: `xcecli_start` (client.c:125-188) already rotates through ALL configured masters on the very first connect (bug-854 fix, comment at line 136). `xcecli_join` (client.c:291-300) is wrapped in `XCECLI_FAILOVER_CALL`, so it already retries and rotates on standby/redirect/transport-error responses. The three daemons' hand-rolled loops predate this (or were never updated after it landed) and now duplicate — imperfectly — what the library does correctly.
- **Implications**: The fix is subtractive as much as additive: delete the per-daemon candidate loops, populate `nmasters`/`masters[]`/`master_ports[]` fully once, and let the existing library mechanism do the rest. This directly satisfies Requirements 1 and most of Requirement 2 without new retry/rotation logic.

### Does the library mechanism, once enabled, also cover runtime reconnect (Requirement 2) automatically?
- **Findings**: Yes for any component whose normal operation already calls `xcecli_*` functions periodically or on demand — every one of those calls is wrapped in `XCECLI_FAILOVER_CALL`/`XCECLI_FAILOVER_CALL_COUNT`, so a transport failure on any call transparently triggers rotate+reconnect+retry within that same call. Fire-and-forget calls (`xcecli_nop`, `xcecli_heartbeat`) use `XCECLI_FF_CALL` instead: a bounded single best-effort reconnect attempt, never the full retry budget — this is the library's own liveness-safety design for exactly the concern in Requirement 3.
- **Implications**: Scheduler's `xcecli_jobdispatch` calls (scheduler.c:1289, called from the `sched_allocator` thread) and apiserver's per-request backend calls (`gCtx.ctx` in clib.go) get transparent reconnect for free once `nmasters` is set correctly — no new polling/reconnect loop needs to be written for either component.

### Does anything need a NEW re-join after a transparent reconnect, beyond what the library does automatically?
- **Findings**: The library's rejoin hook exists precisely because reconnecting the transport does not re-establish application-level cluster membership — a new primary does not automatically know a worker/scheduler/apiserver rejoined unless it re-sends its join (role) message. `xcecli_reconnect_cb` "runs on the calling thread" and returns 0 to resume the interrupted op or <0 to treat the rejoin as failed (loop rotates/backs off again).
- **Implications**: All three components must register a rejoin callback that re-issues `xcecli_join(ctx, <role>, ...)` (with worker also re-sending `hostattrs_t`). Without it, a component would reconnect at the transport level but the new primary would not recognize it as joined — likely surfacing as silently-dropped pushes/notifications rather than an obvious error.

### Thread-affinity of blocking retry calls (Requirement 3)
- **Findings**: Worker already isolates all controller-connection work (`controller_connector_start`, `xcecd_join_cluster`, and the periodic `checking_connection()` health-check) onto one dedicated thread (`start_controller_connector_thread` / `connector_to_controller`), separate from `hardware_monitor`/`system_monitor`/`process_manager` update calls in `xcecd_start()`. This isolation already satisfies Requirement 3 structurally for the worker — it is not something this feature needs to newly build, only preserve while removing the redundant candidate loop.
- Scheduler already runs `sched_job_picker` and `sched_allocator` as two separate `pthread_create`d threads (scheduler.c:511,516); `xcecli_jobdispatch` (the only call site found that could block on retry, scheduler.c:1289) executes from the allocator thread only. A blocked allocator during a controller outage is a benign, expected degradation (there is no work the allocator can do without a controller anyway) and does not stall the job picker or any liveness signal scheduler sends elsewhere — scheduler has no independent heartbeat-carrying thread to protect. No new threading is needed for the scheduler.
- apiserver: unlike the C daemons, ALL request-handling goroutines that touch the backend share the same `gCtx.ctx`, serialized by the library's per-context `failover.lock`. Under a real outage this could queue every concurrent API request behind the full retry budget, which is a client-experience problem (Requirement 4), not a cluster-liveness problem. Needs a lightweight fail-fast guard in the Go layer — see Design Decisions.
- **Implications**: No new dedicated thread is required for worker or scheduler (existing isolation already suffices — flagged as a Validation Hook to confirm during implementation, not re-derived here). apiserver needs a new, small piece of state (not a thread) to short-circuit blocking calls when the backend is known-down.

### Configuration surface mismatch: `N_MAX_MASTERS` vs `MAX_HA_CANDIDATES`
- **Findings**: `src/include/client.h:28` defines `N_MAX_MASTERS (3)` — the hard cap on `xce_client_config_t.masters[]`. `src/include/config.h:78` defines `MAX_HA_CANDIDATES 4` — the cap on `xce_cluster_config_t.candidates[]` used by `xce_parse_peers`. These are different constants. `xce_test` (`test.c:360-367`) already handles the mismatch correctly: parse up to 4 candidates, then clamp to `N_MAX_MASTERS` (3) with a `WARN` if truncated.
- **Implications**: Any new shared conversion helper must replicate this exact clamp-and-warn behavior (not silently drop entries, not error out) so all components behave identically when 4 candidates are configured. Reconciling the two constants into one is out of scope (pre-existing, unrelated to this feature).

### TLS/CA-cert wiring gap (pre-existing, out of boundary)
- **Findings**: `xce_client_config_t` (the struct actually consumed by `xcecli_ctx_init`/`xcecli_start`, i.e. the real connection path) has no CA-cert field at all — only `nmasters`/`masters`/`master_ports`/`from`/`key`. The `ca_cert_path` that worker/scheduler's `--ca-cert` flag populates on `xce_cluster_config_t` is validated by `xce_cluster_config_validate()` but is only ever read by the orphaned `xce_connect_cluster()` stub, never by the real connect path.
- **Implications**: This is a pre-existing gap unrelated to candidate-list wiring — the real connection already establishes TLS today (single-master) via whatever global SSL context setup exists outside this struct. Not this feature's concern; noted here so it isn't mistaken for something this design should fix.

## Architecture Pattern Evaluation

| Option | Description | Strengths | Risks / Limitations | Notes |
|--------|-------------|-----------|---------------------|-------|
| Adopt existing `xce_client_config_t` / `XCECLI_FAILOVER_CALL` mechanism (chosen) | Populate the full candidate list once per component, register a rejoin callback, delete hand-rolled loops | Already built, tested (`libxce-failover`), and exactly what `xce_test` uses today; minimal new code | None material — this is the reference-verified path | Matches roadmap's own framing of this backlog item |
| Complete and adopt `xce_connect_cluster()`/`xce_reconnect_*` | Finish the stubbed `do_connect()`, wire it under each daemon | N/A | Duplicate mechanism to the one already shipped; raw-`endpoint_t` API doesn't carry the join/request/reply protocol the daemons need — would need its own substantial new integration layer on top | Rejected — would build a second, redundant failover mechanism |
| Build a new per-component reconnect loop | Fix each daemon's existing hand-rolled loop in place, independently | Minimal library changes | Perpetuates 3 divergent implementations (the root problem this feature exists to fix); does not gain the library's already-tested retry/backoff/classification logic | Rejected — directly contradicts the roadmap item's stated goal |

## Design Decisions

### Decision: Adopt the existing `xcecli_ctx_t` / `XCECLI_FAILOVER_CALL` mechanism; do not build new failover logic
- **Context**: Requirements 1-3 need multi-candidate connect + runtime reconnect + liveness-safe retry.
- **Alternatives Considered**: see Architecture Pattern Evaluation above.
- **Selected Approach**: Each component populates a fully-formed `xce_client_config_t` (all configured candidates, not just the first) once, calls `xcecli_ctx_init`/`xcecli_start`/`xcecli_join` once, and registers a rejoin callback via `xcecli_set_reconnect_handler`. All subsequent `xcecli_*` calls already retry/rotate/reconnect transparently.
- **Rationale**: The mechanism is shipped, tested, and is the exact one `xce_test`'s `masters` setting already uses correctly. Building anything new would duplicate it.
- **Trade-offs**: None material for worker/scheduler. For apiserver, moving off the current Go-level per-peer loop means the CGO layer becomes responsible for candidate rotation instead of Go — acceptable since it is strictly more capable (transparent runtime reconnect, not just startup).
- **Follow-up**: Confirm during implementation that worker's hardware/process-monitor update calls truly run off the connector thread (Validation Hook, not re-verified byte-for-byte in this research pass).

### Decision: One new shared C helper converts `xce_cluster_config_t` → `xce_client_config_t`
- **Context**: All three components need to turn a parsed/validated candidate list into the `nmasters`/`masters[]`/`master_ports[]` triple, replicating `xce_test`'s clamp-to-`N_MAX_MASTERS`-with-WARN behavior (see mismatch finding above). Today this logic exists ad hoc in `test.c` only.
- **Alternatives Considered**: (1) Duplicate the clamp/copy logic in worker, scheduler, and apiserver's CGO layer separately. (2) One shared helper in `src/lib/cluster.c` (where `xce_parse_peers`/`xce_cluster_config_validate` already live), declared in `xce/cluster.h`.
- **Selected Approach**: (2) — a single `xce_cluster_config_to_client_config()` helper, reused by worker (C), scheduler (C), and apiserver (via CGO).
- **Rationale**: DRY; the clamp-and-warn behavior must be identical across components (Requirement 6's config-syntax-parity spirit extends naturally to the derived struct, not just the input string); one function is one thing to test.
- **Trade-offs**: None — this is pure generalization of logic that would otherwise be copy-pasted three times.

### Decision: apiserver calls the existing C `xce_parse_peers`/`xce_cluster_config_validate` via CGO instead of maintaining a second Go parser
- **Context**: Requirement 6 requires apiserver's candidate-list syntax to match scheduler/worker exactly. apiserver currently has its own hand-written Go `parsePeer` function.
- **Alternatives Considered**: (1) Keep/harden the Go parser to match C syntax by inspection. (2) Call the real C parser via CGO (apiserver already links libxce via CGO for everything else).
- **Selected Approach**: (2).
- **Rationale**: Guarantees byte-identical parsing forever (no drift risk), and apiserver already pays the CGO cost for every other backend interaction.
- **Trade-offs**: One more CGO surface, consistent with the existing `backend/` package pattern.

### Decision: apiserver gets a small Go-side "backend health" circuit breaker for Requirement 4
- **Context**: Requirement 4 needs apiserver to fail fast toward its own HTTP clients when the backend is down, rather than queue every request behind the library's full retry budget.
- **Alternatives Considered**: (1) Let every request block on the existing per-ctx lock and rely on Gin/client-side timeouts. (2) Track backend health from call outcomes + the rejoin callback, and check it before issuing a blocking backend call.
- **Selected Approach**: (2) — new `backend/health.go`: an atomic "degraded since" state, set when a call's classified outcome is retry-budget-exhaustion, cleared when the rejoin callback fires (successful reconnect+rejoin). Backend-call wrappers in `clib.go` check it first and return a clear error immediately while degraded, instead of entering the blocking call.
- **Rationale**: Small, localized, reuses signals the library already produces (call rc, rejoin callback) rather than adding new polling.
- **Trade-offs**: A short window exists where the flag is stale (a request arrives just as the backend goes down, before any call has failed) — acceptable, matches typical circuit-breaker behavior; the request in flight simply experiences the one blocking retry cycle before the flag is set for subsequent requests.

### Decision: rejoin callbacks must be lock-free flag-setters, never direct `xcecli_join` calls
- **Context**: A second `/kiro-validate-design` pass, cross-checking the library-invoked rejoin path against `client_failover.c` directly rather than only its header comments, found that `XCECLI_FAILOVER_CALL` holds `ac->failover.lock` — a plain, non-recursive `pthread_mutex_t` (`mylock_init` → `pthread_mutex_init(l, NULL)`) — for its entire retry loop, and `xcecli_invoke_rejoin()` (which calls the registered rejoin callback) runs synchronously *inside* that loop, lock held (`client_failover.c:397`). Every component's original design had its rejoin callback call `xcecli_join(...)` directly — itself `XCECLI_FAILOVER_CALL`-wrapped, so it would try to re-lock the same mutex on the same thread: a guaranteed self-deadlock on the very first real failover. This was never caught because `xcecli_set_reconnect_handler` has zero production callers and the existing unit test's callback never calls back into the client (it's a no-op probe).
- **Alternatives Considered**: (1) Have the callback call `xcecli_join` directly (original design — deadlocks). (2) Expose a new, lock-free/raw join primitive from libxce for rejoin callbacks to use (would require modifying `libxce-failover`, violating this spec's Allowed Dependencies boundary of consuming it read-only). (3) Callback only sets a lightweight flag and returns immediately (no I/O, no lock); the daemon's own calling code checks and clears the flag *after* the triggering call returns (lock free by then) and issues the real `xcecli_join` as a separate call.
- **Selected Approach**: (3), applied uniformly to worker (`checking_connection`), scheduler (the allocator, after `xcecli_jobdispatch`), and apiserver (each backend-call wrapper, after its `C.xcecli_*` call). apiserver's heartbeat-triggered path (Task 4.4) was already structured this way by construction (it acts after `xcecli_heartbeat` returns) and needed no change.
- **Rationale**: Fixes the deadlock without touching `libxce` at all, staying inside this spec's stated boundary; the fix is a small, uniform pattern applied identically across all three components rather than a special case.
- **Trade-offs**: One extra check-and-clear step after each relevant call site, in exchange for correctness; negligible cost.
- **Follow-up**: Worth flagging upstream to `libxce-failover`'s owners that the rejoin-hook contract (`client.h`'s doc comment for `xcecli_reconnect_cb`) doesn't document this lock-held constraint — a future consumer without this spec's context could reintroduce the deadlock. Not fixed here (out of boundary), but the regression-guard unit test added in design.md's Testing Strategy exists specifically because of this gap in the upstream contract's documentation.

### Decision: apiserver's rejoin registration uses a C shim in `clib.go`, not a standalone `reconnect.go` export
- **Context**: `/kiro-validate-design` review flagged that design.md originally described `reconnect.go` as "a `//export`ed Go function registered via `xcecli_set_reconnect_handler`," implying direct registration. This codebase already documents (`wire_dispatch_support.go:154-159`, explaining why `go_push_handler` needs the `xce_backend_handler` C shim) that cgo symbol resolution is per-file — a `C.<symbol>` reference only resolves in the file holding its own matching `extern` declaration.
- **Alternatives Considered**: (1) Register the exported Go function directly from `reconnect.go`. (2) Add a static C shim + `extern` declaration in `clib.go`'s own cgo preamble (mirroring the proven `xce_backend_handler` pattern) and register that.
- **Selected Approach**: (2).
- **Rationale**: Matches the codebase's own documented, working pattern instead of risking the same resolution failure `xce_backend_handler` was built to avoid.
- **Trade-offs**: None — same amount of code, just correctly placed.
- **Follow-up**: none; resolved by construction in design.md.

### Decision: apiserver's heartbeat prober explicitly triggers rejoin on recovery
- **Context**: `/kiro-validate-design` review found, by direct inspection of `client_internal.h`/`client_failover.c`, that `XCECLI_FF_CALL` (backing `xcecli_heartbeat`, apiserver's only idle-period liveness probe) never calls `xcecli_invoke_rejoin` — only `XCECLI_FAILOVER_CALL` (the full-retry path used by write calls) does. Left unaddressed, apiserver would repair its TCP connection via the heartbeat but never re-join, so push notifications (Req 2.4) would stay dead during any failover where no write request happens to arrive first.
- **Alternatives Considered**: (1) Switch the periodic prober from `xcecli_heartbeat` (fire-and-forget) to `xcecli_hello` (full retry, rejoin-bearing) — the same mechanism worker's `checking_connection()` already uses. (2) Keep `xcecli_heartbeat` (preserves its existing wire semantics/side effects on the controller) and have the Go-side tick callback explicitly detect a failure-to-success transition and manually invoke the same rejoin logic used elsewhere.
- **Selected Approach**: (2).
- **Rationale**: Preserves `xcecli_heartbeat`'s existing wire behavior (unknown/unverified whether the controller treats the heartbeat opcode differently from hello for apiserver accounting) while closing the gap with a small, self-contained Go-side check — no library changes, no behavior change to any other component.
- **Trade-offs**: A few extra lines in `heartbeat.go`; the rejoin logic now has two call sites (the cgo shim and the heartbeat callback), both invoking the same `reconnect.go` function, so no duplicated join-payload logic.
- **Follow-up**: none; resolved by construction in design.md.

### Decision: Do not touch `xce_connect_cluster()`/`xce_reconnect_*`
- **Context**: This second, unused, stubbed primitive exists in the same area of the codebase.
- **Selected Approach**: Leave as-is; do not adopt, complete, or remove it in this spec.
- **Rationale**: Out of this feature's boundary — removing dead code is a separate hardening decision, not required to satisfy any requirement here, and touching it risks scope creep into an unrelated cleanup.
- **Follow-up**: Worth a future hardening backlog note (superseded/dead code) — not filed as part of this spec.

## Risks & Mitigations
- Deleting worker's/scheduler's hand-rolled candidate loops could regress subtle existing behavior (e.g., worker's specific WARN logging on standby redirect) — mitigate by preserving equivalent log statements around the new single-call path, verified in code review.
- apiserver's circuit breaker could mask a fixed backend as still-degraded if the clearing signal (rejoin callback) doesn't fire in some edge case — mitigate with a bounded TTL on the degraded flag (re-probe after a short timeout) rather than relying solely on the rejoin callback.
- `N_MAX_MASTERS` (3) vs `MAX_HA_CANDIDATES` (4) mismatch could silently drop a 4th configured candidate on any component — mitigate by making the shared helper's WARN-and-clamp behavior loud and identical everywhere (already the plan).

## References
- `src/lib/client.c`, `src/lib/client_internal.h`, `src/include/xce/client_failover.h` — the shipped `libxce-failover` mechanism this feature adopts.
- `src/controller/testsuite/test.c:345-382` — the reference (`xce_test`) usage pattern.
- `.kiro/steering/roadmap.md:64` — `component-failover-adoption` backlog item this spec formalizes.
- `.kiro/steering/ha-async-io.md` — thread-role contract informing Requirement 3.