Skip to content

Commit 446dc03

Browse files
Introduce TestBlobContainerBuilder (#126483)
The mostly-optional parameters to `createBlobContainer` are getting rather numerous in this test harness which makes the tests hard to read. This commit introduces a builder to help name the provided parameters and skip the omitted ones. It also adds a `maxConnections` override. Backport of #126445 and #126435 to `8.x` Co-authored-by: Dianna Hohensee <[email protected]>
1 parent 1384959 commit 446dc03

File tree

4 files changed

+139
-30
lines changed

4 files changed

+139
-30
lines changed

modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainerRetriesTests.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ protected BlobContainer createBlobContainer(
111111
final @Nullable Integer maxRetries,
112112
final @Nullable TimeValue readTimeout,
113113
final @Nullable Boolean disableChunkedEncoding,
114+
final @Nullable Integer maxConnections,
114115
final @Nullable ByteSizeValue bufferSize,
115116
final @Nullable Integer maxBulkDeletes,
116117
final @Nullable BlobPath blobContainerPath
@@ -176,7 +177,7 @@ public void testReadLargeBlobWithRetries() throws Exception {
176177
final int maxRetries = randomIntBetween(2, 10);
177178
final AtomicInteger countDown = new AtomicInteger(maxRetries);
178179

179-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null, null);
180+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).build();
180181

181182
// SDK reads in 2 MB chunks so we use twice that to simulate 2 chunks
182183
final byte[] bytes = randomBytes(1 << 22);
@@ -205,7 +206,7 @@ public void testWriteBlobWithRetries() throws Exception {
205206
final int maxRetries = randomIntBetween(2, 10);
206207
final CountDown countDown = new CountDown(maxRetries);
207208

208-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null, null);
209+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).build();
209210
final byte[] bytes = randomBlobContent();
210211
httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> {
211212
assertThat(exchange.getRequestURI().getQuery(), containsString("uploadType=multipart"));
@@ -247,7 +248,7 @@ public void testWriteBlobWithRetries() throws Exception {
247248
public void testWriteBlobWithReadTimeouts() {
248249
final byte[] bytes = randomByteArrayOfLength(randomIntBetween(10, 128));
249250
final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
250-
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, null, null, null, null);
251+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(1).readTimeout(readTimeout).build();
251252

252253
// HTTP server does not send a response
253254
httpServer.createContext("/upload/storage/v1/b/bucket/o", exchange -> {
@@ -300,7 +301,7 @@ public void testWriteLargeBlob() throws IOException {
300301
logger.debug("starting with resumable upload id [{}]", sessionUploadId.get());
301302

302303
final TimeValue readTimeout = allowReadTimeout.get() ? TimeValue.timeValueSeconds(3) : null;
303-
final BlobContainer blobContainer = createBlobContainer(nbErrors + 1, readTimeout, null, null, null, null);
304+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(nbErrors + 1).readTimeout(readTimeout).build();
304305

305306
httpServer.createContext("/upload/storage/v1/b/bucket/o", safeHandler(exchange -> {
306307
final BytesReference requestBody = Streams.readFully(exchange.getRequestBody());
@@ -440,7 +441,7 @@ public String next() {
440441
return Integer.toString(totalDeletesSent++);
441442
}
442443
};
443-
final BlobContainer blobContainer = createBlobContainer(1, null, null, null, null, null);
444+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(1).build();
444445
httpServer.createContext("/batch/storage/v1", safeHandler(exchange -> {
445446
assert pendingDeletes.get() <= MAX_DELETES_PER_BATCH;
446447

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

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose;
8282
import static org.elasticsearch.repositories.s3.S3ClientSettings.DISABLE_CHUNKED_ENCODING;
8383
import static org.elasticsearch.repositories.s3.S3ClientSettings.ENDPOINT_SETTING;
84+
import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_CONNECTIONS_SETTING;
8485
import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_RETRIES_SETTING;
8586
import static org.elasticsearch.repositories.s3.S3ClientSettings.READ_TIMEOUT_SETTING;
8687
import static org.hamcrest.Matchers.allOf;
@@ -154,6 +155,7 @@ protected BlobContainer createBlobContainer(
154155
final @Nullable Integer maxRetries,
155156
final @Nullable TimeValue readTimeout,
156157
final @Nullable Boolean disableChunkedEncoding,
158+
final @Nullable Integer maxConnections,
157159
final @Nullable ByteSizeValue bufferSize,
158160
final @Nullable Integer maxBulkDeletes,
159161
final @Nullable BlobPath blobContainerPath
@@ -174,6 +176,9 @@ protected BlobContainer createBlobContainer(
174176
if (disableChunkedEncoding != null) {
175177
clientSettings.put(DISABLE_CHUNKED_ENCODING.getConcreteSettingForNamespace(clientName).getKey(), disableChunkedEncoding);
176178
}
179+
if (maxConnections != null) {
180+
clientSettings.put(MAX_CONNECTIONS_SETTING.getConcreteSettingForNamespace(clientName).getKey(), maxConnections);
181+
}
177182

178183
final MockSecureSettings secureSettings = new MockSecureSettings();
179184
secureSettings.setString(
@@ -253,7 +258,7 @@ public void testWriteBlobWithRetries() throws Exception {
253258
final int maxRetries = randomInt(5);
254259
final CountDown countDown = new CountDown(maxRetries + 1);
255260

256-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null, null, null);
261+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).disableChunkedEncoding(true).build();
257262

258263
final byte[] bytes = randomBlobContent();
259264
httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_blob_max_retries"), exchange -> {
@@ -301,7 +306,10 @@ public void testWriteBlobWithRetries() throws Exception {
301306
public void testWriteBlobWithReadTimeouts() {
302307
final byte[] bytes = randomByteArrayOfLength(randomIntBetween(10, 128));
303308
final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
304-
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, null, null);
309+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(1)
310+
.readTimeout(readTimeout)
311+
.disableChunkedEncoding(true)
312+
.build();
305313

306314
// HTTP server does not send a response
307315
httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_blob_timeout"), exchange -> {
@@ -335,7 +343,10 @@ public void testWriteLargeBlob() throws Exception {
335343
final boolean useTimeout = rarely();
336344
final TimeValue readTimeout = useTimeout ? TimeValue.timeValueMillis(randomIntBetween(100, 500)) : null;
337345
final ByteSizeValue bufferSize = new ByteSizeValue(5, ByteSizeUnit.MB);
338-
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize, null, null);
346+
final BlobContainer blobContainer = blobContainerBuilder().readTimeout(readTimeout)
347+
.disableChunkedEncoding(true)
348+
.bufferSize(bufferSize)
349+
.build();
339350

340351
final int parts = randomIntBetween(1, 5);
341352
final long lastPartSize = randomLongBetween(10, 512);
@@ -431,7 +442,10 @@ public void testWriteLargeBlobStreaming() throws Exception {
431442
final boolean useTimeout = rarely();
432443
final TimeValue readTimeout = useTimeout ? TimeValue.timeValueMillis(randomIntBetween(100, 500)) : null;
433444
final ByteSizeValue bufferSize = new ByteSizeValue(5, ByteSizeUnit.MB);
434-
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize, null, null);
445+
final BlobContainer blobContainer = blobContainerBuilder().readTimeout(readTimeout)
446+
.disableChunkedEncoding(true)
447+
.bufferSize(bufferSize)
448+
.build();
435449

436450
final int parts = randomIntBetween(1, 5);
437451
final long lastPartSize = randomLongBetween(10, 512);
@@ -540,7 +554,10 @@ public void testReadRetriesAfterMeaningfulProgress() throws Exception {
540554
0,
541555
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
542556
);
543-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null, null);
557+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries)
558+
.disableChunkedEncoding(true)
559+
.bufferSize(ByteSizeValue.ofBytes(bufferSizeBytes))
560+
.build();
544561
final int meaningfulProgressBytes = Math.max(1, bufferSizeBytes / 100);
545562

546563
final byte[] bytes = randomBlobContent();
@@ -613,7 +630,10 @@ public void testReadDoesNotRetryForRepositoryAnalysis() {
613630
0,
614631
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
615632
);
616-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null, null);
633+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries)
634+
.disableChunkedEncoding(true)
635+
.bufferSize(ByteSizeValue.ofBytes(bufferSizeBytes))
636+
.build();
617637

618638
final byte[] bytes = randomBlobContent();
619639

@@ -651,7 +671,10 @@ public void testReadWithIndicesPurposeRetriesForever() throws IOException {
651671
0,
652672
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
653673
);
654-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null, null);
674+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries)
675+
.disableChunkedEncoding(true)
676+
.bufferSize(ByteSizeValue.ofBytes(bufferSizeBytes))
677+
.build();
655678
final int meaningfulProgressBytes = Math.max(1, bufferSizeBytes / 100);
656679

657680
final byte[] bytes = randomBlobContent(512);
@@ -744,7 +767,7 @@ public void handle(HttpExchange exchange) throws IOException {
744767

745768
public void testDoesNotRetryOnNotFound() {
746769
final int maxRetries = between(3, 5);
747-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null, null, null);
770+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).disableChunkedEncoding(true).build();
748771

749772
final AtomicInteger numberOfReads = new AtomicInteger(0);
750773
@SuppressForbidden(reason = "use a http server")
@@ -777,7 +800,11 @@ public void handle(HttpExchange exchange) throws IOException {
777800
public void testSuppressedDeletionErrorsAreCapped() {
778801
final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
779802
int maxBulkDeleteSize = randomIntBetween(1, 10);
780-
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, maxBulkDeleteSize, null);
803+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(1)
804+
.readTimeout(readTimeout)
805+
.disableChunkedEncoding(true)
806+
.maxBulkDeletes(maxBulkDeleteSize)
807+
.build();
781808
httpServer.createContext("/", exchange -> {
782809
if (isMultiDeleteRequest(exchange)) {
783810
exchange.sendResponseHeaders(
@@ -809,7 +836,11 @@ public void testSuppressedDeletionErrorsAreCapped() {
809836
public void testTrimmedLogAndCappedSuppressedErrorOnMultiObjectDeletionException() {
810837
final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
811838
int maxBulkDeleteSize = randomIntBetween(10, 30);
812-
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, maxBulkDeleteSize, null);
839+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(1)
840+
.readTimeout(readTimeout)
841+
.disableChunkedEncoding(true)
842+
.maxBulkDeletes(maxBulkDeleteSize)
843+
.build();
813844

814845
final Pattern pattern = Pattern.compile("<Key>(.+?)</Key>");
815846
httpServer.createContext("/", exchange -> {

modules/repository-url/src/test/java/org/elasticsearch/common/blobstore/url/URLBlobContainerRetriesTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ protected BlobContainer createBlobContainer(
7777
Integer maxRetries,
7878
TimeValue readTimeout,
7979
Boolean disableChunkedEncoding,
80+
Integer maxConnections,
8081
ByteSizeValue bufferSize,
8182
Integer maxBulkDeletes,
8283
BlobPath blobContainerPath

test/framework/src/main/java/org/elasticsearch/repositories/blobstore/AbstractBlobContainerRetriesTestCase.java

Lines changed: 91 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,13 @@ public void tearDown() throws Exception {
7979

8080
protected abstract Class<? extends Exception> unresponsiveExceptionType();
8181

82-
protected abstract BlobContainer createBlobContainer(
83-
@Nullable Integer maxRetries,
84-
@Nullable TimeValue readTimeout,
85-
@Nullable Boolean disableChunkedEncoding,
86-
@Nullable ByteSizeValue bufferSize,
87-
@Nullable Integer maxBulkDeletes,
88-
@Nullable BlobPath blobContainerPath
89-
);
90-
9182
protected org.hamcrest.Matcher<Object> readTimeoutExceptionMatcher() {
9283
return either(instanceOf(SocketTimeoutException.class)).or(instanceOf(ConnectionClosedException.class))
9384
.or(instanceOf(RuntimeException.class));
9485
}
9586

9687
public void testReadNonexistentBlobThrowsNoSuchFileException() {
97-
final BlobContainer blobContainer = createBlobContainer(between(1, 5), null, null, null, null, null);
88+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(between(1, 5)).build();
9889
final long position = randomLongBetween(0, MAX_RANGE_VAL);
9990
final int length = randomIntBetween(1, Math.toIntExact(Math.min(Integer.MAX_VALUE, MAX_RANGE_VAL - position)));
10091
final Exception exception = expectThrows(NoSuchFileException.class, () -> {
@@ -121,7 +112,7 @@ public void testReadBlobWithRetries() throws Exception {
121112

122113
final byte[] bytes = randomBlobContent();
123114
final TimeValue readTimeout = TimeValue.timeValueSeconds(between(1, 3));
124-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null, null);
115+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).readTimeout(readTimeout).build();
125116
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_max_retries"), exchange -> {
126117
Streams.readFully(exchange.getRequestBody());
127118
if (countDown.countDown()) {
@@ -178,7 +169,7 @@ public void testReadRangeBlobWithRetries() throws Exception {
178169
final CountDown countDown = new CountDown(maxRetries + 1);
179170

180171
final TimeValue readTimeout = TimeValue.timeValueSeconds(between(5, 10));
181-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null, null);
172+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).readTimeout(readTimeout).build();
182173
final byte[] bytes = randomBlobContent();
183174
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_range_blob_max_retries"), exchange -> {
184175
Streams.readFully(exchange.getRequestBody());
@@ -250,7 +241,7 @@ public void testReadRangeBlobWithRetries() throws Exception {
250241
public void testReadBlobWithReadTimeouts() {
251242
final int maxRetries = randomInt(5);
252243
final TimeValue readTimeout = TimeValue.timeValueMillis(between(100, 200));
253-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null, null);
244+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).readTimeout(readTimeout).build();
254245

255246
// HTTP server does not send a response
256247
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_unresponsive"), exchange -> {});
@@ -307,7 +298,7 @@ protected OperationPurpose randomFiniteRetryingPurpose() {
307298

308299
public void testReadBlobWithNoHttpResponse() {
309300
final TimeValue readTimeout = TimeValue.timeValueMillis(between(100, 200));
310-
final BlobContainer blobContainer = createBlobContainer(randomInt(5), readTimeout, null, null, null, null);
301+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(randomInt(5)).readTimeout(readTimeout).build();
311302

312303
// HTTP server closes connection immediately
313304
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_no_response"), HttpExchange::close);
@@ -327,7 +318,7 @@ public void testReadBlobWithNoHttpResponse() {
327318

328319
public void testReadBlobWithPrematureConnectionClose() {
329320
final int maxRetries = randomInt(20);
330-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null, null);
321+
final BlobContainer blobContainer = blobContainerBuilder().maxRetries(maxRetries).build();
331322

332323
final boolean alwaysFlushBody = randomBoolean();
333324

@@ -504,4 +495,89 @@ private void ensureOpen() throws IOException {
504495
}
505496
}
506497
}
498+
499+
/**
500+
* Method for subclasses to define how to create a {@link BlobContainer} with the given (optional) parameters. Callers should use
501+
* {@link #blobContainerBuilder} to obtain a builder to construct the arguments to this method rather than calling it directly.
502+
*/
503+
protected abstract BlobContainer createBlobContainer(
504+
@Nullable Integer maxRetries,
505+
@Nullable TimeValue readTimeout,
506+
@Nullable Boolean disableChunkedEncoding,
507+
@Nullable Integer maxConnections,
508+
@Nullable ByteSizeValue bufferSize,
509+
@Nullable Integer maxBulkDeletes,
510+
@Nullable BlobPath blobContainerPath
511+
);
512+
513+
protected final class TestBlobContainerBuilder {
514+
@Nullable
515+
private Integer maxRetries;
516+
@Nullable
517+
private TimeValue readTimeout;
518+
@Nullable
519+
private Boolean disableChunkedEncoding;
520+
@Nullable
521+
private Integer maxConnections;
522+
@Nullable
523+
private ByteSizeValue bufferSize;
524+
@Nullable
525+
private Integer maxBulkDeletes;
526+
@Nullable
527+
private BlobPath blobContainerPath;
528+
529+
public TestBlobContainerBuilder maxRetries(@Nullable Integer maxRetries) {
530+
this.maxRetries = maxRetries;
531+
return this;
532+
}
533+
534+
public TestBlobContainerBuilder readTimeout(@Nullable TimeValue readTimeout) {
535+
this.readTimeout = readTimeout;
536+
return this;
537+
}
538+
539+
public TestBlobContainerBuilder disableChunkedEncoding(@Nullable Boolean disableChunkedEncoding) {
540+
this.disableChunkedEncoding = disableChunkedEncoding;
541+
return this;
542+
}
543+
544+
public TestBlobContainerBuilder maxConnections(@Nullable Integer maxConnections) {
545+
this.maxConnections = maxConnections;
546+
return this;
547+
}
548+
549+
public TestBlobContainerBuilder bufferSize(@Nullable ByteSizeValue bufferSize) {
550+
this.bufferSize = bufferSize;
551+
return this;
552+
}
553+
554+
public TestBlobContainerBuilder maxBulkDeletes(@Nullable Integer maxBulkDeletes) {
555+
this.maxBulkDeletes = maxBulkDeletes;
556+
return this;
557+
}
558+
559+
public TestBlobContainerBuilder blobContainerPath(@Nullable BlobPath blobContainerPath) {
560+
this.blobContainerPath = blobContainerPath;
561+
return this;
562+
}
563+
564+
public BlobContainer build() {
565+
return createBlobContainer(
566+
maxRetries,
567+
readTimeout,
568+
disableChunkedEncoding,
569+
maxConnections,
570+
bufferSize,
571+
maxBulkDeletes,
572+
blobContainerPath
573+
);
574+
}
575+
}
576+
577+
/**
578+
* @return a {@link TestBlobContainerBuilder} to construct the arguments with which to call {@link #createBlobContainer}.
579+
*/
580+
protected final TestBlobContainerBuilder blobContainerBuilder() {
581+
return new TestBlobContainerBuilder();
582+
}
507583
}

0 commit comments

Comments
 (0)