Skip to content

Commit 26c0f2f

Browse files
committed
Refactoring: naming, log messages, internal docs
JAVA-5956, JAVA-6117, JAVA-6113, JAVA-6119, JAVA-6141
1 parent 8890575 commit 26c0f2f

8 files changed

Lines changed: 80 additions & 66 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ public RetryState(final TimeoutContext timeoutContext) {
9696

9797
/**
9898
* @param retries A non-negative number of allowed retries. {@link Integer#MAX_VALUE} is a special value interpreted as being unlimited.
99-
* @param retryUntilTimeoutThrowsException
10099
* @see #attempts()
101100
*/
102101
private RetryState(final int retries, final boolean retryUntilTimeoutThrowsException) {
@@ -147,7 +146,7 @@ private RetryState(final int retries, final boolean retryUntilTimeoutThrowsExcep
147146
* <li>the {@code retryPredicate} is {@code false}.</li>
148147
* </ul>
149148
* The exception thrown represents the failed result of the associated retryable activity,
150-
* i.e., the caller must not do any more attempts.
149+
* i.e., the caller must not make any more attempts.
151150
* @see #advanceOrThrow(Throwable, BinaryOperator, BiPredicate)
152151
*/
153152
void advanceOrThrow(final RuntimeException attemptException, final BinaryOperator<Throwable> onAttemptFailureOperator,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@
5353
import static com.mongodb.internal.operation.CommandOperationHelper.CommandCreator;
5454
import static com.mongodb.internal.operation.CommandOperationHelper.addRetryableWriteErrorLabel;
5555
import static com.mongodb.internal.operation.CommandOperationHelper.initialRetryState;
56-
import static com.mongodb.internal.operation.CommandOperationHelper.isRetryWritesEnabled;
57-
import static com.mongodb.internal.operation.CommandOperationHelper.logRetryExecute;
56+
import static com.mongodb.internal.operation.CommandOperationHelper.isRetryableWriteCommand;
57+
import static com.mongodb.internal.operation.CommandOperationHelper.logRetryCommand;
5858
import static com.mongodb.internal.operation.CommandOperationHelper.onRetryableReadAttemptFailure;
5959
import static com.mongodb.internal.operation.CommandOperationHelper.onRetryableWriteAttemptFailure;
6060
import static com.mongodb.internal.operation.CommandOperationHelper.transformWriteException;
@@ -292,9 +292,9 @@ static <T, R> void executeRetryableWriteAsync(
292292
operationContextWithMinRtt,
293293
source.getServerDescription(),
294294
connection.getDescription()));
295-
// attach `maxWireVersion`, `retryableCommandFlag` ASAP because they are used to check whether we should retry
295+
// attach `maxWireVersion`, `retryableWriteCommandFlag` ASAP because they are used to check whether we should retry
296296
retryState.attach(AttachmentKeys.maxWireVersion(), maxWireVersion, true)
297-
.attach(AttachmentKeys.retryableCommandFlag(), isRetryWritesEnabled(command), true)
297+
.attach(AttachmentKeys.retryableWriteCommandFlag(), isRetryableWriteCommand(command), true)
298298
.attach(AttachmentKeys.commandDescriptionSupplier(), command::getFirstKey, false)
299299
.attach(AttachmentKeys.command(), command, false);
300300
} catch (Throwable t) {
@@ -336,7 +336,7 @@ static <R> AsyncCallbackSupplier<R> decorateReadWithRetriesAsync(final RetryStat
336336
final AsyncCallbackSupplier<R> asyncReadFunction) {
337337
return new RetryingAsyncCallbackSupplier<>(retryState, onRetryableReadAttemptFailure(operationContext),
338338
CommandOperationHelper::loggingShouldAttemptToRetryRead, callback -> {
339-
logRetryExecute(retryState, operationContext);
339+
logRetryCommand(retryState, operationContext);
340340
asyncReadFunction.get(callback);
341341
});
342342
}
@@ -345,7 +345,7 @@ static <R> AsyncCallbackSupplier<R> decorateWriteWithRetriesAsync(final RetrySta
345345
final AsyncCallbackSupplier<R> asyncWriteFunction) {
346346
return new RetryingAsyncCallbackSupplier<>(retryState, onRetryableWriteAttemptFailure(operationContext),
347347
CommandOperationHelper::loggingShouldAttemptToRetryWriteAndAddRetryableLabel, callback -> {
348-
logRetryExecute(retryState, operationContext);
348+
logRetryCommand(retryState, operationContext);
349349
asyncWriteFunction.get(callback);
350350
});
351351
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ private Integer executeBatch(
301301
.attach(AttachmentKeys.commandDescriptionSupplier(), () -> BULK_WRITE_COMMAND_NAME, false);
302302
ClientBulkWriteCommand bulkWriteCommand = createBulkWriteCommand(
303303
retryState, effectiveRetryWrites, effectiveWriteConcern, sessionContext, unexecutedModels, batchEncoder,
304-
() -> retryState.attach(AttachmentKeys.retryableCommandFlag(), true, true));
304+
() -> retryState.attach(AttachmentKeys.retryableWriteCommandFlag(), true, true));
305305
return executeBulkWriteCommandAndExhaustOkResponse(
306306
retryState, connectionSource, connection, bulkWriteCommand, effectiveWriteConcern, operationContextWithMinRtt);
307307
}, operationContext)
@@ -361,7 +361,7 @@ private void executeBatchAsync(
361361
.attach(AttachmentKeys.commandDescriptionSupplier(), () -> BULK_WRITE_COMMAND_NAME, false);
362362
ClientBulkWriteCommand bulkWriteCommand = createBulkWriteCommand(
363363
retryState, effectiveRetryWrites, effectiveWriteConcern, sessionContext, unexecutedModels, batchEncoder,
364-
() -> retryState.attach(AttachmentKeys.retryableCommandFlag(), true, true));
364+
() -> retryState.attach(AttachmentKeys.retryableWriteCommandFlag(), true, true));
365365
executeBulkWriteCommandAndExhaustOkResponseAsync(
366366
retryState, connectionSource, connection, bulkWriteCommand, effectiveWriteConcern, operationContextWithMinRtt, resultCallback);
367367
})
@@ -495,14 +495,14 @@ private static ExhaustiveClientBulkWriteCommandOkResponse createExhaustiveClient
495495
private <R> R doWithRetriesDisabled(
496496
final RetryState outerRetryState,
497497
final Supplier<R> action) {
498-
// TODO-JAVA-5956 The current implementation incorrectly uses `retryableCommandFlag` to achieve the behavior needed.
499-
Optional<Boolean> originalRetryableCommandFlag = outerRetryState.attachment(AttachmentKeys.retryableCommandFlag());
498+
// TODO-JAVA-5956 The current implementation incorrectly uses `retryableWriteCommandFlag` to achieve the behavior needed.
499+
Optional<Boolean> originalRetryableWriteCommandFlag = outerRetryState.attachment(AttachmentKeys.retryableWriteCommandFlag());
500500

501501
try {
502-
outerRetryState.attach(AttachmentKeys.retryableCommandFlag(), false, true);
502+
outerRetryState.attach(AttachmentKeys.retryableWriteCommandFlag(), false, true);
503503
return action.get();
504504
} finally {
505-
originalRetryableCommandFlag.ifPresent(value -> outerRetryState.attach(AttachmentKeys.retryableCommandFlag(), value, true));
505+
originalRetryableWriteCommandFlag.ifPresent(value -> outerRetryState.attach(AttachmentKeys.retryableWriteCommandFlag(), value, true));
506506
}
507507
}
508508

