[automatic failover] Implement failback with given grace period #4204
Conversation
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
- 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
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic failback functionality with a configurable grace period for the multi-cluster Redis client. The implementation replaces index-based cluster management with endpoint-based management and adds periodic failback checks that can automatically switch back to higher-weight clusters after they recover from failures.
- Replaces index-based cluster selection with endpoint-based management using weights
- Implements periodic failback mechanism with configurable grace periods for unhealthy clusters
- Adds comprehensive health check system with multiple strategy implementations
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| MultiClusterPooledConnectionProvider.java | Core refactoring from index-based to endpoint-based cluster management with failback support |
| MultiClusterClientConfig.java | Added configuration for failback intervals, grace periods, and health check strategies |
| HealthCheck*.java | New health check framework with status management and strategy pattern implementation |
| Test files | Updated tests to use new endpoint-based API and added comprehensive failback mechanism tests |
Comments suppressed due to low confidence (5)
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java:123
- Test method name 'testSetActiveMultiClusterIndexEqualsZero' is inconsistent with the new endpoint-based API. Should be renamed to 'testSetActiveClusterWithNull' to reflect the actual functionality being tested.
public void testSetActiveMultiClusterIndexEqualsZero() {
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java:129
- Test method name 'testSetActiveMultiClusterIndexLessThanZero' is inconsistent with the new endpoint-based API. Should be renamed to 'testSetActiveClusterWithNullDuplicate' or similar to reflect the actual functionality being tested.
public void testSetActiveMultiClusterIndexLessThanZero() {
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java:135
- Test method name 'testSetActiveMultiClusterIndexOutOfRange' is inconsistent with the new endpoint-based API. Should be renamed to 'testSetActiveClusterWithNonExistentEndpoint' to reflect the actual functionality being tested.
public void testSetActiveMultiClusterIndexOutOfRange() {
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java
Show resolved
Hide resolved
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java
Show resolved
Hide resolved
src/test/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProviderTest.java
Show resolved
Hide resolved
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.
This PR is based on changes in previous #4200.
Commits essential to this one are;