Skip to content

Commit 75434a1

Browse files
committed
Simplify RetryState creation
JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141
1 parent 205e2d3 commit 75434a1

7 files changed

Lines changed: 77 additions & 168 deletions

File tree

driver-core/src/main/com/mongodb/internal/async/AsyncRunnable.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.mongodb.internal.async;
1818

19-
import com.mongodb.internal.TimeoutContext;
2019
import com.mongodb.internal.async.function.AsyncCallbackLoop;
2120
import com.mongodb.internal.async.function.LoopState;
2221
import com.mongodb.internal.async.function.RetryState;
@@ -39,15 +38,15 @@
3938
* following "sync" method:
4039
*
4140
* <pre>
42-
* public T myMethod()
41+
* public T myMethod() {
4342
* method1();
4443
* method2();
4544
* }</pre>
4645
*
4746
* <p>The async counterpart would be:
4847
*
4948
* <pre>
50-
* public void myMethodAsync(SingleResultCallback&lt;T> callback)
49+
* public void myMethodAsync(SingleResultCallback&lt;T> callback) {
5150
* beginAsync().thenRun(c -> {
5251
* method1Async(c);
5352
* }).thenRun(c -> {
@@ -229,11 +228,11 @@ default <R> AsyncSupplier<R> thenSupply(final AsyncSupplier<R> supplier) {
229228
* @return the composition of this, and the looping branch
230229
* @see RetryingAsyncCallbackSupplier
231230
*/
232-
default AsyncRunnable thenRunRetryingWhile(
233-
final TimeoutContext timeoutContext, final AsyncRunnable runnable, final Predicate<Throwable> shouldRetry) {
231+
default AsyncRunnable thenRunRetryingWhile(final AsyncRunnable runnable, final Predicate<Throwable> shouldRetry) {
234232
return thenRun(callback -> {
235233
new RetryingAsyncCallbackSupplier<Void>(
236-
new RetryState(timeoutContext),
234+
new RetryState(),
235+
(previouslyChosenFailure, lastAttemptFailure) -> lastAttemptFailure,
237236
(rs, lastAttemptFailure) -> shouldRetry.test(lastAttemptFailure),
238237
// `finish` is required here instead of `unsafeFinish`
239238
// because only `finish` meets the contract of

driver-core/src/main/com/mongodb/internal/async/function/RetryState.java

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.mongodb.MongoOperationTimeoutException;
1919
import com.mongodb.annotations.NotThreadSafe;
20-
import com.mongodb.internal.TimeoutContext;
2120
import com.mongodb.internal.async.SingleResultCallback;
2221
import com.mongodb.internal.async.function.LoopState.AttachmentKey;
2322
import com.mongodb.lang.NonNull;
@@ -46,62 +45,31 @@
4645
*/
4746
@NotThreadSafe
4847
public final class RetryState {
49-
public static final int RETRIES = 1;
50-
public static final int INFINITE_ATTEMPTS = Integer.MAX_VALUE;
48+
public static final int MAX_RETRIES = 1;
49+
private static final int INFINITE_RETRIES = Integer.MAX_VALUE;
5150

5251
private final LoopState loopState;
5352
private final int attempts;
54-
private final boolean retryUntilTimeoutThrowsException;
5553
@Nullable
5654
private Throwable previouslyChosenException;
5755

5856
/**
59-
* Creates a {@code RetryState} with a positive number of allowed retry attempts.
60-
* {@link Integer#MAX_VALUE} is a special value interpreted as being unlimited.
61-
* <p>
62-
* If a timeout is not specified in the {@link TimeoutContext#hasTimeoutMS()}, the specified {@code retries} argument acts as a fallback
63-
* bound. Otherwise, retries are unbounded until the timeout is reached.
64-
* <p>
65-
* It is possible to provide an additional {@code retryPredicate} in the {@link #doAdvanceOrThrow} method,
66-
* which can be used to stop retrying based on a custom condition additionally to {@code retries} and {@link TimeoutContext}.
67-
* </p>
68-
*
69-
* @param retries A positive number of allowed retry attempts.
70-
* {@link Integer#MAX_VALUE} is a special value interpreted as being unlimited.
71-
* @param retryUntilTimeoutThrowsException If {@code true}, then if a {@link MongoOperationTimeoutException} is thrown then retrying stops.
72-
*/
73-
public static RetryState withRetryableState(final int retries, final boolean retryUntilTimeoutThrowsException) {
74-
assertTrue(retries > 0);
75-
return new RetryState(retries, retryUntilTimeoutThrowsException);
76-
}
77-
78-
public static RetryState withNonRetryableState() {
79-
return new RetryState(0, false);
80-
}
81-
82-
/**
83-
* Creates a {@link RetryState} that does not limit the number of attempts.
84-
* The number of attempts is limited iff {@link TimeoutContext#hasTimeoutMS()} is true and timeout has expired.
85-
* <p>
86-
* It is possible to provide an additional {@code retryPredicate} in the {@link #doAdvanceOrThrow} method,
87-
* which can be used to stop retrying based on a custom condition additionally to {@link TimeoutContext}.
88-
* </p>
89-
*
90-
* @param timeoutContext A timeout context that will be used to determine if the operation has timed out.
57+
* Creates a {@link RetryState} that does not explicitly limit the number of attempts.
58+
* Retrying still may be stopped because, for example,
59+
* the failed result from the most recent attempt is {@link MongoOperationTimeoutException}.
9160
*/
92-
public RetryState(final TimeoutContext timeoutContext) {
93-
this(INFINITE_ATTEMPTS, timeoutContext.hasTimeoutMS());
61+
public RetryState() {
62+
this(INFINITE_RETRIES);
9463
}
9564

9665
/**
9766
* @param retries A non-negative number of allowed retry attempts.
98-
* {@link Integer#MAX_VALUE} is a special value interpreted as being unlimited.
67+
* {@value #INFINITE_RETRIES} is interpreted as {@linkplain #RetryState() absence of explicit limit}.
9968
*/
100-
private RetryState(final int retries, final boolean retryUntilTimeoutThrowsException) {
69+
public RetryState(final int retries) {
10170
assertTrue(retries >= 0);
10271
loopState = new LoopState();
103-
attempts = retries == INFINITE_ATTEMPTS ? INFINITE_ATTEMPTS : retries + 1;
104-
this.retryUntilTimeoutThrowsException = retryUntilTimeoutThrowsException;
72+
attempts = retries == INFINITE_RETRIES ? INFINITE_RETRIES : retries + 1;
10573
}
10674

10775
/**
@@ -358,7 +326,7 @@ public boolean isFirstAttempt() {
358326
* @see #attempt()
359327
*/
360328
private boolean isLastAttempt(final Throwable attemptException) {
361-
boolean operationTimeout = retryUntilTimeoutThrowsException && attemptException instanceof MongoOperationTimeoutException;
329+
boolean operationTimeout = attemptException instanceof MongoOperationTimeoutException;
362330
boolean attemptLimit = attempt() == attempts - 1;
363331
return loopState.isLastIteration() || operationTimeout || attemptLimit;
364332
}
@@ -403,7 +371,7 @@ public <V> Optional<V> attachment(final AttachmentKey<V> key) {
403371
public String toString() {
404372
return "RetryState{"
405373
+ "loopState=" + loopState
406-
+ ", attempts=" + (attempts == INFINITE_ATTEMPTS ? "infinite" : attempts)
374+
+ ", attempts=" + (attempts == INFINITE_RETRIES ? "infinite" : attempts)
407375
+ ", exception=" + previouslyChosenException
408376
+ '}';
409377
}

driver-core/src/main/com/mongodb/internal/connection/OidcAuthenticator.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,6 @@ private void authenticationLoopAsync(final InternalConnection connection, final
335335
final SingleResultCallback<Void> callback) {
336336
fallbackState = FallbackState.INITIAL;
337337
beginAsync().thenRunRetryingWhile(
338-
operationContext.getTimeoutContext(),
339338
c -> super.authenticateAsync(connection, description, operationContext, c),
340339
e -> triggersRetry(e) && shouldRetryHandler()
341340
).finish(callback);

driver-core/src/main/com/mongodb/internal/operation/CommandOperationHelper.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545

4646
import static com.mongodb.assertions.Assertions.assertFalse;
4747
import static com.mongodb.assertions.Assertions.assertNotNull;
48-
import static com.mongodb.internal.async.function.RetryState.INFINITE_ATTEMPTS;
48+
import static com.mongodb.internal.async.function.RetryState.MAX_RETRIES;
4949
import static com.mongodb.internal.operation.OperationHelper.LOGGER;
5050
import static java.lang.String.format;
5151
import static java.util.Arrays.asList;
@@ -123,11 +123,9 @@ private static Throwable chooseRetryableWriteException(
123123

124124
static RetryState initialRetryState(final boolean retry, final TimeoutContext timeoutContext) {
125125
if (retry) {
126-
boolean retryUntilTimeoutThrowsException = timeoutContext.hasTimeoutMS();
127-
int retries = retryUntilTimeoutThrowsException ? INFINITE_ATTEMPTS : RetryState.RETRIES;
128-
return RetryState.withRetryableState(retries, retryUntilTimeoutThrowsException);
126+
return timeoutContext.hasTimeoutMS() ? new RetryState() : new RetryState(MAX_RETRIES);
129127
}
130-
return RetryState.withNonRetryableState();
128+
return new RetryState(0);
131129
}
132130

133131
private static final List<Integer> RETRYABLE_ERROR_CODES = asList(6, 7, 89, 91, 134, 189, 262, 9001, 13436, 13435, 11602, 11600, 10107);

driver-core/src/main/com/mongodb/internal/operation/MixedBulkWriteOperation.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -187,16 +187,15 @@ public String getCommandName() {
187187

188188
@Override
189189
public BulkWriteResult execute(final WriteBinding binding, final OperationContext operationContext) {
190-
TimeoutContext timeoutContext = operationContext.getTimeoutContext();
191190
/* We cannot use the tracking of attempts built in the `RetryState` class because conceptually we have to maintain multiple attempt
192191
* counters while executing a single bulk write operation:
193192
* - a counter that limits attempts to select server and checkout a connection before we created a batch;
194193
* - a counter per each batch that limits attempts to execute the specific batch.
195194
* Fortunately, these counters do not exist concurrently with each other. While maintaining the counters manually,
196195
* we must adhere to the contract of `RetryingSyncSupplier`. When the retry timeout is implemented, there will be no counters,
197196
* and the code related to the attempt tracking in `BulkWriteTracker` will be removed. */
198-
RetryState retryState = new RetryState(timeoutContext);
199-
BulkWriteTracker.attachNew(retryState, retryWrites, timeoutContext);
197+
RetryState retryState = new RetryState();
198+
BulkWriteTracker.attachNew(retryState, retryWrites, operationContext.getTimeoutContext());
200199
Supplier<BulkWriteResult> retryingBulkWrite = decorateWriteWithRetries(retryState, operationContext, () ->
201200
withSourceAndConnection(binding::getWriteConnectionSource, true, (source, connection, operationContextWithMinRTT) -> {
202201
TimeoutContext timeoutContextWithMinRtt = operationContextWithMinRTT.getTimeoutContext();
@@ -226,10 +225,9 @@ public BulkWriteResult execute(final WriteBinding binding, final OperationContex
226225
}
227226

228227
public void executeAsync(final AsyncWriteBinding binding, final OperationContext operationContext, final SingleResultCallback<BulkWriteResult> callback) {
229-
TimeoutContext timeoutContext = operationContext.getTimeoutContext();
230228
// see the comment in `execute(WriteBinding)` explaining the manual tracking of attempts
231-
RetryState retryState = new RetryState(timeoutContext);
232-
BulkWriteTracker.attachNew(retryState, retryWrites, timeoutContext);
229+
RetryState retryState = new RetryState();
230+
BulkWriteTracker.attachNew(retryState, retryWrites, operationContext.getTimeoutContext());
233231
binding.retain();
234232
AsyncCallbackSupplier<BulkWriteResult> retryingBulkWrite = this.<BulkWriteResult>decorateWriteWithRetries(retryState,
235233
operationContext,
@@ -493,7 +491,7 @@ private static void attach(final RetryState retryState, final BulkWriteTracker t
493491

494492
private BulkWriteTracker(final boolean retry, @Nullable final BulkWriteBatch batch, final TimeoutContext timeoutContext) {
495493
attempt = 0;
496-
attempts = retry ? RetryState.RETRIES + 1 : 1;
494+
attempts = retry ? RetryState.MAX_RETRIES + 1 : 1;
497495
this.batch = batch;
498496
this.retryUntilTimeoutThrowsException = timeoutContext.hasTimeoutMS();
499497
}

driver-core/src/test/unit/com/mongodb/internal/async/AsyncFunctionsAbstractTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
package com.mongodb.internal.async;
1717

1818
import com.mongodb.MongoException;
19-
import com.mongodb.internal.TimeoutContext;
20-
import com.mongodb.internal.TimeoutSettings;
2119
import org.junit.jupiter.api.Test;
2220

2321
import java.util.function.BiConsumer;
@@ -29,8 +27,6 @@
2927
import static org.junit.jupiter.api.Assertions.assertEquals;
3028

3129
abstract class AsyncFunctionsAbstractTest extends AsyncFunctionsTestBase {
32-
private static final TimeoutContext TIMEOUT_CONTEXT = new TimeoutContext(new TimeoutSettings(0, 0, 0, 0L, 0));
33-
3430
@Test
3531
void test1Method() {
3632
// the number of expected variations is often: 1 + N methods invoked
@@ -856,7 +852,6 @@ void testRetryLoop() {
856852
},
857853
(callback) -> {
858854
beginAsync().thenRunRetryingWhile(
859-
TIMEOUT_CONTEXT,
860855
c -> async(plainTest(0) ? 1 : 2, c),
861856
e -> e.getMessage().equals("exception-1")
862857
).finish(callback);

0 commit comments

Comments
 (0)