From f42ef7d028dc4b5b2bdda3e32bfe1f8991c2c21b Mon Sep 17 00:00:00 2001 From: Jeff Yemin Date: Mon, 25 Jul 2022 08:33:07 -0400 Subject: [PATCH] Do not retry a read operation when in a transaction (#982) Fixes a regression introduced in 4.4.0 in scope of JAVA-4034, and was undetected due to the lack of a specification test for the required behavior. JAVA-4684 --- .../internal/operation/OperationHelper.java | 21 +--- .../do-not-retry-read-in-transaction.json | 115 ++++++++++++++++++ .../OperationHelperSpecification.groovy | 19 ++- 3 files changed, 130 insertions(+), 25 deletions(-) create mode 100644 driver-core/src/test/resources/unified-test-format/transactions/do-not-retry-read-in-transaction.json diff --git a/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java b/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java index bb9df2ed9f3..38acf0a484a 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java +++ b/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java @@ -29,7 +29,9 @@ import com.mongodb.diagnostics.logging.Loggers; import com.mongodb.internal.async.AsyncBatchCursor; import com.mongodb.internal.async.SingleResultCallback; +import com.mongodb.internal.async.function.AsyncCallbackBiFunction; import com.mongodb.internal.async.function.AsyncCallbackFunction; +import com.mongodb.internal.async.function.AsyncCallbackSupplier; import com.mongodb.internal.binding.AsyncConnectionSource; import com.mongodb.internal.binding.AsyncReadBinding; import com.mongodb.internal.binding.AsyncWriteBinding; @@ -44,8 +46,6 @@ import com.mongodb.internal.connection.AsyncConnection; import com.mongodb.internal.connection.Connection; import com.mongodb.internal.connection.QueryResult; -import com.mongodb.internal.async.function.AsyncCallbackBiFunction; -import com.mongodb.internal.async.function.AsyncCallbackSupplier; import com.mongodb.internal.session.SessionContext; import com.mongodb.lang.NonNull; import org.bson.BsonDocument; @@ -437,21 +437,12 @@ static boolean canRetryWrite(final ServerDescription serverDescription, final Co return true; } - static boolean isRetryableRead(final boolean retryReads, final ServerDescription serverDescription, - final ConnectionDescription connectionDescription, final SessionContext sessionContext) { - if (!retryReads) { - return false; - } else if (sessionContext.hasActiveTransaction()) { - LOGGER.debug("retryReads set to true but in an active transaction."); - return false; - } else { - return canRetryRead(serverDescription, connectionDescription, sessionContext); - } - } - static boolean canRetryRead(final ServerDescription serverDescription, final ConnectionDescription connectionDescription, final SessionContext sessionContext) { - if (serverIsLessThanVersionThreeDotSix(connectionDescription)) { + if (sessionContext.hasActiveTransaction()) { + LOGGER.debug("retryReads set to true but in an active transaction."); + return false; + } else if (serverIsLessThanVersionThreeDotSix(connectionDescription)) { LOGGER.debug("retryReads set to true but the server does not support retryable reads."); return false; } else if (serverDescription.getLogicalSessionTimeoutMinutes() == null && serverDescription.getType() != ServerType.LOAD_BALANCER) { diff --git a/driver-core/src/test/resources/unified-test-format/transactions/do-not-retry-read-in-transaction.json b/driver-core/src/test/resources/unified-test-format/transactions/do-not-retry-read-in-transaction.json new file mode 100644 index 00000000000..6d9dc704b81 --- /dev/null +++ b/driver-core/src/test/resources/unified-test-format/transactions/do-not-retry-read-in-transaction.json @@ -0,0 +1,115 @@ +{ + "description": "do not retry read in a transaction", + "schemaVersion": "1.4", + "runOnRequirements": [ + { + "minServerVersion": "4.0.0", + "topologies": [ + "replicaset" + ] + }, + { + "minServerVersion": "4.2.0", + "topologies": [ + "sharded", + "load-balanced" + ] + } + ], + "createEntities": [ + { + "client": { + "id": "client0", + "useMultipleMongoses": false, + "observeEvents": [ + "commandStartedEvent" + ], + "uriOptions": { + "retryReads": true + } + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "retryable-read-in-transaction-test" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll" + } + }, + { + "session": { + "id": "session0", + "client": "client0" + } + } + ], + "tests": [ + { + "description": "find does not retry in a transaction", + "operations": [ + { + "name": "startTransaction", + "object": "session0" + }, + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client0", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "find" + ], + "closeConnection": true + } + } + } + }, + { + "name": "find", + "object": "collection0", + "arguments": { + "filter": {}, + "session": "session0" + }, + "expectError": { + "isError": true, + "errorLabelsContain": [ + "TransientTransactionError" + ] + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "coll", + "filter": {}, + "startTransaction": true + }, + "commandName": "find", + "databaseName": "retryable-read-in-transaction-test" + } + } + ] + } + ] + } + ] +} diff --git a/driver-core/src/test/unit/com/mongodb/internal/operation/OperationHelperSpecification.groovy b/driver-core/src/test/unit/com/mongodb/internal/operation/OperationHelperSpecification.groovy index a0e33a4eb8c..9d5fb9ae1c7 100644 --- a/driver-core/src/test/unit/com/mongodb/internal/operation/OperationHelperSpecification.groovy +++ b/driver-core/src/test/unit/com/mongodb/internal/operation/OperationHelperSpecification.groovy @@ -43,7 +43,7 @@ import static com.mongodb.connection.ServerType.REPLICA_SET_PRIMARY import static com.mongodb.connection.ServerType.STANDALONE import static com.mongodb.internal.operation.OperationHelper.AsyncCallableWithConnection import static com.mongodb.internal.operation.OperationHelper.AsyncCallableWithConnectionAndSource -import static com.mongodb.internal.operation.OperationHelper.isRetryableRead +import static com.mongodb.internal.operation.OperationHelper.canRetryRead import static com.mongodb.internal.operation.OperationHelper.isRetryableWrite import static com.mongodb.internal.operation.OperationHelper.validateCollation import static com.mongodb.internal.operation.OperationHelper.validateCollationAndWriteConcern @@ -434,17 +434,16 @@ class OperationHelperSpecification extends Specification { } expect: - isRetryableRead(retryReads, serverDescription, connectionDescription, noTransactionSessionContext) == expected - !isRetryableRead(retryReads, serverDescription, connectionDescription, activeTransactionSessionContext) - !isRetryableRead(retryReads, serverDescription, connectionDescription, noOpSessionContext) + canRetryRead(serverDescription, connectionDescription, noTransactionSessionContext) == expected + !canRetryRead(serverDescription, connectionDescription, activeTransactionSessionContext) + !canRetryRead(serverDescription, connectionDescription, noOpSessionContext) where: - retryReads | serverDescription | connectionDescription | expected - false | retryableServerDescription | threeSixConnectionDescription | false - true | retryableServerDescription | threeSixConnectionDescription | true - true | nonRetryableServerDescription | threeSixConnectionDescription | false - true | retryableServerDescription | threeFourConnectionDescription | false - true | retryableServerDescription | threeSixPrimaryConnectionDescription | true + serverDescription | connectionDescription | expected + retryableServerDescription | threeSixConnectionDescription | true + nonRetryableServerDescription | threeSixConnectionDescription | false + retryableServerDescription | threeFourConnectionDescription | false + retryableServerDescription | threeSixPrimaryConnectionDescription | true }