@@ -513,14 +513,14 @@ private <R> void doWithRetriesDisabledAsync(
513513
final RetryState retryState,
514514
final AsyncSupplier<R> action,
515515
final SingleResultCallback<R> callback) {
516-
// TODO-JAVA-5956 The current implementation incorrectly uses `retryableCommandFlag` to achieve the behavior needed.
517-
Optional<Boolean> originalRetryableCommandFlag = retryState.attachment(AttachmentKeys.retryableCommandFlag());
516+
// TODO-JAVA-5956 The current implementation incorrectly uses `retryableWriteCommandFlag` to achieve the behavior needed.
517+
Optional<Boolean> originalRetryableWriteCommandFlag = retryState.attachment(AttachmentKeys.retryableWriteCommandFlag());
518518

519519
beginAsync().<R>thenSupply(c -> {
520-
retryState.attach(AttachmentKeys.retryableCommandFlag(), false, true);
520+
retryState.attach(AttachmentKeys.retryableWriteCommandFlag(), false, true);
521521
action.finish(c);
522522
}).thenAlwaysRunAndFinish(() -> {
523-
originalRetryableCommandFlag.ifPresent(value -> retryState.attach(AttachmentKeys.retryableCommandFlag(), value, true));
523+
originalRetryableWriteCommandFlag.ifPresent(value -> retryState.attach(AttachmentKeys.retryableWriteCommandFlag(), value, true));
524524
}, callback);
525525
}
526526

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

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -172,18 +172,16 @@ static boolean loggingShouldAttemptToRetryRead(final RetryState retryState, fina
172172
|| (attemptFailure instanceof MongoSecurityException
173173
&& attemptFailure.getCause() != null && isRetryableException(attemptFailure.getCause()));
174174
if (!decision) {
175-
logUnableToRetry(retryState.attachment(AttachmentKeys.commandDescriptionSupplier()).orElse(null), attemptFailure);
175+
logUnableToRetryCommand(retryState, attemptFailure);
176176
}
177177
return decision;
178178
}
179179

