Skip to content

Commit 7eee650

Browse files
authored
Misc cleanups in S3BlobContainerRetriesTests (#126101)
- Simplify multi-object-delete request detection - Replace `AtomicBoolean` with volatile field - Make `ThrottlingDeleteHandler` static
1 parent e52288d commit 7eee650

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@
8080
import java.util.Objects;
8181
import java.util.OptionalInt;
8282
import java.util.Set;
83-
import java.util.concurrent.atomic.AtomicBoolean;
8483
import java.util.concurrent.atomic.AtomicInteger;
8584
import java.util.concurrent.atomic.AtomicLong;
8685
import java.util.function.IntConsumer;
@@ -114,19 +113,19 @@ public class S3BlobContainerRetriesTests extends AbstractBlobContainerRetriesTes
114113

115114
private static final int MAX_NUMBER_SNAPSHOT_DELETE_RETRIES = 10;
116115
private S3Service service;
117-
private AtomicBoolean shouldErrorOnDns;
116+
private volatile boolean shouldErrorOnDns;
118117
private RecordingMeterRegistry recordingMeterRegistry;
119118

120119
@Before
121120
public void setUp() throws Exception {
122-
shouldErrorOnDns = new AtomicBoolean(false);
121+
shouldErrorOnDns = false;
123122
service = new S3Service(Mockito.mock(Environment.class), Settings.EMPTY, Mockito.mock(ResourceWatcherService.class)) {
124123
@Override
125124
protected AmazonS3ClientBuilder buildClientBuilder(S3ClientSettings clientSettings) {
126125
final AmazonS3ClientBuilder builder = super.buildClientBuilder(clientSettings);
127126
final DnsResolver defaultDnsResolver = builder.getClientConfiguration().getDnsResolver();
128127
builder.getClientConfiguration().setDnsResolver(host -> {
129-
if (shouldErrorOnDns.get() && randomBoolean() && randomBoolean()) {
128+
if (shouldErrorOnDns && randomBoolean() && randomBoolean()) {
130129
throw new UnknownHostException(host);
131130
}
132131
return defaultDnsResolver.resolve(host);
@@ -665,7 +664,7 @@ public void testReadWithIndicesPurposeRetriesForever() throws IOException {
665664

666665
final byte[] bytes = randomBlobContent(512);
667666

668-
shouldErrorOnDns.set(true);
667+
shouldErrorOnDns = true;
669668
final AtomicInteger failures = new AtomicInteger();
670669
@SuppressForbidden(reason = "use a http server")
671670
class FlakyReadHandler implements HttpHandler {
@@ -893,7 +892,7 @@ private int expectedNumberOfBatches(int blobsToDelete) {
893892
}
894893

895894
@SuppressForbidden(reason = "use a http server")
896-
private class ThrottlingDeleteHandler extends S3HttpHandler {
895+
private static class ThrottlingDeleteHandler extends S3HttpHandler {
897896

898897
private static final String THROTTLING_ERROR_CODE = "SlowDown";
899898

@@ -918,7 +917,7 @@ private class ThrottlingDeleteHandler extends S3HttpHandler {
918917

919918
@Override
920919
public void handle(HttpExchange exchange) throws IOException {
921-
if (exchange.getRequestMethod().equals("POST") && exchange.getRequestURI().toString().startsWith("/bucket/?delete")) {
920+
if (isMultiDeleteRequest(exchange)) {
922921
onAttemptCallback.accept(numberOfDeleteAttempts.get());
923922
numberOfDeleteAttempts.incrementAndGet();
924923
if (throttleTimesBeforeSuccess.getAndDecrement() > 0) {
@@ -1022,7 +1021,7 @@ public void testSuppressedDeletionErrorsAreCapped() {
10221021
int maxBulkDeleteSize = randomIntBetween(1, 10);
10231022
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, maxBulkDeleteSize);
10241023
httpServer.createContext("/", exchange -> {
1025-
if (exchange.getRequestMethod().equals("POST") && exchange.getRequestURI().toString().startsWith("/bucket/?delete")) {
1024+
if (isMultiDeleteRequest(exchange)) {
10261025
exchange.sendResponseHeaders(
10271026
randomFrom(
10281027
HttpStatus.SC_INTERNAL_SERVER_ERROR,
@@ -1056,7 +1055,7 @@ public void testTrimmedLogAndCappedSuppressedErrorOnMultiObjectDeletionException
10561055

10571056
final Pattern pattern = Pattern.compile("<Key>(.+?)</Key>");
10581057
httpServer.createContext("/", exchange -> {
1059-
if (exchange.getRequestMethod().equals("POST") && exchange.getRequestURI().toString().startsWith("/bucket/?delete")) {
1058+
if (isMultiDeleteRequest(exchange)) {
10601059
final String requestBody = Streams.copyToString(new InputStreamReader(exchange.getRequestBody(), StandardCharsets.UTF_8));
10611060
final var matcher = pattern.matcher(requestBody);
10621061
final StringBuilder deletes = new StringBuilder();
@@ -1208,6 +1207,10 @@ private Map<String, Object> metricAttributes(String action) {
12081207
return Map.of("repo_type", "s3", "repo_name", "repository", "operation", "GetObject", "purpose", "Indices", "action", action);
12091208
}
12101209

1210+
private static boolean isMultiDeleteRequest(HttpExchange exchange) {
1211+
return new S3HttpHandler("bucket").parseRequest(exchange).isMultiObjectDeleteRequest();
1212+
}
1213+
12111214
/**
12121215
* Asserts that an InputStream is fully consumed, or aborted, when it is closed
12131216
*/

0 commit comments

Comments
 (0)