-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[automatic failover] Implement wait on healthCheck results during client init #4207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/automatic-failover
Are you sure you want to change the base?
[automatic failover] Implement wait on healthCheck results during client init #4207
Conversation
- Healtstatus manager with initial listener and registration logic - pluggable health checker strategy introduced, these are draft NoOpStrategy, EchoStrategy, LagAwareStrategy, - fix failing tests impacted from weighted clusters
- add echo ot CommandObjects and UnifiedJEdis - improve StrategySupplier by accepting jedisclientconfig - adapt EchoStrategy to StrategySupplier. Now it handles the creation of connection by accepting endpoint and JedisClientConfig - make healthchecks disabled by default - drop noOpStrategy - add unit&integration tests for health check
- clear redundant catch - replace failover options and drop failoveroptions class - remove forced_unhealthy from healthstatus - fix failback check - add disabled flag to cluster - update/fix related tests
Co-authored-by: Copilot <[email protected]>
- replace failback enabled with failbacksupported in client - fix formatting - set defaults
- fix failing tests
- fix failing tests
- introduce graceperiod - fix issue when CB is forced_open and gracePeriod is completed
… results during consturction of provider - add HealthStatus.UNKNOWN as default for Cluster - handle status changes in order of events during initialization - add tests for status tracker and orderingof events - fix impacted unit&integ tests
- fix formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements automatic failover with wait on health check results during client initialization. It introduces status tracking to wait for initial health check results and adds significant new functionality for health monitoring and automatic cluster management.
- Adds comprehensive health check system with StatusTracker for waiting on initial health check results
- Implements weighted cluster selection and automatic failback mechanisms with grace periods
- Replaces index-based cluster management with endpoint-based approach for better flexibility
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
MultiClusterPooledConnectionProvider.java | Core provider changes - adds health status management, weight-based selection, and failback scheduling |
StatusTracker.java | New class for event-driven health status waiting during initialization |
HealthStatusManager.java | New health status management system with listener support |
HealthCheck.java | Health check execution with timeout and callback mechanisms |
EchoStrategy.java | Default health check strategy using Redis ECHO command |
MultiClusterClientConfig.java | Configuration updates for weights, health checks, and failback settings |
Various test files | Comprehensive test coverage for new health check and failback functionality |
Comments suppressed due to low confidence (1)
src/test/java/redis/clients/jedis/mcf/StatusTrackerTest.java:178
- The test expects a JedisConnectionException but doesn't verify the specific exception type or message. Should use assertThrows() for better exception verification.
fail("Should have thrown JedisConnectionException due to interrupt");
src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java
Show resolved
Hide resolved
src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java
Outdated
Show resolved
Hide resolved
- downgrade logback version for slf4j compatibility - increase timeouts for faultInjector
…MultiClusterPooledConnectionProvider - add test for init and post init events - fix failing tests
- fix failing tests due to method name change
This PR is based on changes in previous #4204.
Summary of changes in PR;
Commits essential to this one are;
- introduce StatusTracker with purpose of waiting initial healthcheck…
- introduce forceActiveCluster by duration
- fix failing tests by waiting on clusters to get healthy
- fix failing scenario test
- adressing reviews and feedback