180180
static boolean loggingShouldAttemptToRetryWriteAndAddRetryableLabel(final RetryState retryState, final Throwable attemptFailure) {
181181
Throwable attemptFailureNotToBeRetried = getWriteAttemptFailureNotToBeRetriedOrAddRetryableLabel(retryState, attemptFailure);
182182
boolean decision = attemptFailureNotToBeRetried == null;
183-
if (!decision && retryState.attachment(AttachmentKeys.retryableCommandFlag()).orElse(false)) {
184-
logUnableToRetry(
185-
retryState.attachment(AttachmentKeys.commandDescriptionSupplier()).orElse(null),
186-
assertNotNull(attemptFailureNotToBeRetried));
183+
if (!decision && retryState.attachment(AttachmentKeys.retryableWriteCommandFlag()).orElse(false)) {
184+
logUnableToRetryCommand(retryState, assertNotNull(attemptFailureNotToBeRetried));
187185
}
188186
return decision;
189187
}
@@ -205,10 +203,10 @@ private static Throwable getWriteAttemptFailureNotToBeRetriedOrAddRetryableLabel
205203
decision = true;
206204
exceptionRetryableRegardlessOfCommand = (MongoException) failure;
207205
}
208-
if (retryState.attachment(AttachmentKeys.retryableCommandFlag()).orElse(false)) {
206+
if (retryState.attachment(AttachmentKeys.retryableWriteCommandFlag()).orElse(false)) {
209207
if (exceptionRetryableRegardlessOfCommand != null) {
210-
/* We are going to retry even if `retryableCommand` is false,
211-
* but we add the retryable label only if `retryableCommand` is true. */
208+
/* We are going to retry even if `retryableWriteCommandFlag` is false,
209+
* but we add the retryable label only if `retryableWriteCommandFlag` is true. */
212210
exceptionRetryableRegardlessOfCommand.addLabel(RETRYABLE_WRITE_ERROR_LABEL);
213211
} else if (decideRetryableAndAddRetryableWriteErrorLabel(failure, retryState.attachment(AttachmentKeys.maxWireVersion())
214212
.orElse(null))) {
@@ -218,9 +216,20 @@ private static Throwable getWriteAttemptFailureNotToBeRetriedOrAddRetryableLabel
218216
return decision ? null : assertNotNull(failure);
219217
}
220218

221-
static boolean isRetryWritesEnabled(@Nullable final BsonDocument command) {
222-
return (command != null && (command.containsKey("txnNumber")
223-
|| command.getFirstKey().equals("commitTransaction") || command.getFirstKey().equals("abortTransaction")));
219+
/**
220+
* Returns {@code true} if the {@code command} is intended to be executed outside a transaction and supports being retried,
221+
* or if the {@code command} is {@code commitTransaction}/{@code abortTransaction}; {@code false} otherwise.
222+
*/
223+
static boolean isRetryableWriteCommand(final BsonDocument command) {
224+
// Given the requirement
225+
// https://github.com/mongodb/specifications/blame/7039e69945d463a14b1b727d16db063e21f48f53/source/transactions/transactions.md#L584-L586:
226+
// When executing the `commitTransaction` and `abortTransaction` commands within a transaction
227+
// drivers MUST use the same `txnNumber` used for all preceding commands in the transaction.
228+
// the additional checks if the `command` is either `commitTransaction`/`abortTransaction, may seem unnecessary.
229+
// However, since the `txnNumber` key is added to commands within transactions by `CommandMessage`,
230+
// the key is not present when the logic of automatic retries inspects a `commitTransaction`/`abortTransaction` command for it.
231+
return (command.containsKey("txnNumber")
232+
|| command.getFirstKey().equals("commitTransaction") || command.getFirstKey().equals("abortTransaction"));
224233
}
225234

226235
static final String RETRYABLE_WRITE_ERROR_LABEL = "RetryableWriteError";
@@ -245,26 +254,26 @@ static void addRetryableWriteErrorLabel(final MongoException exception, final in
245254
}
246255
}
247256

248-
static void logRetryExecute(final RetryState retryState, final OperationContext operationContext) {
257+
static void logRetryCommand(final RetryState retryState, final OperationContext operationContext) {
249258
if (LOGGER.isDebugEnabled() && !retryState.isFirstAttempt()) {
250259
String commandDescription = retryState.attachment(AttachmentKeys.commandDescriptionSupplier()).map(Supplier::get).orElse(null);
251260
Throwable exception = retryState.exception().orElseThrow(Assertions::fail);
252261
int oneBasedAttempt = retryState.attempt() + 1;
253262
long operationId = operationContext.getId();
254263
LOGGER.debug(commandDescription == null
255-
? format("Retrying the operation with operation ID %s due to the error \"%s\". Attempt number: #%d",
256-
operationId, exception, oneBasedAttempt)
257-
: format("Retrying the operation '%s' with operation ID %s due to the error \"%s\". Attempt number: #%d",
258-
commandDescription, operationId, exception, oneBasedAttempt));
264+
? format("Retrying a command within the operation with operation ID %s due to the error \"%s\". Attempt number: #%d",
265+
operationId, exception, oneBasedAttempt)
266+
: format("Retrying the command '%s' within the operation with operation ID %s due to the error \"%s\". Attempt number: #%d",
267+
commandDescription, operationId, exception, oneBasedAttempt));
259268
}
260269
}
261270

262-
private static void logUnableToRetry(@Nullable final Supplier<String> commandDescriptionSupplier, final Throwable originalError) {
271+
private static void logUnableToRetryCommand(final RetryState retryState, final Throwable originalError) {
263272
if (LOGGER.isDebugEnabled()) {
264-
String commandDescription = commandDescriptionSupplier == null ? null : commandDescriptionSupplier.get();
273+
String commandDescription = retryState.attachment(AttachmentKeys.commandDescriptionSupplier()).map(Supplier::get).orElse(null);
265274
LOGGER.debug(commandDescription == null
266-
? format("Unable to retry an operation due to the error \"%s\"", originalError)
267-
: format("Unable to retry the operation %s due to the error \"%s\"", commandDescription, originalError));
275+
? format("Unable to retry a command due to the error \"%s\"", originalError)
276+
: format("Unable to retry the command '%s' due to the error \"%s\"", commandDescription, originalError));
268277
}
269278
}
270279

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
import static com.mongodb.internal.operation.AsyncOperationHelper.exceptionTransformingCallback;
6363
import static com.mongodb.internal.operation.AsyncOperationHelper.withAsyncSourceAndConnection;
6464
import static com.mongodb.internal.operation.CommandOperationHelper.addRetryableWriteErrorLabel;
65-
import static com.mongodb.internal.operation.CommandOperationHelper.logRetryExecute;
65+
import static com.mongodb.internal.operation.CommandOperationHelper.logRetryCommand;
6666
import static com.mongodb.internal.operation.CommandOperationHelper.loggingShouldAttemptToRetryWriteAndAddRetryableLabel;
6767
import static com.mongodb.internal.operation.CommandOperationHelper.onRetryableWriteAttemptFailure;
6868
import static com.mongodb.internal.operation.CommandOperationHelper.transformWriteException;
@@ -149,7 +149,7 @@ private <R> Supplier<R> decorateWriteWithRetries(final RetryState retryState, fi
149149
final Supplier<R> writeFunction) {
150150
return new RetryingSyncSupplier<>(retryState, onRetryableWriteAttemptFailure(operationContext),
151151
this::shouldAttemptToRetryWrite, () -> {
152-
logRetryExecute(retryState, operationContext);
152+
logRetryCommand(retryState, operationContext);
153153
return writeFunction.get();
154154
});
155155
}
@@ -158,7 +158,7 @@ private <R> AsyncCallbackSupplier<R> decorateWriteWithRetries(final RetryState r
158158
final AsyncCallbackSupplier<R> writeFunction) {
159159
return new RetryingAsyncCallbackSupplier<>(retryState, onRetryableWriteAttemptFailure(operationContext),
160160
this::shouldAttemptToRetryWrite, callback -> {
161-
logRetryExecute(retryState, operationContext);
161+
logRetryCommand(retryState, operationContext);
162162
writeFunction.get(callback);
163163
});
164164
}
@@ -486,7 +486,7 @@ private static void attach(final RetryState retryState, final BulkWriteTracker t
486486
retryState.attach(AttachmentKeys.bulkWriteTracker(), tracker, false);
487487
BulkWriteBatch batch = tracker.batch;
488488
if (batch != null) {
489-
retryState.attach(AttachmentKeys.retryableCommandFlag(), batch.getRetryWrites(), false)
489+
retryState.attach(AttachmentKeys.retryableWriteCommandFlag(), batch.getRetryWrites(), false)
490490
.attach(AttachmentKeys.commandDescriptionSupplier(), () -> batch.getPayload().getPayloadType().toString(), false);
491491
}
492492
}

0 commit comments

Comments
 (0)