-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[automatic failover] Refactor MultiDb API: Rename endpoint methods to database and encapsulate retry/circuit breaker configuration #4310
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
Conversation
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 refactors the MultiDb API to improve clarity and maintainability through two main changes: renaming endpoint-related methods to use database-centric terminology, and encapsulating retry and circuit breaker configuration into dedicated nested config classes.
- Renamed all endpoint-related methods to use "database" terminology (e.g.,
addEndpoint()→addDatabase()) - Encapsulated retry configuration into a dedicated
RetryConfignested class with builder pattern - Encapsulated circuit breaker configuration into a dedicated
CircuitBreakerConfignested class with builder pattern
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/redis/clients/jedis/scenario/ActiveActiveFailoverTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/misc/AutomaticFailoverTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/mcf/MultiDbConnectionProviderTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/mcf/MultiDbCircuitBreakerThresholdsTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/mcf/HealthCheckIntegrationTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/mcf/DefaultValuesTest.java | Updated test to verify default values through new API structure |
| src/test/java/redis/clients/jedis/mcf/DatabaseEvaluateThresholdsTest.java | Updated test to use new nested config builders |
| src/test/java/redis/clients/jedis/mcf/ActiveActiveLocalFailoverTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java | Updated test to use new database API and nested config builders |
| src/test/java/redis/clients/jedis/MultiDbClientTest.java | Updated test method names and API calls to use database terminology |
| src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java | Added helper methods for building Resilience4j configs and updated to use new API |
| src/main/java/redis/clients/jedis/mcf/CircuitBreakerThresholdsAdapter.java | Updated to use new nested config structure |
| src/main/java/redis/clients/jedis/builders/MultiDbClientBuilder.java | Updated javadoc examples to use new API |
| src/main/java/redis/clients/jedis/MultiDbConfig.java | Added nested RetryConfig and CircuitBreakerConfig classes with builders |
| src/main/java/redis/clients/jedis/MultiDbClient.java | Renamed methods and updated javadoc to use database terminology |
| docs/failover.md | Updated documentation examples to use new API |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/main/java/redis/clients/jedis/builders/MultiDbClientBuilder.java
Outdated
Show resolved
Hide resolved
- MultiDbClient::addEndpoint -> MultiDbClient::addDatabase
- MultiDbClient::forceActiveEndpoint -> MultiDbClient::forceActiveDatabase
- MultiDbClient::removeEndpoint -> MultiDbClient::removeDatabase
- MultiDbClient::getActiveEndpoint → MultiDbClient::getActiveDatabaseEndpoint
- MultiDbClient::getEndpoints → MultiDbClient::getDatabaseEndpoints
- endpoint(DatabaseConfig databaseConfig) → database(DatabaseConfig databaseConfig) - endpoint(Endpoint endpoint, float weight, JedisClientConfig clientConfig) → database(Endpoint endpoint, float weight, JedisClientConfig clientConfig)
- Related retry settings grouped together
- RetryConfig class created with builder pattern
Example usage
MultiDbConfig.RetryConfig.builder()
.maxAttempts(5)
.waitDuration(1000)
.exponentialBackoffMultiplier(2)
.includedExceptionList(Arrays.asList(JedisConnectionException.class))
.build())
- Related cb settings grouped together
Example usage
.failureDetector(MultiDbConfig.CircuitBreakerConfig.builder()
.slidingWindowSize(3)
.minNumOfFailures(2)
.failureRateThreshold(50f)
.build())
fb4b4b1 to
84712d7
Compare
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
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/main/java/redis/clients/jedis/mcf/MultiDbConnectionProvider.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Test Results 283 files ±0 283 suites ±0 11m 35s ⏱️ +2s Results for commit 7f327e6. ± Comparison against base commit 14356c4. This pull request removes 21 and adds 21 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
run scenario tests |
|
atakavci
left a comment
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.
as i understand these changes will go into GA, so lets replace as much as we can
Co-authored-by: atakavci <a_takavci@yahoo.com>
Co-authored-by: atakavci <a_takavci@yahoo.com>
atakavci
left a comment
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.
LGTM!
Summary
This PR refactors the MultiDb API to improve clarity and maintainability through two major changes:
Changes
1. Endpoint → Database Terminology (Commits: "Rename MultiDbClient endpoint related methods", "Methods Renamed in MultiDbConfig.Builder")
Renamed all endpoint-related methods to use "database" terminology for better clarity:
MultiDbClient methods:
addEndpoint()→addDatabase()removeEndpoint()→removeDatabase()forceActiveEndpoint()→forceActiveDatabase()getActiveEndpoint()→getActiveDatabaseEndpoint()getEndpoints()→getDatabaseEndpoints()MultiDbConfig.Builder methods:
endpoint()→database()(both overloaded variants)2. RetryConfig Encapsulation
MultiDbConfig.RetryConfignested class with builder patternretryMaxAttempts→RetryConfig.maxAttemptsretryWaitDuration→RetryConfig.waitDurationretryWaitDurationExponentialBackoffMultiplier→RetryConfig.exponentialBackoffMultiplierretryIncludedExceptionList→RetryConfig.includedExceptionListretryIgnoreExceptionList→RetryConfig.ignoreExceptionListcommandRetryproperty withcommandRetry(RetryConfig)builder method3. CircuitBreakerConfig Encapsulation
MultiDbConfig.CircuitBreakerConfignested class with builder patterncircuitBreakerFailureRateThreshold→CircuitBreakerConfig.failureRateThresholdcircuitBreakerSlidingWindowSize→CircuitBreakerConfig.slidingWindowSizecircuitBreakerMinNumOfFailures→CircuitBreakerConfig.minNumOfFailurescircuitBreakerIncludedExceptionList→CircuitBreakerConfig.includedExceptionListcircuitBreakerIgnoreExceptionList→CircuitBreakerConfig.ignoreExceptionListfailureDetectorproperty withfailureDetector(CircuitBreakerConfig)builder method4. Implementation Updates
MultiDbConnectionProviderwith helper methods:buildRetryConfig()- converts Jedis RetryConfig to Resilience4j RetryConfigbuildCircuitBreakerConfig()- converts Jedis CircuitBreakerConfig to Resilience4j CircuitBreakerConfigCircuitBreakerThresholdsAdapterto use new API5. Documentation Updates
docs/failover.mdwith new API examples6. Test Updates
API Changes
Endpoint → Database Renaming
Before:
After:
Configuration changes
Before
After (New API):