From fa230f1f3fab2aea3e12864ec7710059c14dc7d0 Mon Sep 17 00:00:00 2001 From: Ki Ageng Satria Pamungkas Date: Fri, 23 Jan 2026 16:57:29 +0700 Subject: [PATCH] feat(health): add configurable retry logic for health checks Add health_check_max_failures config to prevent nodes from being marked unhealthy on transient failures. Nodes now require X consecutive failed health checks (default: 3) before being marked unhealthy, providing better resilience against temporary network issues and momentary lag spikes. Key changes: - Add health_check_max_failures to Global config (default: 3) - Add consecutive_failures tracking to ElNodeState and ClNodeState - Update calculate_el_health() and calculate_cl_health() to implement retry logic - Reset failure counter on successful health check - Update all test helpers and add comprehensive retry/recovery tests The retry logic prevents health status flapping while maintaining quick recovery when nodes become healthy again. Co-Authored-By: Claude Sonnet 4.5 --- Cargo.lock | 2 +- DIARY.md | 44 +++++++++++++ config.example.toml | 4 ++ src/config.rs | 3 + src/health/cl.rs | 110 ++++++++++++++++++++++++++++++--- src/health/el.rs | 110 ++++++++++++++++++++++++++++++--- src/monitor.rs | 16 ++++- src/proxy/http.rs | 3 + src/proxy/selection.rs | 2 + src/proxy/ws.rs | 2 + src/state.rs | 9 +++ tests/steps/cl_health_steps.rs | 5 +- tests/steps/el_health_steps.rs | 1 + 13 files changed, 286 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a3f2a2c..d270840 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2802,7 +2802,7 @@ checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" [[package]] name = "vixy" -version = "0.1.2" +version = "0.1.3" dependencies = [ "axum", "clap", diff --git a/DIARY.md b/DIARY.md index 66aa201..fbbdf6d 100644 --- a/DIARY.md +++ b/DIARY.md @@ -30,6 +30,50 @@ A log of the development journey building Vixy - an Ethereum EL/CL proxy in Rust +### 2026-01-23 - Configurable Health Check Retry Logic + +**What I did:** +- Added configurable health check retry logic to prevent nodes from being marked unhealthy on transient failures +- Added `health_check_max_failures` config field to Global struct (default: 3) +- Added `consecutive_failures` tracking to both ElNodeState and ClNodeState +- Updated health calculation functions to only mark nodes unhealthy after X consecutive failures +- Nodes reset their failure counter when they recover +- Updated all test code to work with the new logic +- Added comprehensive tests for retry and recovery scenarios + +**Key Implementation Details:** +- Config field: `health_check_max_failures` in `[global]` section +- Node state tracking: `consecutive_failures: u32` field +- Health calculation logic: + - If check passes: reset consecutive_failures to 0, mark as healthy + - If check fails: increment consecutive_failures + - Only mark unhealthy if consecutive_failures >= threshold +- Updated function signatures: `calculate_el_health(node, chain_head, max_lag, max_failures)` + +**Tests Added:** +- test_el_node_consecutive_failures_reset_on_recovery +- test_cl_node_consecutive_failures_reset_on_recovery +- Updated existing tests to verify retry behavior (3 failures before unhealthy) + +**Challenges faced:** +- Had to update all test helpers across multiple modules (http.rs, selection.rs, ws.rs, monitor.rs) +- Needed to ensure the retry logic works correctly for both EL and CL nodes +- Had to update both health check functions and all test code simultaneously + +**How I solved it:** +- Added the consecutive_failures field to both node state structs +- Modified health calculation to track failures and only mark unhealthy after threshold +- Updated all test helpers to include the new fields +- Updated tests to verify the retry behavior works correctly + +**What I learned:** +- Rust's type system makes refactoring safe - compiler caught all missing fields +- Configurable retry logic prevents flapping between healthy/unhealthy states +- Resetting the counter on success allows nodes to recover naturally +- Comprehensive tests ensure the retry logic works as expected + +**Mood:** Accomplished - this is a valuable feature for production resilience! + ### 2026-01-21 - Fixed WSS/TLS Connection Support **What I did:** diff --git a/config.example.toml b/config.example.toml index c9dda02..4bd9d34 100644 --- a/config.example.toml +++ b/config.example.toml @@ -17,6 +17,10 @@ proxy_timeout_ms = 30000 # Maximum number of retry attempts for failed proxy requests max_retries = 2 +# Number of consecutive health check failures before marking node as unhealthy +# This prevents transient failures from immediately marking a node as unhealthy +health_check_max_failures = 3 + [metrics] # Enable or disable Prometheus metrics enabled = true diff --git a/src/config.rs b/src/config.rs index efdfed7..ed46d32 100644 --- a/src/config.rs +++ b/src/config.rs @@ -32,6 +32,8 @@ pub struct Global { pub proxy_timeout_ms: u64, /// Maximum number of retry attempts for failed proxy requests pub max_retries: u32, + /// Number of consecutive health check failures before marking node as unhealthy + pub health_check_max_failures: u32, } /// Metrics configuration settings @@ -62,6 +64,7 @@ impl Default for Global { health_check_interval_ms: 1000, proxy_timeout_ms: 30000, max_retries: 2, + health_check_max_failures: 3, } } } diff --git a/src/health/cl.rs b/src/health/cl.rs index 19eaca1..a8b4feb 100644 --- a/src/health/cl.rs +++ b/src/health/cl.rs @@ -81,12 +81,32 @@ pub fn update_cl_chain_head(nodes: &[ClNodeState]) -> u64 { } /// Calculate health status for a CL node based on chain head and max lag -pub fn calculate_cl_health(node: &mut ClNodeState, chain_head: u64, max_lag: u64) { +pub fn calculate_cl_health( + node: &mut ClNodeState, + chain_head: u64, + max_lag: u64, + max_failures: u32, +) { // Calculate lag (how far behind the node is from chain head) node.lag = chain_head.saturating_sub(node.slot); - // Node is healthy if health endpoint is OK AND lag is within threshold - node.is_healthy = node.health_ok && node.lag <= max_lag; + // Determine if this check passed (health endpoint OK AND lag is within threshold) + let check_passed = node.health_ok && node.lag <= max_lag; + + if check_passed { + // Reset consecutive failures on success + node.consecutive_failures = 0; + node.is_healthy = true; + } else { + // Increment consecutive failures + node.consecutive_failures += 1; + + // Only mark as unhealthy if we've exceeded the threshold + if node.consecutive_failures >= max_failures { + node.is_healthy = false; + } + // Otherwise, keep the current health status (might still be healthy from before) + } } #[cfg(test)] @@ -205,6 +225,7 @@ mod tests { health_ok, is_healthy: false, lag: 0, + consecutive_failures: 0, } } @@ -213,7 +234,7 @@ mod tests { let mut node = make_cl_node("test", 1000, true); let chain_head = 1005; - calculate_cl_health(&mut node, chain_head, 10); + calculate_cl_health(&mut node, chain_head, 10, 3); assert_eq!(node.lag, 5, "Lag should be chain_head - slot"); } @@ -221,14 +242,32 @@ mod tests { #[test] fn test_cl_node_unhealthy_when_health_fails() { let mut node = make_cl_node("test", 1000, false); // health_ok = false + node.is_healthy = true; // Start as healthy let chain_head = 1000; let max_lag = 3; - calculate_cl_health(&mut node, chain_head, max_lag); + // First failure + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + // Second failure + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + + // Third failure + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 3); assert!( !node.is_healthy, - "Node should be unhealthy when health_ok is false" + "Node should be unhealthy after 3 failures" ); assert_eq!(node.lag, 0); } @@ -236,14 +275,32 @@ mod tests { #[test] fn test_cl_node_unhealthy_when_lagging() { let mut node = make_cl_node("test", 990, true); // health_ok = true + node.is_healthy = true; // Start as healthy let chain_head = 1000; let max_lag = 3; - calculate_cl_health(&mut node, chain_head, max_lag); + // First failure - still healthy + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + // Second failure - still healthy + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + + // Third failure - now unhealthy + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 3); assert!( !node.is_healthy, - "Node should be unhealthy when lag > max_lag" + "Node should be unhealthy after 3 failures" ); assert_eq!(node.lag, 10); } @@ -254,7 +311,7 @@ mod tests { let chain_head = 1000; let max_lag = 3; - calculate_cl_health(&mut node, chain_head, max_lag); + calculate_cl_health(&mut node, chain_head, max_lag, 3); assert!( node.is_healthy, @@ -269,7 +326,7 @@ mod tests { let chain_head = 1000; let max_lag = 3; - calculate_cl_health(&mut node, chain_head, max_lag); + calculate_cl_health(&mut node, chain_head, max_lag, 3); assert!( node.is_healthy, @@ -278,6 +335,39 @@ mod tests { assert_eq!(node.lag, 3); } + #[test] + fn test_cl_node_consecutive_failures_reset_on_recovery() { + let mut node = make_cl_node("test", 990, true); + node.is_healthy = true; // Start as healthy + let chain_head = 1000; + let max_lag = 3; + + // First failure - still healthy + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + + // Second failure - still healthy + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + + // Recovery - node catches up + node.slot = 1000; + calculate_cl_health(&mut node, chain_head, max_lag, 3); + assert_eq!( + node.consecutive_failures, 0, + "Consecutive failures should reset on success" + ); + assert!(node.is_healthy, "Node should be healthy after recovery"); + } + // ========================================================================= // update_cl_chain_head tests // ========================================================================= diff --git a/src/health/el.rs b/src/health/el.rs index 22e1d5c..8d60a9e 100644 --- a/src/health/el.rs +++ b/src/health/el.rs @@ -85,12 +85,32 @@ pub fn update_el_chain_head(nodes: &[ElNodeState]) -> u64 { } /// Calculate health status for an EL node based on chain head and max lag -pub fn calculate_el_health(node: &mut ElNodeState, chain_head: u64, max_lag: u64) { +pub fn calculate_el_health( + node: &mut ElNodeState, + chain_head: u64, + max_lag: u64, + max_failures: u32, +) { // Calculate lag (how far behind the node is from chain head) node.lag = chain_head.saturating_sub(node.block_number); - // Node is healthy if check succeeded AND lag is within the allowed threshold - node.is_healthy = node.check_ok && node.lag <= max_lag; + // Determine if this check passed (check succeeded AND lag is within threshold) + let check_passed = node.check_ok && node.lag <= max_lag; + + if check_passed { + // Reset consecutive failures on success + node.consecutive_failures = 0; + node.is_healthy = true; + } else { + // Increment consecutive failures + node.consecutive_failures += 1; + + // Only mark as unhealthy if we've exceeded the threshold + if node.consecutive_failures >= max_failures { + node.is_healthy = false; + } + // Otherwise, keep the current health status (might still be healthy from before) + } } #[cfg(test)] @@ -211,6 +231,7 @@ mod tests { check_ok, is_healthy: false, lag: 0, + consecutive_failures: 0, } } @@ -219,7 +240,7 @@ mod tests { let mut node = make_el_node("test", 1000, true); let chain_head = 1005; - calculate_el_health(&mut node, chain_head, 10); + calculate_el_health(&mut node, chain_head, 10, 3); assert_eq!(node.lag, 5, "Lag should be chain_head - block_number"); } @@ -230,7 +251,7 @@ mod tests { let chain_head = 1002; let max_lag = 5; - calculate_el_health(&mut node, chain_head, max_lag); + calculate_el_health(&mut node, chain_head, max_lag, 3); assert!( node.is_healthy, @@ -242,14 +263,32 @@ mod tests { #[test] fn test_el_node_unhealthy_exceeds_lag() { let mut node = make_el_node("test", 990, true); + node.is_healthy = true; // Start as healthy let chain_head = 1000; let max_lag = 5; - calculate_el_health(&mut node, chain_head, max_lag); + // First failure - still healthy (threshold is 3) + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + + // Second failure - still healthy + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + // Third failure - now unhealthy + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 3); assert!( !node.is_healthy, - "Node should be unhealthy when lag > max_lag" + "Node should be unhealthy after 3 failures" ); assert_eq!(node.lag, 10); } @@ -260,7 +299,7 @@ mod tests { let chain_head = 1000; let max_lag = 5; - calculate_el_health(&mut node, chain_head, max_lag); + calculate_el_health(&mut node, chain_head, max_lag, 3); assert!( node.is_healthy, @@ -272,18 +311,69 @@ mod tests { #[test] fn test_el_node_unhealthy_when_check_fails() { let mut node = make_el_node("test", 1000, false); // check_ok = false + node.is_healthy = true; // Start as healthy let chain_head = 1000; let max_lag = 5; - calculate_el_health(&mut node, chain_head, max_lag); + // First failure + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + + // Second failure + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + // Third failure + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 3); assert!( !node.is_healthy, - "Node should be unhealthy when check_ok is false" + "Node should be unhealthy after 3 failures" ); assert_eq!(node.lag, 0); } + #[test] + fn test_el_node_consecutive_failures_reset_on_recovery() { + let mut node = make_el_node("test", 990, true); + node.is_healthy = true; // Start as healthy + let chain_head = 1000; + let max_lag = 5; + + // First failure - still healthy + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 1); + assert!( + node.is_healthy, + "Node should still be healthy after 1 failure" + ); + + // Second failure - still healthy + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!(node.consecutive_failures, 2); + assert!( + node.is_healthy, + "Node should still be healthy after 2 failures" + ); + + // Recovery - node catches up + node.block_number = 1000; + calculate_el_health(&mut node, chain_head, max_lag, 3); + assert_eq!( + node.consecutive_failures, 0, + "Consecutive failures should reset on success" + ); + assert!(node.is_healthy, "Node should be healthy after recovery"); + } + // ========================================================================= // update_el_chain_head tests // ========================================================================= diff --git a/src/monitor.rs b/src/monitor.rs index fda7dc4..8b05183 100644 --- a/src/monitor.rs +++ b/src/monitor.rs @@ -80,7 +80,12 @@ pub async fn check_all_el_nodes(state: &Arc) -> bool { let mut healthy_count = 0u64; for node in el_nodes.iter_mut() { - el::calculate_el_health(node, chain_head, state.max_el_lag); + el::calculate_el_health( + node, + chain_head, + state.max_el_lag, + state.health_check_max_failures, + ); if node.is_primary && node.is_healthy { any_primary_healthy = true; @@ -102,6 +107,7 @@ pub async fn check_all_el_nodes(state: &Arc) -> bool { block_number = node.block_number, check_ok = node.check_ok, lag = node.lag, + consecutive_failures = node.consecutive_failures, is_healthy = node.is_healthy, "EL node health calculated" ); @@ -163,7 +169,12 @@ pub async fn check_all_cl_nodes(state: &Arc) { let mut healthy_count = 0u64; for node in cl_nodes.iter_mut() { - cl::calculate_cl_health(node, chain_head, state.max_cl_lag); + cl::calculate_cl_health( + node, + chain_head, + state.max_cl_lag, + state.health_check_max_failures, + ); if node.is_healthy { healthy_count += 1; @@ -179,6 +190,7 @@ pub async fn check_all_cl_nodes(state: &Arc) { slot = node.slot, health_ok = node.health_ok, lag = node.lag, + consecutive_failures = node.consecutive_failures, is_healthy = node.is_healthy, "CL node health calculated" ); diff --git a/src/proxy/http.rs b/src/proxy/http.rs index 32653fe..20ca312 100644 --- a/src/proxy/http.rs +++ b/src/proxy/http.rs @@ -347,6 +347,7 @@ mod tests { max_cl_lag: 3, proxy_timeout_ms: 30000, max_retries: 2, + health_check_max_failures: 3, }) } @@ -360,6 +361,7 @@ mod tests { check_ok: is_healthy, is_healthy, lag: 0, + consecutive_failures: 0, } } @@ -371,6 +373,7 @@ mod tests { health_ok: is_healthy, is_healthy, lag: 0, + consecutive_failures: 0, } } diff --git a/src/proxy/selection.rs b/src/proxy/selection.rs index 4270faa..73c0631 100644 --- a/src/proxy/selection.rs +++ b/src/proxy/selection.rs @@ -44,6 +44,7 @@ mod tests { check_ok: is_healthy, is_healthy, lag: 0, + consecutive_failures: 0, } } @@ -56,6 +57,7 @@ mod tests { health_ok: is_healthy, is_healthy, lag: 0, + consecutive_failures: 0, } } diff --git a/src/proxy/ws.rs b/src/proxy/ws.rs index 93ec71f..6128edc 100644 --- a/src/proxy/ws.rs +++ b/src/proxy/ws.rs @@ -740,6 +740,7 @@ mod tests { max_cl_lag: 3, proxy_timeout_ms: 30000, max_retries: 2, + health_check_max_failures: 3, }) } @@ -753,6 +754,7 @@ mod tests { check_ok: is_healthy, is_healthy, lag: 0, + consecutive_failures: 0, } } diff --git a/src/state.rs b/src/state.rs index 060fae7..ecc03cd 100644 --- a/src/state.rs +++ b/src/state.rs @@ -26,6 +26,8 @@ pub struct ElNodeState { pub is_healthy: bool, /// Current lag from chain head (in blocks) pub lag: u64, + /// Number of consecutive health check failures + pub consecutive_failures: u32, } impl ElNodeState { @@ -40,6 +42,7 @@ impl ElNodeState { check_ok: false, // Start with check not ok is_healthy: false, // Start unhealthy until health check passes lag: 0, + consecutive_failures: 0, } } } @@ -59,6 +62,8 @@ pub struct ClNodeState { pub is_healthy: bool, /// Current lag from chain head (in slots) pub lag: u64, + /// Number of consecutive health check failures + pub consecutive_failures: u32, } impl ClNodeState { @@ -71,6 +76,7 @@ impl ClNodeState { health_ok: false, // Start with health not ok is_healthy: false, // Start unhealthy until health check passes lag: 0, + consecutive_failures: 0, } } } @@ -96,6 +102,8 @@ pub struct AppState { pub proxy_timeout_ms: u64, /// Maximum number of retry attempts pub max_retries: u32, + /// Number of consecutive health check failures before marking node as unhealthy + pub health_check_max_failures: u32, } impl AppState { @@ -127,6 +135,7 @@ impl AppState { max_cl_lag: config.global.max_cl_lag_slots, proxy_timeout_ms: config.global.proxy_timeout_ms, max_retries: config.global.max_retries, + health_check_max_failures: config.global.health_check_max_failures, } } } diff --git a/tests/steps/cl_health_steps.rs b/tests/steps/cl_health_steps.rs index dd8e935..1fb13bf 100644 --- a/tests/steps/cl_health_steps.rs +++ b/tests/steps/cl_health_steps.rs @@ -22,6 +22,7 @@ fn make_cl_node(name: &str, slot: u64, health_ok: bool) -> ClNodeState { health_ok, is_healthy: false, lag: 0, + consecutive_failures: 0, } } @@ -101,7 +102,7 @@ fn when_health_check_runs(world: &mut VixyWorld) { // Calculate health for each EL node for node in world.el_nodes.iter_mut() { - calculate_el_health(node, world.el_chain_head, world.max_el_lag); + calculate_el_health(node, world.el_chain_head, world.max_el_lag, 3); } } @@ -114,7 +115,7 @@ fn when_health_check_runs(world: &mut VixyWorld) { // Calculate health for each CL node for node in world.cl_nodes.iter_mut() { - calculate_cl_health(node, world.cl_chain_head, world.max_cl_lag); + calculate_cl_health(node, world.cl_chain_head, world.max_cl_lag, 3); } } } diff --git a/tests/steps/el_health_steps.rs b/tests/steps/el_health_steps.rs index 1e0758d..76194eb 100644 --- a/tests/steps/el_health_steps.rs +++ b/tests/steps/el_health_steps.rs @@ -19,6 +19,7 @@ fn make_el_node(name: &str, block_number: u64, check_ok: bool) -> ElNodeState { check_ok, is_healthy: false, lag: 0, + consecutive_failures: 0, } }