Use central registries for Retry and CircuitBreaker instances

This commit is contained in:
Jon Chambers
2025-08-27 11:33:42 -04:00
committed by GitHub
parent a8c6fa93e0
commit f616612104
33 changed files with 326 additions and 349 deletions

View File

@@ -35,8 +35,6 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
public class CloudflareTurnCredentialsManagerTest {
@RegisterExtension
@@ -77,9 +75,9 @@ public class CloudflareTurnCredentialsManagerTest {
IP_URL_PATTERNS,
TURN_HOSTNAME,
2,
new CircuitBreakerConfiguration(),
null,
httpExecutor,
new RetryConfiguration(),
null,
retryExecutor,
dnsResolver
);

View File

@@ -27,8 +27,6 @@ import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.configuration.Cdn3StorageManagerConfiguration;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
import org.whispersystems.textsecuregcm.configuration.secrets.SecretString;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
import org.whispersystems.textsecuregcm.util.SystemMapper;
@@ -58,8 +56,8 @@ public class Cdn3RemoteStorageManagerTest {
new SecretString("clientSecret"),
Map.of(2, "gcs", 3, "r2"),
2,
new CircuitBreakerConfiguration(),
new RetryConfiguration()));
null,
null));
}
@ParameterizedTest

View File

@@ -11,6 +11,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -19,6 +20,8 @@ import static org.mockito.Mockito.when;
import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
import io.github.resilience4j.retry.Retry;
import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
@@ -39,6 +42,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
import org.whispersystems.textsecuregcm.util.CircuitBreakerUtil;
class FaultTolerantHttpClientTest {
@@ -71,12 +75,8 @@ class FaultTolerantHttpClientTest {
.withHeader("Content-Type", "text/plain")
.withBody("Pong!")));
FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder()
.withCircuitBreaker(new CircuitBreakerConfiguration())
.withRetry(new RetryConfiguration())
.withExecutor(httpExecutor)
.withRetryExecutor(retryExecutor)
.withName("test")
FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder("testSimpleGet", httpExecutor)
.withRetry(null, retryExecutor)
.withVersion(HttpClient.Version.HTTP_2)
.build();
@@ -101,12 +101,8 @@ class FaultTolerantHttpClientTest {
.withHeader("Content-Type", "text/plain")
.withBody("Pong!")));
FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder()
.withCircuitBreaker(new CircuitBreakerConfiguration())
.withRetry(new RetryConfiguration())
.withExecutor(httpExecutor)
.withRetryExecutor(retryExecutor)
.withName("test")
FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder("testRetryGet", httpExecutor)
.withRetry(null, retryExecutor)
.withVersion(HttpClient.Version.HTTP_2)
.build();
@@ -126,14 +122,17 @@ class FaultTolerantHttpClientTest {
@Test
void testRetryGetOnException() {
final HttpClient mockHttpClient = mock(HttpClient.class);
final Retry retry = Retry.of("test", new RetryConfiguration().toRetryConfigBuilder()
.retryOnException(throwable -> throwable instanceof IOException)
.build());
final FaultTolerantHttpClient client = new FaultTolerantHttpClient(
"test",
List.of(mockHttpClient),
retryExecutor,
Duration.ofSeconds(1),
new RetryConfiguration(),
throwable -> throwable instanceof IOException,
new CircuitBreakerConfiguration());
retryExecutor,
retry,
CircuitBreaker.ofDefaults("test"));
when(mockHttpClient.sendAsync(any(), any()))
.thenReturn(CompletableFuture.failedFuture(new IOException("test exception")));
@@ -156,14 +155,17 @@ class FaultTolerantHttpClientTest {
void testMultipleClients() throws IOException, InterruptedException {
final HttpClient mockHttpClient1 = mock(HttpClient.class);
final HttpClient mockHttpClient2 = mock(HttpClient.class);
final Retry retry = Retry.of("test", new RetryConfiguration().toRetryConfigBuilder()
.retryOnException(throwable -> throwable instanceof IOException)
.build());
final FaultTolerantHttpClient client = new FaultTolerantHttpClient(
"test",
List.of(mockHttpClient1, mockHttpClient2),
retryExecutor,
Duration.ofSeconds(1),
new RetryConfiguration(),
throwable -> throwable instanceof IOException,
new CircuitBreakerConfiguration());
retryExecutor,
retry,
CircuitBreaker.ofDefaults("test"));
// Just to get a dummy HttpResponse
wireMock.stubFor(get(urlEqualTo("/ping"))
@@ -201,68 +203,50 @@ class FaultTolerantHttpClientTest {
@Test
void testNetworkFailureCircuitBreaker() throws InterruptedException {
CircuitBreakerConfiguration circuitBreakerConfiguration = new CircuitBreakerConfiguration();
final CircuitBreakerConfiguration circuitBreakerConfiguration = new CircuitBreakerConfiguration();
circuitBreakerConfiguration.setSlidingWindowSize(2);
circuitBreakerConfiguration.setSlidingWindowMinimumNumberOfCalls(2);
circuitBreakerConfiguration.setPermittedNumberOfCallsInHalfOpenState(1);
circuitBreakerConfiguration.setFailureRateThreshold(50);
circuitBreakerConfiguration.setWaitDurationInOpenState(Duration.ofSeconds(1));
FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder()
.withCircuitBreaker(circuitBreakerConfiguration)
.withRetry(new RetryConfiguration())
.withRetryExecutor(retryExecutor)
.withExecutor(httpExecutor)
.withName("test")
final String circuitBreakerConfigurationName = getClass().getSimpleName() + "#testNetworkFailureCircuitBreaker";
CircuitBreakerUtil.getCircuitBreakerRegistry()
.addConfiguration(circuitBreakerConfigurationName, circuitBreakerConfiguration.toCircuitBreakerConfig());
final FaultTolerantHttpClient client = FaultTolerantHttpClient.newBuilder("testNetworkFailureCircuitBreaker", httpExecutor)
.withCircuitBreaker(circuitBreakerConfigurationName)
.withRetry(null, retryExecutor)
.withVersion(HttpClient.Version.HTTP_2)
.build();
HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create("http://localhost:" + 39873 + "/failure"))
.GET()
.build();
final HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create("http://localhost:" + 39873 + "/failure"))
.GET()
.build();
try {
client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join();
throw new AssertionError("Should have failed!");
} catch (CompletionException e) {
assertThat(e.getCause()).isInstanceOf(IOException.class);
// good
}
assertThatThrownBy(() -> client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(IOException.class);
try {
client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join();
throw new AssertionError("Should have failed!");
} catch (CompletionException e) {
assertThat(e.getCause()).isInstanceOf(IOException.class);
// good
}
assertThatThrownBy(() -> client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(IOException.class);
try {
client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join();
throw new AssertionError("Should have failed!");
} catch (CompletionException e) {
assertThat(e.getCause()).isInstanceOf(CallNotPermittedException.class);
// good
}
assertThatThrownBy(() -> client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(CallNotPermittedException.class);
Thread.sleep(1001);
try {
client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join();
throw new AssertionError("Should have failed!");
} catch (CompletionException e) {
assertThat(e.getCause()).isInstanceOf(IOException.class);
// good
}
assertThatThrownBy(() -> client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(IOException.class);
try {
client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join();
throw new AssertionError("Should have failed!");
} catch (CompletionException e) {
assertThat(e.getCause()).isInstanceOf(CallNotPermittedException.class);
// good
}
assertThatThrownBy(() -> client.sendAsync(request, HttpResponse.BodyHandlers.ofString()).join())
.isInstanceOf(CompletionException.class)
.hasCauseInstanceOf(CallNotPermittedException.class);
}
}

View File

@@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertThrows;
import io.github.resilience4j.circuitbreaker.CallNotPermittedException;
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
import io.lettuce.core.RedisCommandTimeoutException;
import io.lettuce.core.RedisException;
import io.lettuce.core.resource.ClientResources;
@@ -34,9 +35,14 @@ class FaultTolerantRedisClientTest {
@Nullable final CircuitBreakerConfiguration circuitBreakerConfiguration,
final ClientResources.Builder clientResourcesBuilder) {
return new FaultTolerantRedisClient("test", clientResourcesBuilder,
RedisServerExtension.getRedisURI(), TIMEOUT,
Optional.ofNullable(circuitBreakerConfiguration).orElseGet(CircuitBreakerConfiguration::new));
final CircuitBreaker circuitBreaker = CircuitBreaker.of("test", Optional.ofNullable(circuitBreakerConfiguration)
.orElseGet(CircuitBreakerConfiguration::new).toCircuitBreakerConfig());
return new FaultTolerantRedisClient("test",
clientResourcesBuilder,
RedisServerExtension.getRedisURI(),
TIMEOUT,
circuitBreaker);
}
@AfterEach

View File

@@ -46,7 +46,6 @@ import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
@@ -61,6 +60,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.util.CircuitBreakerUtil;
import org.whispersystems.textsecuregcm.util.Pair;
import org.whispersystems.textsecuregcm.util.RedisClusterUtil;
@@ -71,6 +71,8 @@ class FaultTolerantRedisClusterClientTest {
private static final Duration TIMEOUT = Duration.ofMillis(200);
private static int circuitBreakerConfigurationCount = 0;
@RegisterExtension
static final RedisClusterExtension REDIS_CLUSTER_EXTENSION = RedisClusterExtension.builder()
.timeout(TIMEOUT)
@@ -79,13 +81,24 @@ class FaultTolerantRedisClusterClientTest {
private FaultTolerantRedisClusterClient cluster;
private static FaultTolerantRedisClusterClient buildCluster(
final String name,
@Nullable final CircuitBreakerConfiguration circuitBreakerConfiguration,
final ClientResources.Builder clientResourcesBuilder) {
return new FaultTolerantRedisClusterClient("test",
final String circuitBreakerConfigurationName;
if (circuitBreakerConfiguration != null) {
circuitBreakerConfigurationName = FaultTolerantRedisClusterClientTest.class.getSimpleName() + "-" + circuitBreakerConfigurationCount++;
CircuitBreakerUtil.getCircuitBreakerRegistry().addConfiguration(circuitBreakerConfigurationName, circuitBreakerConfiguration.toCircuitBreakerConfig());
} else {
circuitBreakerConfigurationName = null;
}
return new FaultTolerantRedisClusterClient(name,
clientResourcesBuilder.socketAddressResolver(REDIS_CLUSTER_EXTENSION.getSocketAddressResolver()),
RedisClusterExtension.getRedisURIs(), TIMEOUT,
Optional.ofNullable(circuitBreakerConfiguration).orElseGet(CircuitBreakerConfiguration::new));
RedisClusterExtension.getRedisURIs(),
TIMEOUT,
circuitBreakerConfigurationName);
}
@AfterEach
@@ -95,7 +108,7 @@ class FaultTolerantRedisClusterClientTest {
@Test
void testTimeout() {
cluster = buildCluster(null, ClientResources.builder());
cluster = buildCluster("testTimeout", null, ClientResources.builder());
final ExecutionException asyncException = assertThrows(ExecutionException.class,
() -> cluster.withCluster(connection -> connection.async().blpop(10 * TIMEOUT.toMillis() / 1000d, "key"))
@@ -119,7 +132,7 @@ class FaultTolerantRedisClusterClientTest {
circuitBreakerConfig.setSlidingWindowSize(1);
circuitBreakerConfig.setWaitDurationInOpenState(breakerWaitDuration);
cluster = buildCluster(circuitBreakerConfig, ClientResources.builder());
cluster = buildCluster("testTimeoutCircuitBreaker", circuitBreakerConfig, ClientResources.builder());
final String key = "key";
@@ -149,7 +162,7 @@ class FaultTolerantRedisClusterClientTest {
final ClientResources.Builder builder = CompositeNettyCustomizerClientResourcesBuilder.builder()
.nettyCustomizer(testBreakerManager);
cluster = buildCluster(circuitBreakerConfig, builder);
cluster = buildCluster("testShardUnavailable", circuitBreakerConfig, builder);
// this test will open the breaker on one shard and check that other shards are still available,
// so we get two nodes and a slot+key on each to test
@@ -202,7 +215,7 @@ class FaultTolerantRedisClusterClientTest {
final ClientResources.Builder builder = CompositeNettyCustomizerClientResourcesBuilder.builder()
.nettyCustomizer(testBreakerManager);
cluster = buildCluster(circuitBreakerConfig, builder);
cluster = buildCluster("testShardUnavailablePubSub", circuitBreakerConfig, builder);
cluster.useCluster(
connection -> connection.sync().upstream().commands().configSet("notify-keyspace-events", "K$glz"));

View File

@@ -33,19 +33,15 @@ import io.netty.channel.ChannelPromise;
import io.netty.channel.embedded.EmbeddedChannel;
import java.io.IOException;
import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.StreamSupport;
import javax.annotation.Nullable;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import reactor.core.publisher.Flux;
import reactor.core.scheduler.Schedulers;
@@ -59,15 +55,15 @@ class LettuceShardCircuitBreakerTest {
eventBus = mock(EventBus.class);
when(eventBus.get()).thenReturn(Flux.never());
channelCircuitBreakerHandler = new LettuceShardCircuitBreaker.ChannelCircuitBreakerHandler(
"test", new CircuitBreakerConfiguration().toCircuitBreakerConfig(), Collections.emptySet(), eventBus,
Schedulers.immediate());
"test", null, Collections.emptySet(), eventBus, Schedulers.immediate());
}
@Test
void testAfterChannelInitialized() {
final LettuceShardCircuitBreaker lettuceShardCircuitBreaker = new LettuceShardCircuitBreaker("test",
new CircuitBreakerConfiguration().toCircuitBreakerConfig(), Schedulers.immediate());
final LettuceShardCircuitBreaker lettuceShardCircuitBreaker =
new LettuceShardCircuitBreaker("test", null, Schedulers.immediate());
lettuceShardCircuitBreaker.setEventBus(eventBus);
final Channel channel = new EmbeddedChannel(

View File

@@ -22,6 +22,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.apache.commons.lang3.RandomStringUtils;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
@@ -30,6 +31,7 @@ import org.testcontainers.containers.ComposeContainer;
import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.containers.wait.strategy.WaitStrategy;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.util.CircuitBreakerUtil;
import org.whispersystems.textsecuregcm.util.TestcontainersImages;
public class RedisClusterExtension implements BeforeAllCallback, BeforeEachCallback, AfterEachCallback, ExtensionContext.Store.CloseableResource {
@@ -43,6 +45,9 @@ public class RedisClusterExtension implements BeforeAllCallback, BeforeEachCallb
private ClientResources redisClientResources;
private FaultTolerantRedisClusterClient redisClusterClient;
private static final String CIRCUIT_BREAKER_CONFIGURATION_NAME =
RedisClusterExtension.class.getSimpleName() + "-" + RandomStringUtils.insecure().nextAlphanumeric(8);
private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(2);
private static final int REDIS_PORT = 6379;
@@ -97,6 +102,11 @@ public class RedisClusterExtension implements BeforeAllCallback, BeforeEachCallb
@Override
public void beforeAll(final ExtensionContext context) throws Exception {
if (composeContainer == null) {
final CircuitBreakerConfiguration circuitBreakerConfig = new CircuitBreakerConfiguration();
circuitBreakerConfig.setWaitDurationInOpenState(Duration.ofMillis(500));
CircuitBreakerUtil.getCircuitBreakerRegistry().addConfiguration(CIRCUIT_BREAKER_CONFIGURATION_NAME, circuitBreakerConfig.toCircuitBreakerConfig());
final File clusterComposeFile = File.createTempFile("redis-cluster", ".yml");
clusterComposeFile.deleteOnExit();
@@ -181,14 +191,11 @@ public class RedisClusterExtension implements BeforeAllCallback, BeforeEachCallb
.socketAddressResolver(getSocketAddressResolver())
.build();
final CircuitBreakerConfiguration circuitBreakerConfig = new CircuitBreakerConfiguration();
circuitBreakerConfig.setWaitDurationInOpenState(Duration.ofMillis(500));
redisClusterClient = new FaultTolerantRedisClusterClient("test-cluster",
redisClientResources.mutate(),
getRedisURIs(),
timeout,
circuitBreakerConfig);
CIRCUIT_BREAKER_CONFIGURATION_NAME);
redisClusterClient.useCluster(connection -> connection.sync().flushall(FlushMode.SYNC));
}

View File

@@ -6,6 +6,7 @@
package org.whispersystems.textsecuregcm.redis;
import com.redis.testcontainers.RedisContainer;
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
import io.lettuce.core.FlushMode;
import io.lettuce.core.RedisURI;
import io.lettuce.core.resource.ClientResources;
@@ -64,7 +65,7 @@ public class RedisServerExtension implements BeforeAllCallback, BeforeEachCallba
redisClientResources.mutate(),
getRedisURI(),
Duration.ofSeconds(2),
circuitBreakerConfig);
CircuitBreaker.of("test", circuitBreakerConfig.toCircuitBreakerConfig()));
faultTolerantRedisClient.useConnection(connection -> connection.sync().flushall(FlushMode.SYNC));
}

View File

@@ -32,7 +32,6 @@ import org.junit.jupiter.api.extension.RegisterExtension;
import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials;
import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentialsGenerator;
import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
import org.whispersystems.textsecuregcm.configuration.SecureStorageServiceConfiguration;
class SecureStorageClientTest {
@@ -104,8 +103,8 @@ class SecureStorageClientTest {
lY6ZKNA81Lm3YADYtObmK1IUrOPo9BeIaPy0UM08SmN880Vunqa91Q==
-----END CERTIFICATE-----
"""),
new CircuitBreakerConfiguration(),
new RetryConfiguration());
null,
null);
secureStorageClient = new SecureStorageClient(credentialsGenerator, httpExecutor, retryExecutor, config);
}

View File

@@ -39,7 +39,6 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.whispersystems.textsecuregcm.configuration.RetryConfiguration;
import org.whispersystems.textsecuregcm.storage.SubscriptionException;
import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil;
@@ -63,7 +62,7 @@ class AppleAppStoreManagerTest {
reset(apiClient, signedDataVerifier);
executor = Executors.newSingleThreadScheduledExecutor();
appleAppStoreManager = new AppleAppStoreManager(apiClient, signedDataVerifier,
SUBSCRIPTION_GROUP_ID, Map.of(PRODUCT_ID, LEVEL), new RetryConfiguration(), executor, executor);
SUBSCRIPTION_GROUP_ID, Map.of(PRODUCT_ID, LEVEL), null, executor, executor);
}
@AfterEach