feat(ws): Add health-aware WebSocket reconnection with subscription replay#3
Merged
pamungkaski merged 4 commits intomainfrom Jan 15, 2026
Merged
Conversation
…eplay Implement automatic WebSocket reconnection when upstream EL nodes become unhealthy. The proxy now monitors node health and seamlessly switches to healthy nodes while preserving client subscriptions. Key features: - SubscriptionTracker to track active eth_subscribe requests - Health monitor task that polls node health every second - Automatic reconnection with subscription replay on new upstream - Subscription ID translation to preserve client-facing IDs - Comprehensive metrics for connections, reconnections, and subscriptions New metrics added: - ws_connections_active/total: Track active and lifetime connections - ws_messages_total: Count messages by direction (upstream/downstream) - ws_reconnections_total: Successful reconnection count - ws_reconnection_attempts_total: Attempts by status (success/failed) - ws_subscriptions_active/total: Track active subscriptions - ws_upstream_node: Current upstream node indicator Integration tests added for WebSocket failover scenarios. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add metrics recording in the health monitor to ensure metrics are populated from startup. This fixes the Prometheus metrics integration test which was failing because no metrics were being recorded. Metrics now recorded in monitor: - EL/CL chain head values - Per-node block/slot numbers, lag, and health status - Healthy node counts for EL and CL - Failover status and counter Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collapse nested if statements in WebSocket proxy code to satisfy clippy's collapsible_if lint. Uses Rust's let-chain syntax to combine conditions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The el_requests_total, el_request_duration_seconds, cl_requests_total, and cl_request_duration_seconds metrics were defined but never called. Now both el_proxy_handler and cl_proxy_handler record: - Request count (incremented on each request) - Request duration (histogram observation) EL requests also include the tier label (primary/backup). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
merklefruit
reviewed
Jan 15, 2026
| - Subscription ID translation is essential for seamless client experience | ||
| - Rust's type system (clippy) encourages clean abstractions via type aliases | ||
|
|
||
| **Mood:** Accomplished - this was a significant feature addition with real-world value! |
Contributor
There was a problem hiding this comment.
Claude feels accomplished, nice!
Collaborator
Author
There was a problem hiding this comment.
we are closer to AGI
pamungkaski
added a commit
that referenced
this pull request
Jan 23, 2026
…rovements Add comprehensive documentation for production incident investigation and fix plan: **Root Cause Analysis:** - Issue #2 (CRITICAL): Subscription replay responses break clients - Issue #5 (CRITICAL): Never switches back to primary after recovery - Issue #1: Messages lost during reconnection window - Issue #3: Health checks block without timeout **Documentation Added:** 1. WEBSOCKET-RECONNECTION-FIX.md - Complete root cause analysis (5 issues) - Design principles violated - 3-phase fix plan (P0/P1/P2) - Full TDD implementations with code - Rollout strategy and success metrics 2. TESTING-IMPROVEMENTS.md - Analysis of 8 critical test gaps - Why existing tests missed production issues - 4-phase test improvement plan - Complete test implementations (integration, property-based, chaos, load) - CI/CD integration strategy 3. docs/PRODUCTION-INCIDENT-2026-01-23.md - Incident summary and timeline - Quick reference guide - Links to detailed documentation **Key Findings:** - Tests focused on happy paths, missed edge cases - No tests for regular requests after reconnection - No tests for multiple simultaneous subscriptions - No load/chaos testing to catch concurrency issues **Next Steps:** - Phase 0 (24h): Fix Issues #2 and #5 (critical hotfix) - Phase 1 (1wk): Add critical integration tests - Then implement remaining fixes and test improvements Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
pamungkaski
added a commit
that referenced
this pull request
Jan 23, 2026
Implemented remaining fixes from production incident timeline to eliminate lock starvation, improve health check performance, and prevent message loss. Issue #3 (Phase 1): Health check timeouts - Added 5-second timeouts to all HTTP clients in health checking - Prevents health checks from blocking indefinitely when nodes are unresponsive - Affected files: src/health/el.rs, src/health/cl.rs Issue #4 (Phase 2): Concurrent health checks - Refactored monitor to not hold write locks during I/O operations - Pattern: read lock → concurrent checks → write lock for updates - Uses futures_util::future::join_all for parallel execution - Eliminates lock contention between health monitor and WebSocket monitor - Reduces health check cycle time from O(n) to O(1) - Affected files: src/monitor.rs Issue #1 (Phase 1): Message queueing during reconnection - Added VecDeque message queue and AtomicBool reconnecting flag - Messages from clients queued during reconnection instead of being dropped - Queue replayed after successful reconnection - Prevents message loss during 2-5 second reconnection window - Affected files: src/proxy/ws.rs Testing: - All 88 unit tests pass - Clippy clean (no warnings) - Integration tests: 26 scenarios (25 passed, 1 skipped), 160 steps passed - Kurtosis: 23 scenarios, 144 steps - WSS: 3 scenarios, 16 steps Impact: - Eliminates lock starvation during health checks - Improves system responsiveness under load - Zero message loss during reconnection - Health checks timeout properly (5s max) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
pamungkaski
added a commit
that referenced
this pull request
Jan 23, 2026
Replaced external issue/finding references with self-explanatory comments that describe what the code does and why, without requiring access to session documentation. Changes: - Health checks: "Use timeout to prevent blocking" instead of "Issue #3 Fix" - Monitor: "Don't hold write lock during I/O to prevent lock contention" - WebSocket reconnection: Clear explanations of queueing, background tasks, replay - Subscription handling: "Replayed response from reconnection" instead of "Issue #2" Benefits: - Comments are self-contained and understandable without external context - Future developers don't need access to issue tracking - Code is more maintainable and easier to understand - Explains WHY not just reference ticket numbers All tests pass: 92/92 unit tests ✅ Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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
2 participants
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.
No description provided.