Skip to content

Commit 7fe6e87

Browse files
committed
fix(retry): drop public Javadoc links to package-private Retry; strip prior interceptor from network chain too; correct backoff range docs
Address Copilot review (PR #1701, review 4247997916): - BaseS3Client.setMaxRetries and MinioAsyncClient.Builder.maxRetries Javadocs no longer reference {@link Retry#MAX_RETRY}. The Retry class is package-private, so the link target was unresolvable from generated public docs. Replaced with the literal default {@code 10}, which is what callers actually see. - BaseS3Client.wrapWithRetry now also strips any prior RetryInterceptor from networkInterceptors(), not only the application-interceptor list. Previously the Javadoc claimed every prior RetryInterceptor was replaced, but only application interceptors were. A caller that had (mis)registered RetryInterceptor as a network interceptor would have ended up with two retry layers. Defends against that and brings the Javadoc claim into line with the code. - Retry.exponentialBackoffMs Javadoc previously stated the result range as [0, sleep]. The actual range is [1, sleep] because nextDouble() is in [0.0, 1.0) and the (long) cast truncates rand * sleep to at most sleep - 1, so the subtraction never reaches sleep itself. minio-go has the same off-by-one (Float64() is also [0.0, 1.0)); this is documented now rather than papered over, so callers reading the formula see the same bounds the implementation produces. Validated locally on Java 17: - :api:check (compile + spotless + spotbugs + tests) PASS - :api:javadoc PASS
1 parent 2f2fb4f commit 7fe6e87

3 files changed

Lines changed: 16 additions & 8 deletions

File tree

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,10 @@ protected BaseS3Client(BaseS3Client client) {
137137

138138
/**
139139
* Re-wires the retry interceptor on {@code client} so it reads {@link #maxRetries} from this
140-
* instance. Strips any prior {@link Http.RetryInterceptor} (e.g. one bound to a different
141-
* instance via the copy constructor) before installing this one.
140+
* instance. Strips any prior {@link Http.RetryInterceptor} from BOTH the application-interceptor
141+
* and network-interceptor chains (e.g. one bound to a different instance via the copy
142+
* constructor, or accidentally registered on the network chain) before installing this one as an
143+
* application interceptor.
142144
*
143145
* <p>Also forces {@code retryOnConnectionFailure(false)} on the underlying client. The SDK's own
144146
* {@link Http.RetryInterceptor} is the single source of retry policy; layering OkHttp's
@@ -149,12 +151,13 @@ protected BaseS3Client(BaseS3Client client) {
149151
private OkHttpClient wrapWithRetry(OkHttpClient client) {
150152
OkHttpClient.Builder builder = client.newBuilder().retryOnConnectionFailure(false);
151153
builder.interceptors().removeIf(i -> i instanceof Http.RetryInterceptor);
154+
builder.networkInterceptors().removeIf(i -> i instanceof Http.RetryInterceptor);
152155
return builder.addInterceptor(new Http.RetryInterceptor(() -> this.maxRetries)).build();
153156
}
154157

155158
/**
156159
* Sets the maximum number of attempts for transient HTTP failures. Pass {@code 1} to disable
157-
* automatic retries. Defaults to {@link Retry#MAX_RETRY}.
160+
* automatic retries. Defaults to {@code 10}.
158161
*
159162
* @param maxRetries maximum attempts (must be {@code >= 1}).
160163
*/

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ public Builder credentialsProvider(Provider provider) {
210210
* Supplies a custom {@link OkHttpClient}. The SDK wraps this client to install {@link
211211
* Http.RetryInterceptor} and forces {@code retryOnConnectionFailure(false)} so that {@link
212212
* Http.RetryInterceptor} owns the entire retry policy. Any prior {@code RetryInterceptor} on
213-
* the supplied client is replaced.
213+
* either the application-interceptor or network-interceptor chain of the supplied client is
214+
* stripped before the new one is installed (as an application interceptor).
214215
*/
215216
public Builder httpClient(OkHttpClient httpClient) {
216217
Utils.validateNotNull(httpClient, "http client");
@@ -231,7 +232,7 @@ public Builder httpClient(OkHttpClient httpClient, boolean close) {
231232

232233
/**
233234
* Sets the maximum number of attempts per request. Pass {@code 1} to disable automatic retries.
234-
* Defaults to {@link Retry#MAX_RETRY}.
235+
* Defaults to {@code 10}.
235236
*/
236237
public Builder maxRetries(int maxRetries) {
237238
if (maxRetries < 1) throw new IllegalArgumentException("maxRetries must be >= 1");

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,15 @@ static boolean isRequestErrorRetryable(IOException e) {
114114
*
115115
* <pre>
116116
* sleep = min(DEFAULT_RETRY_CAP_MS, DEFAULT_RETRY_UNIT_MS * 2^attempt)
117-
* sleep -= random.nextDouble() * sleep * MAX_JITTER // full jitter when MAX_JITTER == 1.0
117+
* sleep -= (long)(random.nextDouble() * sleep * MAX_JITTER) // full jitter when MAX_JITTER == 1.0
118118
* </pre>
119119
*
120-
* <p>With {@code MAX_JITTER == 1.0}, returns a uniform random value in {@code [0, min(cap, base *
121-
* 2^attempt)]}.
120+
* <p>With {@code MAX_JITTER == 1.0}, returns a value in {@code [1, min(cap, base * 2^attempt)]}.
121+
* The lower bound is {@code 1} rather than {@code 0} because {@link
122+
* java.util.concurrent.ThreadLocalRandom#nextDouble()} is in {@code [0.0, 1.0)} and the {@code
123+
* (long)} cast truncates {@code rand * sleep} to at most {@code sleep - 1}. This matches the
124+
* behaviour of minio-go's {@code exponentialBackoffWait}, which uses the same formula and
125+
* therefore the same bounds.
122126
*/
123127
static long exponentialBackoffMs(int attempt) {
124128
int exp = Math.min(Math.max(attempt, 0), 30);

0 commit comments

Comments
 (0)