-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[automatic failover] Add SSL configuration support to LagAwareStrategy #4291
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
[automatic failover] Add SSL configuration support to LagAwareStrategy #4291
Conversation
src/test/java/redis/clients/jedis/scenario/LagAwareStrategySslIT.java
Outdated
Show resolved
Hide resolved
c551436 to
2d4599b
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
This PR adds SSL configuration support to LagAwareStrategy for secure communication with Redis Enterprise REST API, eliminating the need to modify the default Java truststore for custom certificates.
- Added SslOptions parameter to LagAwareStrategy.Config for custom SSL configuration
- Enhanced RedisRestAPI to support SSL context configuration and hostname verification modes
- Refactored REST endpoint utility to a shared class for better code reuse
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/redis/clients/jedis/SslOptions.java | Added getSslVerifyMode() method and fixed null keystore type handling |
| src/main/java/redis/clients/jedis/mcf/LagAwareStrategy.java | Integrated SSL options into Config and ConfigBuilder classes |
| src/main/java/redis/clients/jedis/mcf/RedisRestAPI.java | Added SSL context configuration for HTTPS connections |
| src/test/java/redis/clients/jedis/scenario/RestEndpointUtil.java | Extracted shared REST endpoint utility for code reuse |
| src/test/java/redis/clients/jedis/scenario/LagAwareStrategySslIT.java | Added comprehensive SSL integration tests |
| src/test/java/redis/clients/jedis/mcf/RedisRestAPIIntegrationTest.java | Updated to use shared RestEndpointUtil and improved error logging |
| pom.xml | Added test includes for integration test discovery |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
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.
LGTM
- use getters instead of fields
…ctive-lagware-ssl-config
…#4297) * [automatic failover] Remove the check for 'GenericObjectPool.getNumWaiters()' in 'TrackingConnectionPool' (#4270) - remove the check for number of waitiers in TrackingConnectionPool * [automatic failover] Configure max total connections for EchoStrategy (#4268) - set maxtotal connections for echoStrategy * [automatic failover] Replace 'CircuitBreaker' with 'Cluster' for 'CircuitBreakerFailoverBase.clusterFailover' (#4275) * - replace CircuitBreaker with Cluster for CircuitBreakerFailoverBase.clusterFailover - improve thread safety with provider initialization * - formatting * [automatic failover] Minor optimizations on fast failover (#4277) * - minor optimizations on fail fast * - volatile failfast * [automatic failover] Implement health check retries (#4273) * - replace minConsecutiveSuccessCount with numberOfRetries - add retries into healtCheckImpl - apply changes to strategy implementations config classes - fix unit tests * - fix typo * - fix failing tests * - add tests for retry logic * - formatting * - format * - revisit numRetries for healthCheck ,replace with numProbes and implement built in policies - new types probecontext, ProbePolicy, HealthProbeContext - add delayer executor pool to healthcheckımpl - adjustments on worker pool of healthCheckImpl for shared use of workers * - format * - expand comment with example case * - drop pooled executor for delays * - polish * - fix tests * - formatting * - checking failing tests * - fix test * - fix flaky tests * - fix flaky test * - add tests for builtin probing policies * - fix flaky test * [automatic failover] Move failover provider to mcf (#4294) * - move failover provider to mcf * - make iterateActiveCluster package private * [automatic failover] Add SSL configuration support to LagAwareStrategy (#4291) * User-provided ssl config for lag-aware health check * ssl scenario test for lag-aware healthcheck * format * format * address review comments - use getters instead of fields * [automatic failover] Implement max number of failover attempts (#4293) * - implement max failover attempt - add tests * - fix user receive the intended exception * -clean+format * - java doc for exceptions * format * - more tests on excaption types in max failover attempts mechanism * format * fix failing timing in test * disable health checks * rename to switchToHealthyCluster * format --------- Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com>
…ure rate) capabililty to circuit breaker (#4295) * [automatic failover] Remove the check for 'GenericObjectPool.getNumWaiters()' in 'TrackingConnectionPool' (#4270) - remove the check for number of waitiers in TrackingConnectionPool * [automatic failover] Configure max total connections for EchoStrategy (#4268) - set maxtotal connections for echoStrategy * [automatic failover] Replace 'CircuitBreaker' with 'Cluster' for 'CircuitBreakerFailoverBase.clusterFailover' (#4275) * - replace CircuitBreaker with Cluster for CircuitBreakerFailoverBase.clusterFailover - improve thread safety with provider initialization * - formatting * [automatic failover] Minor optimizations on fast failover (#4277) * - minor optimizations on fail fast * - volatile failfast * [automatic failover] Implement health check retries (#4273) * - replace minConsecutiveSuccessCount with numberOfRetries - add retries into healtCheckImpl - apply changes to strategy implementations config classes - fix unit tests * - fix typo * - fix failing tests * - add tests for retry logic * - formatting * - format * - revisit numRetries for healthCheck ,replace with numProbes and implement built in policies - new types probecontext, ProbePolicy, HealthProbeContext - add delayer executor pool to healthcheckımpl - adjustments on worker pool of healthCheckImpl for shared use of workers * - format * - expand comment with example case * - drop pooled executor for delays * - polish * - fix tests * - formatting * - checking failing tests * - fix test * - fix flaky tests * - fix flaky test * - add tests for builtin probing policies * - fix flaky test * [automatic failover] Move failover provider to mcf (#4294) * - move failover provider to mcf * - make iterateActiveCluster package private * [automatic failover] Add SSL configuration support to LagAwareStrategy (#4291) * User-provided ssl config for lag-aware health check * ssl scenario test for lag-aware healthcheck * format * format * address review comments - use getters instead of fields * [automatic failover] Implement max number of failover attempts (#4293) * - implement max failover attempt - add tests * - fix user receive the intended exception * -clean+format * - java doc for exceptions * format * - more tests on excaption types in max failover attempts mechanism * format * fix failing timing in test * disable health checks * rename to switchToHealthyCluster * format * - Add dual-threshold (min failures + failure rate) failover to circuit breaker executor - Map config to resilience4j via CircuitBreakerThresholdsAdapter - clean up/simplfy config: drop slow-call and window type - Add thresholdMinNumOfFailures; update some of the defaults - Update provider to use thresholds adapter - Update docs; align examples with new defaults - Add tests for 0% rate, edge thresholds * polish * Update src/main/java/redis/clients/jedis/mcf/CircuitBreakerThresholdsAdapter.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * - fix typo * - fix min total calls calculation * format * - merge issues fixed * fix javadoc ref * - move threshold evaluations to failoverbase - simplfy executer and cbfailoverconnprovider - adjust config getters - fix failing tests due to COUNT_BASED -> TIME_BASED - new tests for thresholds calculations and impact on circuit state transitions * - avoid facilitating actual CBConfig type in tests * Update src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Trigger workflows * - evaluate only in failure recorded and failover immediately - add more test on threshold calculations - enable command line arg for overwriting surefire.excludedGroups * format * check pom * - fix error prone test * [automatic failover] Set and test default values for failover config&components (#4298) * - set & test default values * - format * - fix tests failing due to changing defaults * - fix flaky test * - remove unnecessary checks for failover attempt * - clean and trim adapter class - add docs and more explanantion * fix javadoc issue * - switch to all_succes to fix flaky timing * - fix issue in CircuitBreakerFailoverConnectionProvider * introduce ReflectionTestUtil --------- Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…4306) * [automatic failover] Set and test default values for failover config&components (#4298) * - set & test default values * - format * - fix tests failing due to changing defaults * [automatic failover] Add dual thresholds (min num of failures + failure rate) capabililty to circuit breaker (#4295) * [automatic failover] Remove the check for 'GenericObjectPool.getNumWaiters()' in 'TrackingConnectionPool' (#4270) - remove the check for number of waitiers in TrackingConnectionPool * [automatic failover] Configure max total connections for EchoStrategy (#4268) - set maxtotal connections for echoStrategy * [automatic failover] Replace 'CircuitBreaker' with 'Cluster' for 'CircuitBreakerFailoverBase.clusterFailover' (#4275) * - replace CircuitBreaker with Cluster for CircuitBreakerFailoverBase.clusterFailover - improve thread safety with provider initialization * - formatting * [automatic failover] Minor optimizations on fast failover (#4277) * - minor optimizations on fail fast * - volatile failfast * [automatic failover] Implement health check retries (#4273) * - replace minConsecutiveSuccessCount with numberOfRetries - add retries into healtCheckImpl - apply changes to strategy implementations config classes - fix unit tests * - fix typo * - fix failing tests * - add tests for retry logic * - formatting * - format * - revisit numRetries for healthCheck ,replace with numProbes and implement built in policies - new types probecontext, ProbePolicy, HealthProbeContext - add delayer executor pool to healthcheckımpl - adjustments on worker pool of healthCheckImpl for shared use of workers * - format * - expand comment with example case * - drop pooled executor for delays * - polish * - fix tests * - formatting * - checking failing tests * - fix test * - fix flaky tests * - fix flaky test * - add tests for builtin probing policies * - fix flaky test * [automatic failover] Move failover provider to mcf (#4294) * - move failover provider to mcf * - make iterateActiveCluster package private * [automatic failover] Add SSL configuration support to LagAwareStrategy (#4291) * User-provided ssl config for lag-aware health check * ssl scenario test for lag-aware healthcheck * format * format * address review comments - use getters instead of fields * [automatic failover] Implement max number of failover attempts (#4293) * - implement max failover attempt - add tests * - fix user receive the intended exception * -clean+format * - java doc for exceptions * format * - more tests on excaption types in max failover attempts mechanism * format * fix failing timing in test * disable health checks * rename to switchToHealthyCluster * format * - Add dual-threshold (min failures + failure rate) failover to circuit breaker executor - Map config to resilience4j via CircuitBreakerThresholdsAdapter - clean up/simplfy config: drop slow-call and window type - Add thresholdMinNumOfFailures; update some of the defaults - Update provider to use thresholds adapter - Update docs; align examples with new defaults - Add tests for 0% rate, edge thresholds * polish * Update src/main/java/redis/clients/jedis/mcf/CircuitBreakerThresholdsAdapter.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * - fix typo * - fix min total calls calculation * format * - merge issues fixed * fix javadoc ref * - move threshold evaluations to failoverbase - simplfy executer and cbfailoverconnprovider - adjust config getters - fix failing tests due to COUNT_BASED -> TIME_BASED - new tests for thresholds calculations and impact on circuit state transitions * - avoid facilitating actual CBConfig type in tests * Update src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Trigger workflows * - evaluate only in failure recorded and failover immediately - add more test on threshold calculations - enable command line arg for overwriting surefire.excludedGroups * format * check pom * - fix error prone test * [automatic failover] Set and test default values for failover config&components (#4298) * - set & test default values * - format * - fix tests failing due to changing defaults * - fix flaky test * - remove unnecessary checks for failover attempt * - clean and trim adapter class - add docs and more explanantion * fix javadoc issue * - switch to all_succes to fix flaky timing * - fix issue in CircuitBreakerFailoverConnectionProvider * introduce ReflectionTestUtil --------- Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [automatic failover] feat: Add MultiDbClient with multi-endpoint failover and circuit breaker support (#4300) * feat: introduce ResilientRedisClient with multi-endpoint failover support Add ResilientRedisClient extending UnifiedJedis with automatic failover capabilities across multiple weighted Redis endpoints. Includes circuit breaker pattern, health monitoring, and configurable retry logic for high-availability Redis deployments. * format * mark ResilientRedisClientTest as integration one * fix test - make sure endpoint is healthy before activating it * Rename ResilientClient to align with design - ResilientClient -> MultiDbClient (builder, tests, etc) * Rename setActiveEndpoint to setActiveDatabaseEndpoint * Rename clusterSwitchListener to databaseSwitchListener * Rename multiClusterConfig to multiDbConfig * fix api doc's error * fix compilation error after rebase * format * fix example in javadoc * Update ActiveActiveFailoverTest scenariou test to use builder's # Conflicts: # src/test/java/redis/clients/jedis/scenario/ActiveActiveFailoverTest.java * rename setActiveDatabaseEndpoint -. setActiveDatabase * is healthy throw exception if cluster does not exists * format * [automatic failover]Use Endpoint interface instead HostAndPort in multi db (#4302) [clean up] Use Endpoint interface where possible * - fix variable name type * fix typo in variable name * - fix flaky test --------- Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ure rate) capabililty to circuit breaker (#4295) * [automatic failover] Remove the check for 'GenericObjectPool.getNumWaiters()' in 'TrackingConnectionPool' (#4270) - remove the check for number of waitiers in TrackingConnectionPool * [automatic failover] Configure max total connections for EchoStrategy (#4268) - set maxtotal connections for echoStrategy * [automatic failover] Replace 'CircuitBreaker' with 'Cluster' for 'CircuitBreakerFailoverBase.clusterFailover' (#4275) * - replace CircuitBreaker with Cluster for CircuitBreakerFailoverBase.clusterFailover - improve thread safety with provider initialization * - formatting * [automatic failover] Minor optimizations on fast failover (#4277) * - minor optimizations on fail fast * - volatile failfast * [automatic failover] Implement health check retries (#4273) * - replace minConsecutiveSuccessCount with numberOfRetries - add retries into healtCheckImpl - apply changes to strategy implementations config classes - fix unit tests * - fix typo * - fix failing tests * - add tests for retry logic * - formatting * - format * - revisit numRetries for healthCheck ,replace with numProbes and implement built in policies - new types probecontext, ProbePolicy, HealthProbeContext - add delayer executor pool to healthcheckımpl - adjustments on worker pool of healthCheckImpl for shared use of workers * - format * - expand comment with example case * - drop pooled executor for delays * - polish * - fix tests * - formatting * - checking failing tests * - fix test * - fix flaky tests * - fix flaky test * - add tests for builtin probing policies * - fix flaky test * [automatic failover] Move failover provider to mcf (#4294) * - move failover provider to mcf * - make iterateActiveCluster package private * [automatic failover] Add SSL configuration support to LagAwareStrategy (#4291) * User-provided ssl config for lag-aware health check * ssl scenario test for lag-aware healthcheck * format * format * address review comments - use getters instead of fields * [automatic failover] Implement max number of failover attempts (#4293) * - implement max failover attempt - add tests * - fix user receive the intended exception * -clean+format * - java doc for exceptions * format * - more tests on excaption types in max failover attempts mechanism * format * fix failing timing in test * disable health checks * rename to switchToHealthyCluster * format * - Add dual-threshold (min failures + failure rate) failover to circuit breaker executor - Map config to resilience4j via CircuitBreakerThresholdsAdapter - clean up/simplfy config: drop slow-call and window type - Add thresholdMinNumOfFailures; update some of the defaults - Update provider to use thresholds adapter - Update docs; align examples with new defaults - Add tests for 0% rate, edge thresholds * polish * Update src/main/java/redis/clients/jedis/mcf/CircuitBreakerThresholdsAdapter.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * - fix typo * - fix min total calls calculation * format * - merge issues fixed * fix javadoc ref * - move threshold evaluations to failoverbase - simplfy executer and cbfailoverconnprovider - adjust config getters - fix failing tests due to COUNT_BASED -> TIME_BASED - new tests for thresholds calculations and impact on circuit state transitions * - avoid facilitating actual CBConfig type in tests * Update src/test/java/redis/clients/jedis/failover/FailoverIntegrationTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Trigger workflows * - evaluate only in failure recorded and failover immediately - add more test on threshold calculations - enable command line arg for overwriting surefire.excludedGroups * format * check pom * - fix error prone test * [automatic failover] Set and test default values for failover config&components (#4298) * - set & test default values * - format * - fix tests failing due to changing defaults * - fix flaky test * - remove unnecessary checks for failover attempt * - clean and trim adapter class - add docs and more explanantion * fix javadoc issue * - switch to all_succes to fix flaky timing * - fix issue in CircuitBreakerFailoverConnectionProvider * introduce ReflectionTestUtil --------- Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Problem:
LagAwareStrategy health checks to Redis Enterprise REST API (port 9443) required adding certificates to the default Java truststore for custom certificates, self-signed certificates. This approach is not ideal for application-specific SSL configuration.
Solution:
LagAwareStrategy.ConfigbuilderUsage:
Closes #4292