[automatic failover] Implement add/remove endpoints #4200
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
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the multi-cluster failover provider to support dynamic addition and removal of endpoints, replaces the integer-based cluster index API with an Endpoint-based API, and wires in health‐check strategies with weighted failover.
- Switch from
activeMultiClusterIndextosetActiveCluster/iterateActiveClusterusingEndpointkeys. - Introduce
add()andremove()onMultiClusterPooledConnectionProviderwith health checks and weighted selection. - Update existing tests to use the new API and add coverage for dynamic endpoint management.
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java | Core provider: added add/remove, replaced index-based API, integrated health checks and weighted logic |
| src/main/java/redis/clients/jedis/mcf/HealthCheck.java | New scheduled health‐check task and listener notification |
| src/main/java/redis/clients/jedis/mcf/HealthStatusManager.java | Manages registration and notification of health‐status listeners |
| src/main/java/redis/clients/jedis/mcf/FailoverOptions.java | Builder for retry, weight, failback, and health‐check strategy options |
| src/main/java/redis/clients/jedis/mcf/EchoStrategy.java | Default HealthCheckStrategy using Redis ECHO |
| src/main/java/redis/clients/jedis/mcf/LagAwareStrategy.java | New strategy querying a REST API for DB availability |
| src/main/java/redis/clients/jedis/mcf/RedisRestAPIHelper.java | Helper for calling external REST endpoints |
| src/main/java/redis/clients/jedis/mcf/Endpoint.java | Interface abstraction for host/port endpoints |
| src/main/java/redis/clients/jedis/UnifiedJedis.java | Removed deprecated constructor, added echo command |
| src/main/java/redis/clients/jedis/CommandObjects.java | Added command object for echo |
| src/test/java/redis/clients/jedis/** | Updated tests to use iterateActiveCluster, setActiveCluster, and added dynamic endpoint tests |
Comments suppressed due to low confidence (1)
src/main/java/redis/clients/jedis/mcf/RedisRestAPIHelper.java:36
- [nitpick] Public method
getBdbsis missing Javadoc describing its behavior, expected response format, and error conditions. Consider adding a brief comment.
public List<String> getBdbs() throws IOException {
src/main/java/redis/clients/jedis/providers/MultiClusterPooledConnectionProvider.java
Show resolved
Hide resolved
- 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
atakavci
force-pushed
the
ali/aa-failover-addEndpoints
branch
from
93732b8 to
55adf47
Compare
Co-authored-by: Copilot <[email protected]>
- replace failback enabled with failbacksupported in client - fix formatting - set defaults
- fix failing tests
atakavci
force-pushed
the
ali/aa-failover-addEndpoints
branch
from
55adf47 to
ddcec73
Compare
- fix failing tests
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 #4189.
Commits essential to this one are;
- add remove endpoints
- replace cluster disabled with failbackCandidate
- remove failback candidate
- fix remove logic