Skip to content

Commit 6662a8b

Browse files
pxsalehipxsalehi
committed
address review comments
1 parent 6b85c33 commit 6662a8b

File tree

5 files changed

+33
-39
lines changed

5 files changed

+33
-39
lines changed

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ protected BlobContainer createBlobContainer(
115115
final @Nullable Integer maxRetries,
116116
final @Nullable TimeValue readTimeout,
117117
final @Nullable Boolean disableChunkedEncoding,
118-
final @Nullable ByteSizeValue bufferSize
118+
final @Nullable ByteSizeValue bufferSize,
119+
final @Nullable Integer maxBulkDeletes
119120
) {
120121
final Settings.Builder clientSettings = Settings.builder();
121122
final String client = randomAlphaOfLength(5).toLowerCase(Locale.ROOT);
@@ -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);
180+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null);
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);
209+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null);
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);
251+
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, null, null, null);
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);
304+
final BlobContainer blobContainer = createBlobContainer(nbErrors + 1, readTimeout, null, null, null);
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);
444+
final BlobContainer blobContainer = createBlobContainer(1, null, null, null, null);
444445
httpServer.createContext("/batch/storage/v1", safeHandler(exchange -> {
445446
assert pendingDeletes.get() <= MAX_DELETES_PER_BATCH;
446447

@@ -476,7 +477,7 @@ public void testCompareAndExchangeWhenThrottled() throws IOException {
476477
httpServer.createContext("/", new ResponseInjectingHttpHandler(requestHandlers, new GoogleCloudStorageHttpHandler("bucket")));
477478

478479
final int maxRetries = randomIntBetween(1, 3);
479-
final BlobContainer container = createBlobContainer(maxRetries, null, null, null);
480+
final BlobContainer container = createBlobContainer(maxRetries, null, null, null, null);
480481
final byte[] data = randomBytes(randomIntBetween(1, BlobContainerUtils.MAX_REGISTER_CONTENT_LENGTH));
481482
final String key = randomIdentifier();
482483

‎modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ public BlobContainer blobContainer(BlobPath path) {
342342
}
343343

344344
private static class DeletionExceptions {
345-
private Exception exception = null;
345+
Exception exception = null;
346346
private int count = 0;
347347

348348
void useOrMaybeSuppress(Exception e) {

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

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,6 @@ protected Class<? extends Exception> unresponsiveExceptionType() {
158158
}
159159

160160
@Override
161-
protected BlobContainer createBlobContainer(
162-
final @Nullable Integer maxRetries,
163-
final @Nullable TimeValue readTimeout,
164-
final @Nullable Boolean disableChunkedEncoding,
165-
final @Nullable ByteSizeValue bufferSize
166-
) {
167-
return createBlobContainer(maxRetries, readTimeout, disableChunkedEncoding, bufferSize, null);
168-
}
169-
170161
protected BlobContainer createBlobContainer(
171162
final @Nullable Integer maxRetries,
172163
final @Nullable TimeValue readTimeout,
@@ -265,7 +256,7 @@ public void testWriteBlobWithRetries() throws Exception {
265256
final int maxRetries = randomInt(5);
266257
final CountDown countDown = new CountDown(maxRetries + 1);
267258

268-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null);
259+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null, null);
269260

270261
final byte[] bytes = randomBlobContent();
271262
httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_blob_max_retries"), exchange -> {
@@ -315,7 +306,7 @@ public void testWriteBlobWithRetries() throws Exception {
315306
public void testWriteBlobWithReadTimeouts() {
316307
final byte[] bytes = randomByteArrayOfLength(randomIntBetween(10, 128));
317308
final TimeValue readTimeout = TimeValue.timeValueMillis(randomIntBetween(100, 500));
318-
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null);
309+
final BlobContainer blobContainer = createBlobContainer(1, readTimeout, true, null, null);
319310

320311
// HTTP server does not send a response
321312
httpServer.createContext(downloadStorageEndpoint(blobContainer, "write_blob_timeout"), exchange -> {
@@ -353,7 +344,7 @@ public void testWriteBlobWithExceptionThrownAtClosingTime() throws Exception {
353344
var maxRetries = randomInt(3);
354345
var blobLength = randomIntBetween(1, 4096 * 3);
355346
var blobName = getTestName().toLowerCase(Locale.ROOT);
356-
var blobContainer = createBlobContainer(maxRetries, null, true, null);
347+
var blobContainer = createBlobContainer(maxRetries, null, true, null, null);
357348

358349
var uploadedBytes = new AtomicReference<BytesReference>();
359350
httpServer.createContext(downloadStorageEndpoint(blobContainer, blobName), exchange -> {
@@ -400,7 +391,7 @@ public void testWriteLargeBlob() throws Exception {
400391
final boolean useTimeout = rarely();
401392
final TimeValue readTimeout = useTimeout ? TimeValue.timeValueMillis(randomIntBetween(100, 500)) : null;
402393
final ByteSizeValue bufferSize = ByteSizeValue.of(5, ByteSizeUnit.MB);
403-
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize);
394+
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize, null);
404395

405396
final int parts = randomIntBetween(1, 5);
406397
final long lastPartSize = randomLongBetween(10, 512);
@@ -501,7 +492,7 @@ public void testWriteLargeBlobStreaming() throws Exception {
501492
final boolean useTimeout = rarely();
502493
final TimeValue readTimeout = useTimeout ? TimeValue.timeValueMillis(randomIntBetween(100, 500)) : null;
503494
final ByteSizeValue bufferSize = ByteSizeValue.of(5, ByteSizeUnit.MB);
504-
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize);
495+
final BlobContainer blobContainer = createBlobContainer(null, readTimeout, true, bufferSize, null);
505496

506497
final int parts = randomIntBetween(1, 5);
507498
final long lastPartSize = randomLongBetween(10, 512);
@@ -615,7 +606,7 @@ public void testReadRetriesAfterMeaningfulProgress() throws Exception {
615606
0,
616607
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
617608
);
618-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes));
609+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null);
619610
final int meaningfulProgressBytes = Math.max(1, bufferSizeBytes / 100);
620611

621612
final byte[] bytes = randomBlobContent();
@@ -688,7 +679,7 @@ public void testReadDoesNotRetryForRepositoryAnalysis() {
688679
0,
689680
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
690681
);
691-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes));
682+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null);
692683

693684
final byte[] bytes = randomBlobContent();
694685

@@ -726,7 +717,7 @@ public void testReadWithIndicesPurposeRetriesForever() throws IOException {
726717
0,
727718
randomFrom(1000, Math.toIntExact(S3Repository.BUFFER_SIZE_SETTING.get(Settings.EMPTY).getBytes()))
728719
);
729-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes));
720+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, ByteSizeValue.ofBytes(bufferSizeBytes), null);
730721
final int meaningfulProgressBytes = Math.max(1, bufferSizeBytes / 100);
731722

732723
final byte[] bytes = randomBlobContent(512);
@@ -819,7 +810,7 @@ public void handle(HttpExchange exchange) throws IOException {
819810

820811
public void testDoesNotRetryOnNotFound() {
821812
final int maxRetries = between(3, 5);
822-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null);
813+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, true, null, null);
823814

824815
final AtomicInteger numberOfReads = new AtomicInteger(0);
825816
@SuppressForbidden(reason = "use a http server")
@@ -851,7 +842,7 @@ public void handle(HttpExchange exchange) throws IOException {
851842

852843
public void testSnapshotDeletesRetryOnThrottlingError() throws IOException {
853844
// disable AWS-client retries
854-
final BlobContainer blobContainer = createBlobContainer(0, null, true, null);
845+
final BlobContainer blobContainer = createBlobContainer(0, null, true, null, null);
855846

856847
int numBlobsToDelete = randomIntBetween(500, 3000);
857848
List<String> blobsToDelete = new ArrayList<>();
@@ -871,7 +862,7 @@ public void testSnapshotDeletesRetryOnThrottlingError() throws IOException {
871862

872863
public void testSnapshotDeletesAbortRetriesWhenThreadIsInterrupted() {
873864
// disable AWS-client retries
874-
final BlobContainer blobContainer = createBlobContainer(0, null, true, null);
865+
final BlobContainer blobContainer = createBlobContainer(0, null, true, null, null);
875866

876867
int numBlobsToDelete = randomIntBetween(500, 3000);
877868
List<String> blobsToDelete = new ArrayList<>();
@@ -908,7 +899,7 @@ public void testSnapshotDeletesAbortRetriesWhenThreadIsInterrupted() {
908899

909900
public void testNonSnapshotDeletesAreNotRetried() {
910901
// disable AWS-client retries
911-
final BlobContainer blobContainer = createBlobContainer(0, null, true, null);
902+
final BlobContainer blobContainer = createBlobContainer(0, null, true, null, null);
912903

913904
int numBlobsToDelete = randomIntBetween(500, 3000);
914905
List<String> blobsToDelete = new ArrayList<>();
@@ -937,7 +928,7 @@ public void testNonSnapshotDeletesAreNotRetried() {
937928

938929
public void testNonThrottlingErrorsAreNotRetried() {
939930
// disable AWS-client retries
940-
final BlobContainer blobContainer = createBlobContainer(0, null, true, null);
931+
final BlobContainer blobContainer = createBlobContainer(0, null, true, null, null);
941932

942933
int numBlobsToDelete = randomIntBetween(500, 3000);
943934
List<String> blobsToDelete = new ArrayList<>();
@@ -1016,7 +1007,7 @@ private Set<OperationPurpose> operationPurposesThatRetryOnDelete() {
10161007

10171008
public void testGetRegisterRetries() {
10181009
final var maxRetries = between(0, 3);
1019-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null);
1010+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null);
10201011

10211012
interface FailingHandlerFactory {
10221013
void addHandler(String blobName, Integer... responseCodes);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ protected BlobContainer createBlobContainer(
7676
Integer maxRetries,
7777
TimeValue readTimeout,
7878
Boolean disableChunkedEncoding,
79-
ByteSizeValue bufferSize
79+
ByteSizeValue bufferSize,
80+
Integer maxBulkDeletes
8081
) {
8182
Settings.Builder settingsBuilder = Settings.builder();
8283

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ protected abstract BlobContainer createBlobContainer(
8282
@Nullable Integer maxRetries,
8383
@Nullable TimeValue readTimeout,
8484
@Nullable Boolean disableChunkedEncoding,
85-
@Nullable ByteSizeValue bufferSize
85+
@Nullable ByteSizeValue bufferSize,
86+
@Nullable Integer maxBulkDeletes
8687
);
8788

8889
protected org.hamcrest.Matcher<Object> readTimeoutExceptionMatcher() {
@@ -91,7 +92,7 @@ protected org.hamcrest.Matcher<Object> readTimeoutExceptionMatcher() {
9192
}
9293

9394
public void testReadNonexistentBlobThrowsNoSuchFileException() {
94-
final BlobContainer blobContainer = createBlobContainer(between(1, 5), null, null, null);
95+
final BlobContainer blobContainer = createBlobContainer(between(1, 5), null, null, null, null);
9596
final long position = randomLongBetween(0, MAX_RANGE_VAL);
9697
final int length = randomIntBetween(1, Math.toIntExact(Math.min(Integer.MAX_VALUE, MAX_RANGE_VAL - position)));
9798
final Exception exception = expectThrows(NoSuchFileException.class, () -> {
@@ -118,7 +119,7 @@ public void testReadBlobWithRetries() throws Exception {
118119

119120
final byte[] bytes = randomBlobContent();
120121
final TimeValue readTimeout = TimeValue.timeValueSeconds(between(1, 3));
121-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null);
122+
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null);
122123
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_max_retries"), exchange -> {
123124
Streams.readFully(exchange.getRequestBody());
124125
if (countDown.countDown()) {
@@ -175,7 +176,7 @@ public void testReadRangeBlobWithRetries() throws Exception {
175176
final CountDown countDown = new CountDown(maxRetries + 1);
176177

177178
final TimeValue readTimeout = TimeValue.timeValueSeconds(between(5, 10));
178-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null);
179+
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null);
179180
final byte[] bytes = randomBlobContent();
180181
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_range_blob_max_retries"), exchange -> {
181182
Streams.readFully(exchange.getRequestBody());
@@ -247,7 +248,7 @@ public void testReadRangeBlobWithRetries() throws Exception {
247248
public void testReadBlobWithReadTimeouts() {
248249
final int maxRetries = randomInt(5);
249250
final TimeValue readTimeout = TimeValue.timeValueMillis(between(100, 200));
250-
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null);
251+
final BlobContainer blobContainer = createBlobContainer(maxRetries, readTimeout, null, null, null);
251252

252253
// HTTP server does not send a response
253254
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_unresponsive"), exchange -> {});
@@ -304,7 +305,7 @@ protected OperationPurpose randomFiniteRetryingPurpose() {
304305

305306
public void testReadBlobWithNoHttpResponse() {
306307
final TimeValue readTimeout = TimeValue.timeValueMillis(between(100, 200));
307-
final BlobContainer blobContainer = createBlobContainer(randomInt(5), readTimeout, null, null);
308+
final BlobContainer blobContainer = createBlobContainer(randomInt(5), readTimeout, null, null, null);
308309

309310
// HTTP server closes connection immediately
310311
httpServer.createContext(downloadStorageEndpoint(blobContainer, "read_blob_no_response"), HttpExchange::close);
@@ -324,7 +325,7 @@ public void testReadBlobWithNoHttpResponse() {
324325

325326
public void testReadBlobWithPrematureConnectionClose() {
326327
final int maxRetries = randomInt(20);
327-
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null);
328+
final BlobContainer blobContainer = createBlobContainer(maxRetries, null, null, null, null);
328329

329330
final boolean alwaysFlushBody = randomBoolean();
330331

0 commit comments

Comments
 (0)