fix(proxy): fail over to healthy EL backups from live node state - #10
Merged
Conversation
Backups were only eligible when el_failover_active was true, but that flag is written once per monitor cycle, after the CL checks. Between a primary being marked unhealthy and the flag update, requests were refused with "No healthy EL node available" while a healthy backup sat idle (production incident 2026-07-15). select_el_node / select_el_ws_node now fail over based on the node states they already read; the flag stays for observability only (metrics, /status, transition logs) and no longer gates routing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Health checks built a fresh reqwest::Client per probe (EL once, CL twice per node), paying DNS + TCP + TLS each time. Under connection pressure the probes fail while the proxy's pooled connections still work, flapping nodes unhealthy spuriously. Probes now ride the shared proxy client with a per-request 5s deadline, measuring the same path proxied traffic uses. Also tightens the monitor cycle so failover engages sooner during outages: - CL health + slot probes run concurrently (was two sequential timeouts per unreachable node) - EL and CL passes run concurrently (independent state) - the failover flag is refreshed inside the EL pass, so flag and node states always move together Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes EL routing failover so requests immediately fall back to healthy backup EL nodes based on live node health state (rather than waiting for a per-cycle el_failover_active snapshot), and hardens health monitoring so probes use the shared proxy HTTP client with bounded timeouts and improved concurrency.
Changes:
- Update EL HTTP/WS node selection to always prefer a healthy primary, otherwise immediately select a healthy backup (no routing dependency on
el_failover_active). - Run EL and CL monitor passes concurrently; run CL health + slot checks concurrently; apply a shared 5s per-request probe deadline using
AppState::http_client. - Add regression tests covering “backup serves traffic before failover flag flips” and “flag refresh happens in the same EL pass that marks primaries unhealthy”.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/state.rs | Clarifies el_failover_active semantics as observability-only (not used for routing). |
| src/proxy/ws.rs | Removes failover-flag gating from WS selection; WS now fails over based on live node state. |
| src/proxy/selection.rs | Simplifies EL/EL-WS selection APIs to derive failover from node health state directly. |
| src/proxy/http.rs | Removes failover-flag gating from HTTP EL routing and adds a regression test for backup selection. |
| src/monitor.rs | Runs EL/CL checks concurrently; uses shared HTTP client for probes; refreshes failover flag inside EL pass; adds regression test for immediate backup usability. |
| src/health/mod.rs | Introduces a shared per-probe timeout constant used by health checks. |
| src/health/el.rs | Reuses shared HTTP client and enforces per-request probe timeout for EL checks. |
| src/health/cl.rs | Reuses shared HTTP client, enforces per-request probe timeout, and runs health+slot checks concurrently. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
During the 2026-07-15 connection issue, vixy logged
WARN vixy::proxy::http: No healthy EL node availableand returned 503s while a healthy backup node existed.Root cause:
select_el_nodeonly considered backups whenel_failover_activewas true — but that flag is a per-cycle snapshot, updated at the end of the monitor cycle, after the CL checks. Under connection failures those CL checks were slow (fresh HTTP client per probe, health + slot checked sequentially, up to ~10s of timeouts), so every cycle had a window where the primary was already marked unhealthy but the flag hadn't flipped → healthy fallback ignored → 503.A second contributor: every health probe built a fresh
reqwest::Client(fresh DNS + TCP + TLS). Under connection pressure the probes fail while the proxy's pooled connections still work, flapping nodes unhealthy spuriously — the likely source of the intermittent blips seen downstream.Changes
Commit 1 — routing fix
select_el_node/select_el_ws_nodefail over to healthy backups from live node state; no flag consultationel_failover_activeremains for observability only (metrics,/status, transition logs)Commit 2 — health-check hardening
AppState::http_client) with a 5s per-request deadline — they now measure the same pool proxied traffic usesVerification
clippy -- -D warningsclean/elat 50ms):200 primary → ~500ms of 502s (detection window, 2 failed checks) → 200 backup, zero 503s; all-nodes-down still returns 503; primary revival switches traffic back and clears the flagFollow-up (not in this PR)
max_retriesin config is dead — nothing in the forward path retries. Retrying connect-level failures against another healthy node would erase even the detection-window 502s.🤖 Generated with Claude Code