[automatic failover] Merge automatic failover (DO NOT MERGE) #3576
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
…ic-failover feature (#3507) * - improve extensbility that will needed in aa-failover feature * - suppresswarnings and remove casting
* - draft implementation for automatic-failover * - remove commented out tests * - format * - fix failing test * - fix flaky test * - fix multidbpusub subscriptions handover test * - wait for subscriptions with failing test
…ent (#3513) * - move BaseRedisClient to core package and add it to AbstractRedisClient * - add override annotations to AbstractRedisClient
* - Add/Remove databases safely * - secure switchToDatabase * - guard listeners and db switch against race conditions. * feedbacks from @ggivo - add close to both MultiDbConnection and CircuitBreaker - skip switchToDatabase when source and destination is same db * - add test around attempt to switch to same db
…er (#3522) * - simplfy tracking exceptions check - add metrics evaluation tests for double-threshold - add more tests on CB evaluates metrics and state transition, including edge cases * - tune number of success/failures in test case * - Add recordResult(Throwable), recordSuccess(), and recordFailure() public methods to CircuitBreaker - Add getSnapshot() public method to expose metrics directly - Change getMetrics() to package-private (internal use only) - Simplify handleFailure() in endpoint implementations to use recordResult() - Update all tests to use new public API - Drop repeating test case shouldOpenImmediatelyWhenMinimumCountReachedAndRateIsZero * - fix test cases; drop unnecessary calls to evaluateMetrics when there is call to recordFailure
…3521) * abstract clock for easy testing * Improve LockFreeSlidingWindowMetrics: fix bugs and add tests Bug Fixes: - Fix: Ensure snapshot metrics remain accurate after a full window rotation - Fix: events recorded exactly at bucket boundaries were miscounted - Enforce window size % bucket size == 0 - Move LockFreeSlidingWindowMetricsUnitTests to correct package (io.lettuce.core.failover.metrics) * remove unused reset methods * extract interface for MetricsSnapshot - remove snapshotTime - not used & not correctly calcualted - remove reset metrics - unused as of now * add LockFreeSlidingWindowMetrics benchmark test * performance tests moved to metrics package * replace with port from reselience4j * update copyrights * format * clean up javadocs * clean up - fix incorrect javadoc - fix failing benchmark * [automatic failover] Hide failover metrics implementation - CircuitBreakerMetrics, MetricsSnapshot - public - metrics implementation details stay inside io.lettuce.core.failover.metrics - Update CircuitBreaker to obtain its metrics via CircuitBreakerMetricsFactory.createLockFree() * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * rename createLockFree -> createDefaultMetrics * address review comments by @atakavci - remove CircuitBreakerMetrics, CircuitBreakerMetricsImpl - rename SlidingWindowMetrics -> CircuitBreakerMetrics * format * Enforce min-window size of 2 buckets Current implementation requires at least 2 buckets window With windowSize=1, only one node is created with next=null When updateWindow() advances the window it sets HEAD to headNext, which is null for a single-node window On the next call to updateWindow(), tries to access head.next but head is now null, causing: NullPointerException: Cannot read field "next" because "head" is null * Clean-up benchmark - benchmark matrix threads (1,4) window_size ("2", "30", "180") - performs 1_000_000 ops in simulated 5min test window - benchmark record events - benchmark record & read snapshot * remove MetricsPerformanceTests.java - no reliable way to assert on performance, instead added basic benchmark test to benchmark recording/snapshot reading average times - gc benchmarks are available for local testing * reset method removed * Apply suggestion from @Copilot Co-authored-by: Copilot <[email protected]> * Apply suggestion from @atakavci Co-authored-by: atakavci <[email protected]> * Update src/main/java/io/lettuce/core/failover/metrics/CircuitBreakerMetrics.java Co-authored-by: Tihomir Krasimirov Mateev <[email protected]> * add missing license header and javadoc * add missing license header and javadoc * correct author for jmh failover metrics --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: atakavci <[email protected]> Co-authored-by: Tihomir Krasimirov Mateev <[email protected]>
…uitBreaker state transitions (#3527) * abstract clock for easy testing * Improve LockFreeSlidingWindowMetrics: fix bugs and add tests Bug Fixes: - Fix: Ensure snapshot metrics remain accurate after a full window rotation - Fix: events recorded exactly at bucket boundaries were miscounted - Enforce window size % bucket size == 0 - Move LockFreeSlidingWindowMetricsUnitTests to correct package (io.lettuce.core.failover.metrics) * remove unused reset methods * extract interface for MetricsSnapshot - remove snapshotTime - not used & not correctly calcualted - remove reset metrics - unused as of now * add LockFreeSlidingWindowMetrics benchmark test * performance tests moved to metrics package * replace with port from reselience4j * update copyrights * format * clean up javadocs * clean up - fix incorrect javadoc - fix failing benchmark * [automatic failover] Hide failover metrics implementation - CircuitBreakerMetrics, MetricsSnapshot - public - metrics implementation details stay inside io.lettuce.core.failover.metrics - Update CircuitBreaker to obtain its metrics via CircuitBreakerMetricsFactory.createLockFree() * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * rename createLockFree -> createDefaultMetrics * address review comments by @atakavci - remove CircuitBreakerMetrics, CircuitBreakerMetricsImpl - rename SlidingWindowMetrics -> CircuitBreakerMetrics * format * Enforce min-window size of 2 buckets Current implementation requires at least 2 buckets window With windowSize=1, only one node is created with next=null When updateWindow() advances the window it sets HEAD to headNext, which is null for a single-node window On the next call to updateWindow(), tries to access head.next but head is now null, causing: NullPointerException: Cannot read field "next" because "head" is null * Clean-up benchmark - benchmark matrix threads (1,4) window_size ("2", "30", "180") - performs 1_000_000 ops in simulated 5min test window - benchmark record events - benchmark record & read snapshot * remove MetricsPerformanceTests.java - no reliable way to assert on performance, instead added basic benchmark test to benchmark recording/snapshot reading average times - gc benchmarks are available for local testing * reset method removed * reset circuit breaker metrics on state transition * fix test : shouldMaintainMetricsAfterSwitch() CB metrics are updated async on command completion, meaning waiting on command completion threads might proceed before metrics snapshot is updated. * format * evaluateMetrics - javadocs & make it package private * format --------- Co-authored-by: Copilot <[email protected]>
…d retry logic (CAE-1685) (#3541) * initial port Jedis health monitoring * wip integrate healthchecks * formating * formating * add test case plan * Endpoints without health checks configured should return HEALTHY Changes - add connection.getHealthStatus(RedisUri endpoint) - HEALTHY - returned for Databases without health checks configured - add test * Create MultiDbClient with custom health check strategy supplier Changes - add test to ensure health status changes from custom health checks are reflected * Create MultiDbClient with custom health check strategy supplier Changes - add test to ensure health status changes from custom health checks are reflected * faster await timeout * add test - use different health check strategies for different endpoints * wait for initial healthy database * add test - configure health check interval and timeout * add test - trigger failover when health check detects unhealthy endpoint * add test - should not failover to unhealthy endpoints * add test - Should trigger failover via circuit breaker even when health check returns HEALTHY * reduce await poll interval in HealthCheckIntegrationTest * mark un-implemented tests are disabled * add test - Should transition from UNKNOWN to HEALTHY * add test - Should create health check when adding new database * fix - Should stop health check when removing database * add test - Should stop health check when removing database * add test - HealthCheckLifecycleTests - Should start health checks automatically when connection is created - Should stop health checks when connection is closed * fix HealthCheck not stopped on StatefulRedisMultiDbConnection.close() * remove HealthStatusListenerTests stubs, health check events, not exposed publicly * format * add health checks unit test * clean up - rename health check thread names to lettuce-* - clean up warnings - format - javadocs & autor updated * address failing tests - Update StatefulMultiDbConnectionIntegrationTests to account for added additional test server in MultiDbTestSupport - Junit4 @after replaced with JUnit5 * address failing tests - Update StatefulMultiDbConnectionIntegrationTests to account for added additional test server in MultiDbTestSupport - Junit4 @after replaced with JUnit5 * package private StatusTracker * make healthStatusManager required when creating MultiDbStatefullConnection * remove un-implemented probing integration tests - covered with unit tests * introduce isHealthy() to replace getHealthStatus() * register listeners before adding HealthChecks
…nt/PubSubEndpoint (#3543) * - move CB creation responsibility from RedisDatabase to client * - introduce interface for CB * - add CircuitBreaker interface - introduce 'CircuitBreakerGeneration' to track CB state changes and issue 'recordResult' on correct stateholder - apply command rejections whenCB is not CLOSED * - fix typo * - add metricsWindowSize to CircuitBreakerConfig - renaming DatabaseEndpoint.bind - add java docs - add tests for Command.onComplete callbacsk for registered in DatabaseEndpoint - introduce toxiproxy - add circuitbreaker test to veify metrics collection and failover triggers * - fix test * - fix failing test due to order of listeners in CB state change events * on feedbacks from @ggivo - drop record functions from CB interface - revisit exposed functions on CB impl - handle and record exception in databaseendpoint.write - fix tests - get rid of thread.sleep's in tests * - remove thread.sleep from test * - format * - limit visibility - improve metrics objects for testability - drop use of thread.sleep in DatabaseEndpointCallbackTests * - revisit the tests to provide the assertions they claim in comments. * - test to check commands failing after endpoint switch * - formatting * - change accesibility of CircuitBreakerGeneration - drop metricsFactory instance approach - fix naming typo - drop TestMetricsFActory - improve reflectinTestUtils * feedback from @ggivo - drop recordFailure/recordsuccess from CircuitBreakerImpl * feedback from @ggivo - revisit CircuitBreakerGeneration interface
* add Ping strategy * add PingStrategyIntegrationTests add integration test * health checks refactored (inject DatabaseConnectionProvider instead ClientOptions Inject DatabaseConnectionProvider into HealthCheckStrategySupplier's. Injecting per DB connection factory allows reuse of MultiDB client resources - ClientOptions no longer propagated to HealthCheckStrategySupplier - HealthCheckStrategySupplier refactored to use DatabaseConnectionProvider * clean up - renamed DatabaseConnectionProvider -> DatabaseRawConnectionFactory - api docs updated * format * Fix sporadic test failures - Shared TestClientResources shutdown during tests, caused subsequent test to fail. * clean up - rename internal vars * clean up - add unit test - remove unused HealthCheckStrategySupplier DEFAULT_WITH_PROVIDER
…erConfig (CAE-1695) (#3571) * add DatabaseConfig.Builder * healthCheckStrategySupplier now defaults to PingStrategy.DEFAULT in the builder - When using the builder without setting healthCheckStrategySupplier: Health checks will use PingStrategy.DEFAULT - When explicitly setting to null: Health checks will be disabled (as documented) - When setting to a custom supplier: Uses the custom health check strategy Example Usage: // Uses PingStrategy.DEFAULT for health checks DatabaseConfig config1 = DatabaseConfig.builder(uri) .weight(1.0f) .build(); // Explicitly disables health checks DatabaseConfig config2 = DatabaseConfig.builder(uri) .healthCheckStrategySupplier(null) .build(); // Uses custom health check strategy DatabaseConfig config3 = DatabaseConfig.builder(uri) .healthCheckStrategySupplier(customSupplier) .build(); * HealthCheckStrategySupplier.NO_HEALTH_CHECK instead null * Remove DatabaseConfig constructors // To create DatabaseConfig use provided builder DatabaseConfig config = DatabaseConfig.builder(redisURI) .weight(1.5f) .clientOptions(options) .circuitBreakerConfig(cbConfig) .healthCheckStrategySupplier(supplier) .build(); * remove redundant public modifiers * Builder for CircuitBreakerConfig // Minimal configuration with defaults CircuitBreakerConfig config = CircuitBreakerConfig.builder().build(); // Custom configuration CircuitBreakerConfig config = CircuitBreakerConfig.builder() .failureRateThreshold(25.0f) .minimumNumberOfFailures(500) .metricsWindowSize(5) .build(); // With custom tracked exceptions Set<Class<? extends Throwable>> customExceptions = new HashSet<>(); customExceptions.add(RuntimeException.class); CircuitBreakerConfig config = CircuitBreakerConfig.builder() .failureRateThreshold(15.5f) .minimumNumberOfFailures(200) .trackedExceptions(customExceptions) .metricsWindowSize(3) .build(); * enforce min window size of 2s * tracked exceptions should not be null * add convenience methods for Tracked Exceptions //Combine add and remove CircuitBreakerConfig config = CircuitBreakerConfig.builder() .addTrackedExceptions(MyCustomException.class) .removeTrackedExceptions(TimeoutException.class) .build(); // Replace all tracked exceptions Set<Class<? extends Throwable>> customExceptions = new HashSet<>(); customExceptions.add(RuntimeException.class); customExceptions.add(IOException.class); CircuitBreakerConfig config = CircuitBreakerConfig.builder() .trackedExceptions(customExceptions) .build(); * remove option to configure per database clientOptions till #3572 is resolved * Disable health checks in test configs to isolate circuit breaker testing Configure DB1, DB2, and DB3 with NO_HEALTH_CHECK to prevent health check interference when testing circuit breaker failure detection. * forma * clean up * address review comments (Copilot)
The redisURI parameter in PingStrategy constructors was never used in the implementation. The actual endpoint URI is passed to doHealthCheck() method when performing health checks, making the constructor parameter redundant. Changes: - Removed RedisURI parameter from both PingStrategy constructors - Updated DEFAULT supplier to use lambda instead of method reference Remove unused redisURI parameter from PingStrategy constructors The redisURI parameter in PingStrategy constructors was never used in the implementation. The actual endpoint URI is passed to doHealthCheck() method when performing health checks, making the constructor parameter redundant. Changes: - Removed RedisURI parameter from both PingStrategy constructors - Updated DEFAULT supplier to use lambda instead of method reference # Conflicts: # src/test/java/io/lettuce/core/failover/health/PingStrategyIntegrationTests.java
* add DatabaseConfig.Builder
* healthCheckStrategySupplier now defaults to PingStrategy.DEFAULT in the builder
- When using the builder without setting healthCheckStrategySupplier: Health checks will use PingStrategy.DEFAULT
- When explicitly setting to null: Health checks will be disabled (as documented)
- When setting to a custom supplier: Uses the custom health check strategy
Example Usage:
// Uses PingStrategy.DEFAULT for health checks
DatabaseConfig config1 = DatabaseConfig.builder(uri)
.weight(1.0f)
.build();
// Explicitly disables health checks
DatabaseConfig config2 = DatabaseConfig.builder(uri)
.healthCheckStrategySupplier(null)
.build();
// Uses custom health check strategy
DatabaseConfig config3 = DatabaseConfig.builder(uri)
.healthCheckStrategySupplier(customSupplier)
.build();
* HealthCheckStrategySupplier.NO_HEALTH_CHECK instead null
* Remove DatabaseConfig constructors
// To create DatabaseConfig use provided builder
DatabaseConfig config = DatabaseConfig.builder(redisURI)
.weight(1.5f)
.clientOptions(options)
.circuitBreakerConfig(cbConfig)
.healthCheckStrategySupplier(supplier)
.build();
* remove redundant public modifiers
* Builder for CircuitBreakerConfig
// Minimal configuration with defaults
CircuitBreakerConfig config = CircuitBreakerConfig.builder().build();
// Custom configuration
CircuitBreakerConfig config = CircuitBreakerConfig.builder()
.failureRateThreshold(25.0f)
.minimumNumberOfFailures(500)
.metricsWindowSize(5)
.build();
// With custom tracked exceptions
Set<Class<? extends Throwable>> customExceptions = new HashSet<>();
customExceptions.add(RuntimeException.class);
CircuitBreakerConfig config = CircuitBreakerConfig.builder()
.failureRateThreshold(15.5f)
.minimumNumberOfFailures(200)
.trackedExceptions(customExceptions)
.metricsWindowSize(3)
.build();
* enforce min window size of 2s
* tracked exceptions should not be null
* add convenience methods for Tracked Exceptions
//Combine add and remove
CircuitBreakerConfig config = CircuitBreakerConfig.builder()
.addTrackedExceptions(MyCustomException.class)
.removeTrackedExceptions(TimeoutException.class)
.build();
// Replace all tracked exceptions
Set<Class<? extends Throwable>> customExceptions = new HashSet<>();
customExceptions.add(RuntimeException.class);
customExceptions.add(IOException.class);
CircuitBreakerConfig config = CircuitBreakerConfig.builder()
.trackedExceptions(customExceptions)
.build();
* remove option to configure per database clientOptions till #3572 is resolved
* Disable health checks in test configs to isolate circuit breaker testing
Configure DB1, DB2, and DB3 with NO_HEALTH_CHECK to prevent health check
interference when testing circuit breaker failure detection.
* forma
* clean up
* address review comments (Copilot)
* Add example for automatic failover
* Use builders
* shutdown primary instance
* remove unused imports
* Update src/test/java/io/lettuce/examples/AutomaticFailover.java
Co-authored-by: atakavci <[email protected]>
* revert accidentally disabled user timeout config
---------
Co-authored-by: ggivo <[email protected]>
Co-authored-by: atakavci <[email protected]>
…lover-1 (#3575) * add Benchmark (jmh) benchmark result for 1343845 * Bump to 8.4-GA-pre.3 (#3516) * add Benchmark (jmh) benchmark result for e8d59fc * Add official 8.4 to test matrix and make it default (#3520) * Add support for XREADGROUP CLAIM arg (#3486) * Add support for XREADGROUP CLAIM arg * Add NOACK scenario in ITs * Fix NOACK IT scenario. Add test. * Implement new fields as integers. Fix tests. * Rename values for consistency. * Address some comments from code review * add Benchmark (jmh) benchmark result for 295546c * Add support CAS/CAD (#3512) * Implement CAS/CAD commands * Add tests * Fix readonly commands count * Remove not needed license comments. * Implement msetex command (#3510) * Implement msetex command * Refactor to use SetArgs * Use dedicated MSetExArgs for MSETEX command * Fix formatting * Keep only instant/duration API * Rm not needed license comment. * Fix tests * Preserve null values when parsing SearchReplies (#3518) EncodedComplexOutput was skipping null values instead of passing them on. Then SearchReplyParser needs to store null values as they are and not try to decode them. This affected both RESP2 and RESP3 parsing. Added two integration tests in RediSearchAggregateIntegrationTests to verify that nulls in JSON documents are parsed correctly. * add Benchmark (jmh) benchmark result for 0796a4e * Modify release notes and bum pom version. (#3525) * add Benchmark (jmh) benchmark result for 7fefd6a * add Benchmark (jmh) benchmark result for 838fe47 * add Benchmark (jmh) benchmark result for 73a7bab * add Benchmark (jmh) benchmark result for 0e49f73 * SearchArgs.returnField with alias produces malformed redis command #3528 (#3530) * add Benchmark (jmh) benchmark result for a4eab37 * fix consistency with get(int) that returns wrapped (#3464) DelegateJsonObject/DelegateJsonArray for nested structures Signed-off-by: NeatGuyCoding <[email protected]> * Bumping Netty to 4.2.5.Final (#3536) * add Benchmark (jmh) benchmark result for 274af38 * add Benchmark (jmh) benchmark result for 8f2080a * add Benchmark (jmh) benchmark result for fe79196 * add Benchmark (jmh) benchmark result for 289398b * add Benchmark (jmh) benchmark result for 2f226a6 * add Benchmark (jmh) benchmark result for a1bb28d * add Benchmark (jmh) benchmark result for d7e6a0a * add Benchmark (jmh) benchmark result for 9230a17 * Add ftHybrid (#3540) * Add ftHybrid * rm max, withCount from SortBy * refactor CombineArgs * Move postprocessing inside PostProcessingArgs * Refactor VectorSearchMethod * Mark new files as experimental * Format * Fix RESP2 parsing * Fix tests for previous versions * Minor fixes in tests * Format * Add enabled on command * Refactor scoring * Tighten integration test with field assertions * Rm commented loadALl * Use keywords instead magic strings * Fixed Range building * Rm defaults from javadoc * Expose method to add upstream driver libraries to CLIENT SETINFO payload (#3542) * Expose method to add upstream driver libraries to CLIENT SETINFO payload * Create a separate class to hold driver name and upstream drivers information * Fix PR comments * Update since tag * add Benchmark (jmh) benchmark result for be132f9 * Release 7.2.0 (#3559) * add Benchmark (jmh) benchmark result for fdcfb74 * Fix command queue corruption on encoding failures (#3443) * Correctly handling the encoding error for Lettuce [POC] Summary: Add encoding error tracking to prevent command queue corruption - Add markEncodingError() and hasEncodingError() methods to RedisCommand interface - Implement encoding error flag in Command class with volatile boolean - Mark commands with encoding errors in CommandEncoder on encode failures - Add lazy cleanup of encoding failures in CommandHandler response processing - Update all RedisCommand implementations to support encoding error tracking - Add comprehensive unit tests and integration tests for encoding error handling Fixes issue where encoding failures could corrupt the outstanding command queue by leaving failed commands in the stack without proper cleanup, causing responses to be matched to wrong commands. Test Plan: UTs, Integration testing Reviewers: yayang, ureview Reviewed By: yayang Tags: #has_java JIRA Issues: REDIS-14050 Differential Revision: https://code.uberinternal.com/D19068147 * Fix error command handling code logic and add integration test for encoding failure Summary: Fix error command handling code logic and add integration test for encoding failure Test Plan: unittest, integration test Reviewers: #ldap_storage_sre_cache, ureview, jingzhao Reviewed By: #ldap_storage_sre_cache, jingzhao Tags: #has_java JIRA Issues: REDIS-14192 Differential Revision: https://code.uberinternal.com/D19271701 * latest changes * Addressing the reactive streams issue * Addressing the encoding issues Addressing some general cases * Formatting issues * Test failures addressed * Polishing --------- Co-authored-by: Jing Zhao <[email protected]> Co-authored-by: Tihomir Mateev <[email protected]> * add Benchmark (jmh) benchmark result for f65b8d1 * add Benchmark (jmh) benchmark result for c6b42f0 * add Benchmark (jmh) benchmark result for 5c5f117 * add Benchmark (jmh) benchmark result for 329c39c * add Benchmark (jmh) benchmark result for fa7e5d0 --------- Signed-off-by: NeatGuyCoding <[email protected]> Co-authored-by: github-action-benchmark <[email protected]> Co-authored-by: Aleksandar Todorov <[email protected]> Co-authored-by: Magnus Hyllander <[email protected]> Co-authored-by: Tihomir Krasimirov Mateev <[email protected]> Co-authored-by: NeatGuyCoding <[email protected]> Co-authored-by: Viktoriya Kutsarova <[email protected]> Co-authored-by: yang <[email protected]> Co-authored-by: Jing Zhao <[email protected]> Co-authored-by: Tihomir Mateev <[email protected]>
ggivo
changed the title
* Mark failover API as experimental Mark all public classes and interfaces in the failover package as @experimental to indicate that this API may change in future releases. Update @SInCE annotations from 7.1/7.2/7.3 to 7.4 package-private implementation classes are not anotated as @experimental * update version to 7.4.0-SNAPSHOT * more experimental tags - classes outside failover package as experimental * format
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.
DO NOT MERGE!!
PR used to keep up to date automatic-failover-1 branch
Make sure that:
mvn formatter:formattarget. Don’t submit any formatting related changes.