Skip to content

Commit 7abdc1e

Browse files
atakavciggivo
atakavci
authored and
ggivo
committed
[automatic failover] Set and test default values for failover config&components (#4298)
* - set & test default values * - format * - fix tests failing due to changing defaults
1 parent 7a6fe1d commit 7abdc1e

File tree

9 files changed

+119
-35
lines changed

9 files changed

+119
-35
lines changed

‎pom.xml‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -482,15 +482,7 @@
482482
<include>**/Endpoint.java</include>
483483
<include>src/main/java/redis/clients/jedis/mcf/*.java</include>
484484
<include>src/test/java/redis/clients/jedis/failover/*.java</include>
485-
<include>**/mcf/EchoStrategyIntegrationTest.java</include>
486-
<include>**/mcf/LagAwareStrategyUnitTest.java</include>
487-
<include>**/mcf/RedisRestAPI*.java</include>
488-
<include>**/mcf/ActiveActiveLocalFailoverTest*</include>
489-
<include>**/mcf/FailbackMechanism*.java</include>
490-
<include>**/mcf/PeriodicFailbackTest*.java</include>
491-
<include>**/mcf/AutomaticFailoverTest*.java</include>
492-
<include>**/mcf/MultiCluster*.java</include>
493-
<include>**/mcf/StatusTracker*.java</include>
485+
<include>src/test/java/redis/clients/jedis/mcf/*.java</include>
494486
<include>**/Health*.java</include>
495487
<include>**/*IT.java</include>
496488
<include>**/scenario/RestEndpointUtil.java</include>

‎src/main/java/redis/clients/jedis/MultiClusterClientConfig.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,16 @@ public static interface StrategySupplier {
130130
.asList(JedisConnectionException.class);
131131

132132
/** Default failure rate threshold percentage for circuit breaker activation. */
133-
private static final float CIRCUIT_BREAKER_FAILURE_RATE_THRESHOLD_DEFAULT = 50.0f;
133+
private static final float CIRCUIT_BREAKER_FAILURE_RATE_THRESHOLD_DEFAULT = 10.0f;
134134

135135
/** Default minimum number of calls required before circuit breaker can calculate failure rate. */
136-
private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_MIN_CALLS_DEFAULT = 100;
136+
private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_MIN_CALLS_DEFAULT = 1000;
137137

138138
/** Default sliding window type for circuit breaker failure tracking. */
139139
private static final SlidingWindowType CIRCUIT_BREAKER_SLIDING_WINDOW_TYPE_DEFAULT = SlidingWindowType.COUNT_BASED;
140140

141141
/** Default sliding window size for circuit breaker failure tracking. */
142-
private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_SIZE_DEFAULT = 100;
142+
private static final int CIRCUIT_BREAKER_SLIDING_WINDOW_SIZE_DEFAULT = 2;
143143

144144
/** Default slow call duration threshold in milliseconds. */
145145
private static final int CIRCUIT_BREAKER_SLOW_CALL_DURATION_THRESHOLD_DEFAULT = 60000;
@@ -156,10 +156,10 @@ public static interface StrategySupplier {
156156
.asList(CallNotPermittedException.class, ConnectionFailoverException.class);
157157

158158
/** Default interval in milliseconds for checking if failed clusters have recovered. */
159-
private static final long FAILBACK_CHECK_INTERVAL_DEFAULT = 5000;
159+
private static final long FAILBACK_CHECK_INTERVAL_DEFAULT = 120000;
160160

161161
/** Default grace period in milliseconds to keep clusters disabled after they become unhealthy. */
162-
private static final long GRACE_PERIOD_DEFAULT = 10000;
162+
private static final long GRACE_PERIOD_DEFAULT = 60000;
163163

164164
/** Default maximum number of failover attempts. */
165165
private static final int MAX_NUM_FAILOVER_ATTEMPTS_DEFAULT = 10;

‎src/main/java/redis/clients/jedis/mcf/EchoStrategy.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,19 @@
1111
import redis.clients.jedis.MultiClusterClientConfig.StrategySupplier;
1212

1313
public class EchoStrategy implements HealthCheckStrategy {
14+
private static final int MAX_HEALTH_CHECK_POOL_SIZE = 2;
1415

1516
private final UnifiedJedis jedis;
1617
private final HealthCheckStrategy.Config config;
1718

1819
public EchoStrategy(HostAndPort hostAndPort, JedisClientConfig jedisClientConfig) {
19-
this(hostAndPort, jedisClientConfig, HealthCheckStrategy.Config.builder().build());
20+
this(hostAndPort, jedisClientConfig, HealthCheckStrategy.Config.create());
2021
}
2122

2223
public EchoStrategy(HostAndPort hostAndPort, JedisClientConfig jedisClientConfig,
2324
HealthCheckStrategy.Config config) {
2425
GenericObjectPoolConfig<Connection> poolConfig = new GenericObjectPoolConfig<>();
25-
poolConfig.setMaxTotal(2);
26+
poolConfig.setMaxTotal(MAX_HEALTH_CHECK_POOL_SIZE);
2627
this.jedis = new JedisPooled(hostAndPort, jedisClientConfig, poolConfig);
2728
this.config = config;
2829
}

‎src/main/java/redis/clients/jedis/mcf/HealthCheckStrategy.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ default void close() {
4949
int getDelayInBetweenProbes();
5050

5151
public static class Config {
52+
private static final int INTERVAL_DEFAULT = 5000;
53+
private static final int TIMEOUT_DEFAULT = 1000;
54+
private static final int NUM_PROBES_DEFAULT = 3;
55+
private static final int DELAY_IN_BETWEEN_PROBES_DEFAULT = 500;
56+
5257
protected final int interval;
5358
protected final int timeout;
5459
protected final int numProbes;
@@ -97,14 +102,14 @@ public ProbingPolicy getPolicy() {
97102
* @return a new Config instance
98103
*/
99104
public static Config create() {
100-
return new Builder<>().build();
105+
return builder().build();
101106
}
102107

103108
/**
104109
* Create a new builder for HealthCheckStrategy.Config.
105110
* @return a new Builder instance
106111
*/
107-
public static Builder<?, Config> builder() {
112+
public static Builder<?, ? extends Config> builder() {
108113
return new Builder<>();
109114
}
110115

@@ -114,11 +119,11 @@ public static Builder<?, Config> builder() {
114119
* @param <C> the config type being built
115120
*/
116121
public static class Builder<T extends Builder<T, C>, C extends Config> {
117-
protected int interval = 1000;
118-
protected int timeout = 1000;
119-
protected int numProbes = 3;
122+
protected int interval = INTERVAL_DEFAULT;
123+
protected int timeout = TIMEOUT_DEFAULT;
124+
protected int numProbes = NUM_PROBES_DEFAULT;
120125
protected ProbingPolicy policy = ProbingPolicy.BuiltIn.ALL_SUCCESS;
121-
protected int delayInBetweenProbes = 100;
126+
protected int delayInBetweenProbes = DELAY_IN_BETWEEN_PROBES_DEFAULT;
122127

123128
/**
124129
* Set the interval between health checks in milliseconds.

‎src/main/java/redis/clients/jedis/mcf/LagAwareStrategy.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,15 @@ public HealthStatus doHealthCheck(Endpoint endpoint) {
9494
public static class Config extends HealthCheckStrategy.Config {
9595

9696
public static final boolean EXTENDED_CHECK_DEFAULT = true;
97-
public static final Duration AVAILABILITY_LAG_TOLERANCE_DEFAULT = Duration.ofMillis(100);
97+
public static final Duration AVAILABILITY_LAG_TOLERANCE_DEFAULT = Duration.ofMillis(5000);
9898

9999
private final Endpoint restEndpoint;
100100
private final Supplier<RedisCredentials> credentialsSupplier;
101101

102102
// SSL configuration for HTTPS connections to Redis Enterprise REST API
103103
private final SslOptions sslOptions;
104104

105-
// Maximum acceptable lag in milliseconds (default: 100);
105+
// Maximum acceptable lag in milliseconds (default: 5000);
106106
private final Duration availability_lag_tolerance;
107107

108108
// Enable extended lag checking (default: true - performs lag validation in addition to standard
@@ -111,7 +111,7 @@ public static class Config extends HealthCheckStrategy.Config {
111111
private final boolean extendedCheckEnabled;
112112

113113
public Config(Endpoint restEndpoint, Supplier<RedisCredentials> credentialsSupplier) {
114-
this(builder(restEndpoint, credentialsSupplier).interval(1000).timeout(1000).numProbes(3)
114+
this(builder(restEndpoint, credentialsSupplier)
115115
.availabilityLagTolerance(AVAILABILITY_LAG_TOLERANCE_DEFAULT)
116116
.extendedCheckEnabled(EXTENDED_CHECK_DEFAULT));
117117
}
@@ -157,6 +157,15 @@ public static ConfigBuilder builder(Endpoint restEndpoint,
157157
return new ConfigBuilder(restEndpoint, credentialsSupplier);
158158
}
159159

160+
/**
161+
* Use {@link LagAwareStrategy.Config#builder(Endpoint, Supplier)} instead.
162+
* @return a new Builder instance
163+
*/
164+
public static ConfigBuilder builder() {
165+
throw new UnsupportedOperationException(
166+
"Endpoint and credentials are required to build LagAwareStrategy.Config.");
167+
}
168+
160169
/**
161170
* Create a new Config instance with default values.
162171
* <p>

‎src/test/java/redis/clients/jedis/mcf/DefaultValuesTest.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package redis.clients.jedis.mcf;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertNotNull;
5+
6+
import java.time.Duration;
7+
8+
import org.junit.jupiter.api.Test;
9+
import redis.clients.jedis.DefaultJedisClientConfig;
10+
import redis.clients.jedis.HostAndPort;
11+
import redis.clients.jedis.JedisClientConfig;
12+
import redis.clients.jedis.MultiClusterClientConfig;
13+
14+
public class DefaultValuesTest {
15+
16+
HostAndPort fakeEndpoint = new HostAndPort("fake", 6379);
17+
JedisClientConfig config = DefaultJedisClientConfig.builder().build();
18+
19+
@Test
20+
void testDefaultValuesInConfig() {
21+
22+
MultiClusterClientConfig.ClusterConfig clusterConfig = MultiClusterClientConfig.ClusterConfig
23+
.builder(fakeEndpoint, config).build();
24+
MultiClusterClientConfig multiConfig = new MultiClusterClientConfig.Builder(
25+
new MultiClusterClientConfig.ClusterConfig[] { clusterConfig }).build();
26+
27+
// check for grace period
28+
assertEquals(60000, multiConfig.getGracePeriod());
29+
30+
// check for cluster config
31+
assertEquals(clusterConfig, multiConfig.getClusterConfigs()[0]);
32+
33+
// check healthchecks enabled
34+
assertNotNull(clusterConfig.getHealthCheckStrategySupplier());
35+
36+
// check default healthcheck strategy is echo
37+
assertEquals(EchoStrategy.DEFAULT, clusterConfig.getHealthCheckStrategySupplier());
38+
39+
// check number of probes
40+
assertEquals(3,
41+
clusterConfig.getHealthCheckStrategySupplier().get(fakeEndpoint, config).getNumProbes());
42+
43+
assertEquals(500, clusterConfig.getHealthCheckStrategySupplier().get(fakeEndpoint, config)
44+
.getDelayInBetweenProbes());
45+
46+
assertEquals(ProbingPolicy.BuiltIn.ALL_SUCCESS,
47+
clusterConfig.getHealthCheckStrategySupplier().get(fakeEndpoint, config).getPolicy());
48+
49+
// check health check interval
50+
assertEquals(5000,
51+
clusterConfig.getHealthCheckStrategySupplier().get(fakeEndpoint, config).getInterval());
52+
53+
// check lag aware tolerance
54+
LagAwareStrategy.Config lagAwareConfig = LagAwareStrategy.Config
55+
.builder(fakeEndpoint, config.getCredentialsProvider()).build();
56+
assertEquals(Duration.ofMillis(5000), lagAwareConfig.getAvailabilityLagTolerance());
57+
58+
// TODO: check CB number of failures threshold -- 1000
59+
// assertEquals(1000, multiConfig.circuitBreakerMinNumOfFailures());
60+
61+
// check CB failure rate threshold
62+
assertEquals(10, multiConfig.getCircuitBreakerFailureRateThreshold());
63+
64+
// check CB sliding window size
65+
assertEquals(2, multiConfig.getCircuitBreakerSlidingWindowSize());
66+
67+
// check failback check interval
68+
assertEquals(120000, multiConfig.getFailbackCheckInterval());
69+
70+
// check failover max attempts before give up
71+
assertEquals(10, multiConfig.getMaxNumFailoverAttempts());
72+
73+
// check delay between failover attempts
74+
assertEquals(12000, multiConfig.getDelayInBetweenFailoverAttempts());
75+
76+
}
77+
}

‎src/test/java/redis/clients/jedis/mcf/FailbackMechanismUnitTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void testFailbackCheckIntervalConfiguration() {
3232
MultiClusterClientConfig defaultConfig = new MultiClusterClientConfig.Builder(
3333
new MultiClusterClientConfig.ClusterConfig[] { clusterConfig }).build();
3434

35-
assertEquals(5000, defaultConfig.getFailbackCheckInterval());
35+
assertEquals(120000, defaultConfig.getFailbackCheckInterval());
3636

3737
// Test custom value
3838
MultiClusterClientConfig customConfig = new MultiClusterClientConfig.Builder(
@@ -105,7 +105,7 @@ void testGracePeriodConfiguration() {
105105
MultiClusterClientConfig defaultConfig = new MultiClusterClientConfig.Builder(
106106
new MultiClusterClientConfig.ClusterConfig[] { clusterConfig }).build();
107107

108-
assertEquals(10000, defaultConfig.getGracePeriod()); // Default is 10 seconds
108+
assertEquals(60000, defaultConfig.getGracePeriod()); // Default is 10 seconds
109109

110110
// Test custom value
111111
MultiClusterClientConfig customConfig = new MultiClusterClientConfig.Builder(

‎src/test/java/redis/clients/jedis/mcf/HealthCheckTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ void testStrategySupplierPolymorphism() {
531531
// Test without config
532532
HealthCheckStrategy strategyWithoutConfig = supplier.get(testEndpoint, null);
533533
assertNotNull(strategyWithoutConfig);
534-
assertEquals(1000, strategyWithoutConfig.getInterval()); // Default values
534+
assertEquals(5000, strategyWithoutConfig.getInterval()); // Default values
535535
assertEquals(1000, strategyWithoutConfig.getTimeout());
536536
}
537537

‎src/test/java/redis/clients/jedis/mcf/LagAwareStrategyUnitTest.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void healthy_when_bdb_available_and_cached_uid_used_on_next_check() throws Excep
5050
try (MockedConstruction<RedisRestAPI> mockedConstructor = mockConstruction(RedisRestAPI.class,
5151
(mock, context) -> {
5252
when(mock.getBdbs()).thenReturn(Arrays.asList(bdbInfo));
53-
when(mock.checkBdbAvailability("1", true, 100L)).thenReturn(true);
53+
when(mock.checkBdbAvailability("1", true, 5000L)).thenReturn(true);
5454
reference[0] = mock;
5555
})) {
5656
Config lagCheckConfig = Config.builder(endpoint, creds).interval(500).timeout(250)
@@ -61,7 +61,7 @@ void healthy_when_bdb_available_and_cached_uid_used_on_next_check() throws Excep
6161

6262
assertEquals(HealthStatus.HEALTHY, strategy.doHealthCheck(endpoint));
6363
verify(api, times(1)).getBdbs(); // Should not call getBdbs again when cached
64-
verify(api, times(2)).checkBdbAvailability("1", true, 100L);
64+
verify(api, times(2)).checkBdbAvailability("1", true, 5000L);
6565
}
6666
}
6767
}
@@ -97,7 +97,7 @@ void exception_and_cache_reset_on_exception_then_recovers_next_time() throws Exc
9797
// First call throws exception, second call returns bdbInfo
9898
when(mock.getBdbs()).thenThrow(new RuntimeException("boom"))
9999
.thenReturn(Arrays.asList(bdbInfo));
100-
when(mock.checkBdbAvailability("42", true, 100L)).thenReturn(true);
100+
when(mock.checkBdbAvailability("42", true, 5000L)).thenReturn(true);
101101
reference[0] = mock;
102102
})) {
103103

@@ -115,7 +115,7 @@ void exception_and_cache_reset_on_exception_then_recovers_next_time() throws Exc
115115
// Verify getBdbs was called twice (once failed, once succeeded)
116116
verify(api, times(2)).getBdbs();
117117
// Verify availability check was called only once (on the successful attempt)
118-
verify(api, times(1)).checkBdbAvailability("42", true, 100L);
118+
verify(api, times(1)).checkBdbAvailability("42", true, 5000L);
119119
}
120120
}
121121
}
@@ -173,10 +173,10 @@ void exception_when_no_matching_host_found() throws Exception {
173173
void config_builder_creates_config_with_default_values() {
174174
Config config = Config.builder(endpoint, creds).build();
175175

176-
assertEquals(1000, config.interval);
176+
assertEquals(5000, config.interval);
177177
assertEquals(1000, config.timeout);
178178
assertEquals(3, config.numProbes);
179-
assertEquals(Duration.ofMillis(100), config.getAvailabilityLagTolerance());
179+
assertEquals(Duration.ofMillis(5000), config.getAvailabilityLagTolerance());
180180
assertEquals(endpoint, config.getRestEndpoint());
181181
assertEquals(creds, config.getCredentialsSupplier());
182182
}
@@ -288,7 +288,7 @@ void base_config_builder_factory_method_works() {
288288
void base_config_create_factory_method_uses_defaults() {
289289
HealthCheckStrategy.Config config = HealthCheckStrategy.Config.create();
290290

291-
assertEquals(1000, config.getInterval());
291+
assertEquals(5000, config.getInterval());
292292
assertEquals(1000, config.getTimeout());
293293
assertEquals(3, config.getNumProbes());
294294
}

0 commit comments

Comments
 (0)