Skip to content

Commit 60aeaed

Browse files
committed
fix: use ThreadLocalRandom for backoff jitter, disable OkHttp retry, strengthen test assertions
Replace the shared static Random instance with ThreadLocalRandom.current() at the backoff call site and for the fan-out name generator in MinioAsyncClient — eliminating shared-state CAS contention under concurrent retry load. Disable OkHttp's retryOnConnectionFailure in doExecuteAsync so the SDK's executeWithRetry is the sole retry policy; previously OkHttp could silently add an extra attempt on stale-connection IOExceptions, causing more total attempts than maxRetries. Strengthen the two remaining assertNotNull catch blocks in RetryTest (testMaxRetriesOneDisablesRetry, testSetMaxRetriesPostConstruction) to typed InvalidResponseException catches with responseCode()==500 assertions, matching the pattern applied to the other tests in the previous commit.
1 parent 53da859 commit 60aeaed

3 files changed

Lines changed: 10 additions & 10 deletions

File tree

api/src/main/java/io/minio/BaseS3Client.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,16 @@
5151
import java.io.OutputStreamWriter;
5252
import java.io.PrintWriter;
5353
import java.nio.charset.StandardCharsets;
54-
import java.security.SecureRandom;
5554
import java.util.Arrays;
5655
import java.util.Collections;
5756
import java.util.HashMap;
5857
import java.util.Locale;
5958
import java.util.Map;
60-
import java.util.Random;
6159
import java.util.Set;
6260
import java.util.concurrent.CompletableFuture;
6361
import java.util.concurrent.CompletionException;
6462
import java.util.concurrent.ConcurrentHashMap;
63+
import java.util.concurrent.ThreadLocalRandom;
6564
import java.util.concurrent.Executors;
6665
import java.util.concurrent.ScheduledExecutorService;
6766
import java.util.concurrent.TimeUnit;
@@ -94,7 +93,6 @@ public abstract class BaseS3Client implements AutoCloseable {
9493
"ServerSideEncryptionConfigurationNotFoundError";
9594
// maximum allowed bucket policy size is 20KiB
9695
protected static final int MAX_BUCKET_POLICY_SIZE = 20 * 1024;
97-
protected static final Random RANDOM = new Random(new SecureRandom().nextLong());
9896
protected static final ObjectMapper OBJECT_MAPPER =
9997
JsonMapper.builder()
10098
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
@@ -318,7 +316,7 @@ private CompletableFuture<Response> executeWithRetry(
318316
if (attempt + 1 >= maxAttempts || !Retry.isRetryable(cause)) {
319317
return Utils.<Response>failedFuture(cause);
320318
}
321-
long delayMs = Retry.computeBackoffMs(attempt + 1, RANDOM);
319+
long delayMs = Retry.computeBackoffMs(attempt + 1, ThreadLocalRandom.current());
322320
CompletableFuture<Response> retryFuture = new CompletableFuture<>();
323321
RETRY_SCHEDULER.schedule(
324322
() ->
@@ -351,7 +349,8 @@ private CompletableFuture<Response> doExecuteAsync(Http.S3Request s3request, Str
351349
PrintWriter traceStream = this.traceStream;
352350
if (traceStream != null) traceStream.print(request.httpTraces());
353351

354-
OkHttpClient httpClient = this.httpClient;
352+
OkHttpClient httpClient =
353+
this.httpClient.newBuilder().retryOnConnectionFailure(false).build();
355354
okhttp3.Request httpRequest = request.httpRequest();
356355
CompletableFuture<Response> completableFuture = newCompleteableFuture();
357356
httpClient

api/src/main/java/io/minio/MinioAsyncClient.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
import java.util.concurrent.ExecutorService;
9494
import java.util.concurrent.Executors;
9595
import java.util.concurrent.Future;
96+
import java.util.concurrent.ThreadLocalRandom;
9697
import java.util.concurrent.atomic.AtomicBoolean;
9798
import okhttp3.HttpUrl;
9899
import okhttp3.MediaType;
@@ -3370,7 +3371,7 @@ public CompletableFuture<PutObjectFanOutResponse> putObjectFanOut(PutObjectFanOu
33703371
// Build POST object data
33713372
String objectName =
33723373
"fan-out-"
3373-
+ new BigInteger(32, RANDOM).toString(32)
3374+
+ new BigInteger(32, ThreadLocalRandom.current()).toString(32)
33743375
+ "-"
33753376
+ System.currentTimeMillis();
33763377
PostPolicy policy =

api/src/test/java/io/minio/RetryTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -459,8 +459,8 @@ public void testMaxRetriesOneDisablesRetry() throws Exception {
459459
try {
460460
client.listBuckets();
461461
Assert.fail("expected exception");
462-
} catch (Exception e) {
463-
Assert.assertNotNull("expected non-null exception", e);
462+
} catch (InvalidResponseException e) {
463+
Assert.assertEquals(500, e.responseCode());
464464
}
465465
// maxRetries=1 means a single attempt; must not retry
466466
Assert.assertEquals(1, server.getRequestCount());
@@ -481,8 +481,8 @@ public void testSetMaxRetriesPostConstruction() throws Exception {
481481
try {
482482
client.listBuckets();
483483
Assert.fail("expected exception");
484-
} catch (Exception e) {
485-
Assert.assertNotNull("expected non-null exception", e);
484+
} catch (InvalidResponseException e) {
485+
Assert.assertEquals(500, e.responseCode());
486486
}
487487
Assert.assertEquals(1, server.getRequestCount());
488488
}

0 commit comments

Comments
 (0)