Skip to content

Commit

Permalink
Do not retry a read operation when in a transaction (#982)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jyemin committed Jul 25, 2022
1 parent 06c0820 commit f42ef7d
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
}
]
}
]
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}


Expand Down

0 comments on commit f42ef7d

Please sign in to comment.