Conversation
There was a problem hiding this comment.
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.
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.
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>
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):