Skip to content

Commit 4a15e31

Browse files
authored
Ignore wTimeoutMS in WriteConcern when timeoutMS is set (#1368)
JAVA-4062
1 parent 235adf4 commit 4a15e31

File tree

21 files changed

+234
-45
lines changed

21 files changed

+234
-45
lines changed

driver-core/src/main/com/mongodb/assertions/Assertions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.util.function.Supplier;
2626

2727
/**
28-
* <p>Design by contract assertions.</p> <p>This class is not part of the public API and may be removed or changed at any time.</p>
28+
* <p>Design by contract assertions.</p>
2929
* All {@code assert...} methods throw {@link AssertionError} and should be used to check conditions which may be violated if and only if
3030
* the driver code is incorrect. The intended usage of this methods is the same as of the
3131
* <a href="https://docs.oracle.com/javase/8/docs/technotes/guides/language/assert.html">Java {@code assert} statement</a>. The reason

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ private void doAdvanceOrThrow(final Throwable attemptException,
203203
*/
204204
if (hasTimeoutMs() && !loopState.isLastIteration()) {
205205
previouslyChosenException = createMongoTimeoutException(
206-
"MongoDB operation timed out during a retry attempt",
206+
"Retry attempt timed out.",
207207
previouslyChosenException);
208208
}
209209
throw previouslyChosenException;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.mongodb.Function;
2020
import com.mongodb.WriteConcern;
21+
import com.mongodb.internal.TimeoutContext;
2122
import com.mongodb.lang.Nullable;
2223
import org.bson.BsonDocument;
2324

@@ -58,7 +59,7 @@ CommandCreator getCommandCreator() {
5859
}
5960

6061
@Override
61-
protected Function<BsonDocument, BsonDocument> getRetryCommandModifier() {
62+
protected Function<BsonDocument, BsonDocument> getRetryCommandModifier(final TimeoutContext timeoutContext) {
6263
return cmd -> cmd;
6364
}
6465
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ public final class AsyncOperations<TDocument> {
7474
public AsyncOperations(final MongoNamespace namespace, final Class<TDocument> documentClass, final ReadPreference readPreference,
7575
final CodecRegistry codecRegistry, final ReadConcern readConcern, final WriteConcern writeConcern,
7676
final boolean retryWrites, final boolean retryReads, final TimeoutSettings timeoutSettings) {
77-
this.operations = new Operations<>(namespace, documentClass, readPreference, codecRegistry, readConcern, writeConcern,
77+
WriteConcern writeConcernToUse = writeConcern;
78+
if (timeoutSettings.getTimeoutMS() != null) {
79+
writeConcernToUse = assertNotNull(WriteConcernHelper.cloneWithoutTimeout(writeConcern));
80+
}
81+
this.operations = new Operations<>(namespace, documentClass, readPreference, codecRegistry, readConcern, writeConcernToUse,
7882
retryWrites, retryReads);
7983
this.timeoutSettings = timeoutSettings;
8084
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.mongodb.MongoTimeoutException;
2626
import com.mongodb.MongoWriteConcernException;
2727
import com.mongodb.WriteConcern;
28+
import com.mongodb.internal.TimeoutContext;
2829
import com.mongodb.internal.async.SingleResultCallback;
2930
import com.mongodb.internal.binding.AsyncWriteBinding;
3031
import com.mongodb.internal.binding.WriteBinding;
@@ -125,7 +126,8 @@ CommandCreator getCommandCreator() {
125126
};
126127
if (alreadyCommitted) {
127128
return (operationContext, serverDescription, connectionDescription) ->
128-
getRetryCommandModifier().apply(creator.create(operationContext, serverDescription, connectionDescription));
129+
getRetryCommandModifier(operationContext.getTimeoutContext())
130+
.apply(creator.create(operationContext, serverDescription, connectionDescription));
129131
} else if (recoveryToken != null) {
130132
return (operationContext, serverDescription, connectionDescription) ->
131133
creator.create(operationContext, serverDescription, connectionDescription)
@@ -136,10 +138,10 @@ CommandCreator getCommandCreator() {
136138

137139
@Override
138140
@SuppressWarnings("deprecation") //wTimeout
139-
protected Function<BsonDocument, BsonDocument> getRetryCommandModifier() {
141+
protected Function<BsonDocument, BsonDocument> getRetryCommandModifier(final TimeoutContext timeoutContext) {
140142
return command -> {
141143
WriteConcern retryWriteConcern = getWriteConcern().withW("majority");
142-
if (retryWriteConcern.getWTimeout(MILLISECONDS) == null) {
144+
if (retryWriteConcern.getWTimeout(MILLISECONDS) == null && !timeoutContext.hasTimeoutMS()) {
143145
retryWriteConcern = retryWriteConcern.withWTimeout(10000, MILLISECONDS);
144146
}
145147
command.put("writeConcern", retryWriteConcern.asDocument());

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060

6161
import java.util.List;
6262

63+
import static com.mongodb.assertions.Assertions.assertNotNull;
6364
import static java.util.concurrent.TimeUnit.MILLISECONDS;
6465

6566
/**
@@ -82,7 +83,11 @@ public SyncOperations(final MongoNamespace namespace, final Class<TDocument> doc
8283
public SyncOperations(@Nullable final MongoNamespace namespace, final Class<TDocument> documentClass, final ReadPreference readPreference,
8384
final CodecRegistry codecRegistry, final ReadConcern readConcern, final WriteConcern writeConcern,
8485
final boolean retryWrites, final boolean retryReads, final TimeoutSettings timeoutSettings) {
85-
this.operations = new Operations<>(namespace, documentClass, readPreference, codecRegistry, readConcern, writeConcern,
86+
WriteConcern writeConcernToUse = writeConcern;
87+
if (timeoutSettings.getTimeoutMS() != null) {
88+
writeConcernToUse = assertNotNull(WriteConcernHelper.cloneWithoutTimeout(writeConcern));
89+
}
90+
this.operations = new Operations<>(namespace, documentClass, readPreference, codecRegistry, readConcern, writeConcernToUse,
8691
retryWrites, retryReads);
8792
this.timeoutSettings = timeoutSettings;
8893
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.mongodb.Function;
2020
import com.mongodb.WriteConcern;
21+
import com.mongodb.internal.TimeoutContext;
2122
import com.mongodb.internal.async.SingleResultCallback;
2223
import com.mongodb.internal.binding.AsyncWriteBinding;
2324
import com.mongodb.internal.binding.WriteBinding;
@@ -55,17 +56,19 @@ public WriteConcern getWriteConcern() {
5556
@Override
5657
public Void execute(final WriteBinding binding) {
5758
isTrue("in transaction", binding.getOperationContext().getSessionContext().hasActiveTransaction());
59+
TimeoutContext timeoutContext = binding.getOperationContext().getTimeoutContext();
5860
return executeRetryableWrite(binding, "admin", null, new NoOpFieldNameValidator(),
5961
new BsonDocumentCodec(), getCommandCreator(),
60-
writeConcernErrorTransformer(binding.getOperationContext().getTimeoutContext()), getRetryCommandModifier());
62+
writeConcernErrorTransformer(timeoutContext), getRetryCommandModifier(timeoutContext));
6163
}
6264

6365
@Override
6466
public void executeAsync(final AsyncWriteBinding binding, final SingleResultCallback<Void> callback) {
6567
isTrue("in transaction", binding.getOperationContext().getSessionContext().hasActiveTransaction());
68+
TimeoutContext timeoutContext = binding.getOperationContext().getTimeoutContext();
6669
executeRetryableWriteAsync(binding, "admin", null, new NoOpFieldNameValidator(),
6770
new BsonDocumentCodec(), getCommandCreator(),
68-
writeConcernErrorTransformerAsync(binding.getOperationContext().getTimeoutContext()), getRetryCommandModifier(),
71+
writeConcernErrorTransformerAsync(timeoutContext), getRetryCommandModifier(timeoutContext),
6972
errorHandlingCallback(callback, LOGGER));
7073
}
7174

@@ -86,5 +89,5 @@ CommandCreator getCommandCreator() {
8689
*/
8790
protected abstract String getCommandName();
8891

89-
protected abstract Function<BsonDocument, BsonDocument> getRetryCommandModifier();
92+
protected abstract Function<BsonDocument, BsonDocument> getRetryCommandModifier(TimeoutContext timeoutContext);
9093
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424
import com.mongodb.bulk.WriteConcernError;
2525
import com.mongodb.internal.TimeoutContext;
2626
import com.mongodb.internal.connection.ProtocolHelper;
27+
import com.mongodb.lang.Nullable;
2728
import org.bson.BsonArray;
2829
import org.bson.BsonDocument;
2930
import org.bson.BsonString;
3031

32+
import java.util.concurrent.TimeUnit;
3133
import java.util.stream.Collectors;
3234

3335
import static com.mongodb.internal.operation.CommandOperationHelper.addRetryableWriteErrorLabel;
@@ -42,6 +44,21 @@ public static void appendWriteConcernToCommand(final WriteConcern writeConcern,
4244
commandDocument.put("writeConcern", writeConcern.asDocument());
4345
}
4446
}
47+
@Nullable
48+
public static WriteConcern cloneWithoutTimeout(@Nullable final WriteConcern writeConcern) {
49+
if (writeConcern == null || writeConcern.getWTimeout(TimeUnit.MILLISECONDS) == null) {
50+
return writeConcern;
51+
}
52+
53+
WriteConcern mapped;
54+
Object w = writeConcern.getWObject();
55+
if (w == null) {
56+
mapped = WriteConcern.ACKNOWLEDGED;
57+
} else {
58+
mapped = w instanceof Integer ? new WriteConcern((Integer) w) : new WriteConcern((String) w);
59+
}
60+
return mapped.withJournal(writeConcern.getJournal());
61+
}
4562

4663
public static void throwOnWriteConcernError(final BsonDocument result, final ServerAddress serverAddress,
4764
final int maxWireVersion, final TimeoutContext timeoutContext) {

driver-core/src/main/com/mongodb/internal/session/BaseClientSessionImpl.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.mongodb.MongoClientException;
2121
import com.mongodb.ServerAddress;
2222
import com.mongodb.TransactionOptions;
23+
import com.mongodb.WriteConcern;
2324
import com.mongodb.internal.TimeoutContext;
2425
import com.mongodb.internal.TimeoutSettings;
2526
import com.mongodb.internal.binding.ReferenceCounted;
@@ -29,6 +30,7 @@
2930
import org.bson.BsonDocument;
3031
import org.bson.BsonTimestamp;
3132

33+
import java.util.concurrent.TimeUnit;
3234
import java.util.concurrent.atomic.AtomicBoolean;
3335

3436
import static com.mongodb.assertions.Assertions.assertTrue;
@@ -55,6 +57,14 @@ public class BaseClientSessionImpl implements ClientSession {
5557
@Nullable
5658
private TimeoutContext timeoutContext;
5759

60+
protected static boolean hasTimeoutMS(@Nullable final TimeoutContext timeoutContext) {
61+
return timeoutContext != null && timeoutContext.hasTimeoutMS();
62+
}
63+
64+
protected static boolean hasWTimeoutMS(@Nullable final WriteConcern writeConcern) {
65+
return writeConcern != null && writeConcern.getWTimeout(TimeUnit.MILLISECONDS) != null;
66+
}
67+
5868
public BaseClientSessionImpl(final ServerSessionPool serverSessionPool, final Object originator, final ClientSessionOptions options) {
5969
this.serverSessionPool = serverSessionPool;
6070
this.originator = originator;
@@ -228,4 +238,8 @@ protected TimeoutSettings getTimeoutSettings(final TransactionOptions transactio
228238
.withMaxCommitMS(transactionOptions.getMaxCommitTime(MILLISECONDS))
229239
.withTimeout(timeoutMS, MILLISECONDS);
230240
}
241+
242+
protected enum TransactionState {
243+
NONE, IN, COMMITTED, ABORTED
244+
}
231245
}

driver-core/src/test/functional/com/mongodb/internal/connection/TestCommandListener.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.concurrent.locks.Condition;
4242
import java.util.concurrent.locks.Lock;
4343
import java.util.concurrent.locks.ReentrantLock;
44+
import java.util.function.Predicate;
4445
import java.util.stream.Collectors;
4546

4647
import static com.mongodb.ClusterFixture.TIMEOUT;
@@ -152,15 +153,28 @@ public List<CommandStartedEvent> getCommandStartedEvents() {
152153
return getEvents(CommandStartedEvent.class, Integer.MAX_VALUE);
153154
}
154155

156+
public List<CommandStartedEvent> getCommandStartedEvents(final String commandName) {
157+
return getEvents(CommandStartedEvent.class,
158+
commandEvent -> commandEvent.getCommandName().equals(commandName),
159+
Integer.MAX_VALUE);
160+
}
161+
155162
public List<CommandSucceededEvent> getCommandSucceededEvents() {
156163
return getEvents(CommandSucceededEvent.class, Integer.MAX_VALUE);
157164
}
158165

159166
private <T extends CommandEvent> List<T> getEvents(final Class<T> type, final int maxEvents) {
167+
return getEvents(type, e -> true, maxEvents);
168+
}
169+
170+
private <T extends CommandEvent> List<T> getEvents(final Class<T> type,
171+
final Predicate<? super CommandEvent> filter,
172+
final int maxEvents) {
160173
lock.lock();
161174
try {
162175
return getEvents().stream()
163176
.filter(e -> e.getClass() == type)
177+
.filter(filter)
164178
.map(type::cast)
165179
.limit(maxEvents).collect(Collectors.toList());
166180
} finally {

driver-core/src/test/resources/unified-test-format/client-side-operation-timeout/deprecated-options.json

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@
141141
},
142142
{
143143
"description": "commitTransaction ignores wTimeoutMS if timeoutMS is set",
144+
"comment": "Moved timeoutMS from commitTransaction to startTransaction manually, as commitTransaction does not support a timeoutMS option.",
144145
"operations": [
145146
{
146147
"name": "createEntities",
@@ -186,7 +187,10 @@
186187
},
187188
{
188189
"name": "startTransaction",
189-
"object": "session"
190+
"object": "session",
191+
"arguments": {
192+
"timeoutMS": 10000
193+
}
190194
},
191195
{
192196
"name": "countDocuments",
@@ -198,10 +202,7 @@
198202
},
199203
{
200204
"name": "commitTransaction",
201-
"object": "session",
202-
"arguments": {
203-
"timeoutMS": 10000
204-
}
205+
"object": "session"
205206
}
206207
],
207208
"expectEvents": [
@@ -430,6 +431,7 @@
430431
},
431432
{
432433
"description": "abortTransaction ignores wTimeoutMS if timeoutMS is set",
434+
"comment": "Moved timeoutMS from abortTransaction to startTransaction manually, as abortTransaction does not support a timeoutMS option.",
433435
"operations": [
434436
{
435437
"name": "createEntities",
@@ -475,7 +477,10 @@
475477
},
476478
{
477479
"name": "startTransaction",
478-
"object": "session"
480+
"object": "session",
481+
"arguments": {
482+
"timeoutMS": 10000
483+
}
479484
},
480485
{
481486
"name": "countDocuments",
@@ -487,10 +492,7 @@
487492
},
488493
{
489494
"name": "abortTransaction",
490-
"object": "session",
491-
"arguments": {
492-
"timeoutMS": 10000
493-
}
495+
"object": "session"
494496
}
495497
],
496498
"expectEvents": [

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ final class RetryStateTest {
5050

5151
private static final TimeoutContext TIMEOUT_CONTEXT_INFINITE_GLOBAL_TIMEOUT = new TimeoutContext(new TimeoutSettings(0L, 0L,
5252
0L, 0L, 0L));
53-
private static final String EXPECTED_TIMEOUT_MESSAGE = "MongoDB operation timed out during a retry attempt";
53+
private static final String EXPECTED_TIMEOUT_MESSAGE = "Retry attempt timed out.";
5454

5555
static Stream<Arguments> infiniteTimeout() {
5656
return Stream.of(
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright 2008-present MongoDB, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.mongodb.internal.operation;
18+
19+
import com.mongodb.WriteConcern;
20+
import org.junit.jupiter.params.ParameterizedTest;
21+
import org.junit.jupiter.params.provider.MethodSource;
22+
23+
import java.util.concurrent.TimeUnit;
24+
25+
import static com.mongodb.assertions.Assertions.assertNull;
26+
import static org.junit.jupiter.api.Assertions.assertEquals;
27+
28+
class WriteConcernHelperTest {
29+
30+
static WriteConcern[] shouldRemoveWtimeout(){
31+
return new WriteConcern[]{
32+
WriteConcern.ACKNOWLEDGED,
33+
WriteConcern.MAJORITY,
34+
WriteConcern.W1,
35+
WriteConcern.W2,
36+
WriteConcern.W3,
37+
WriteConcern.UNACKNOWLEDGED,
38+
WriteConcern.JOURNALED,
39+
40+
WriteConcern.ACKNOWLEDGED.withWTimeout(100, TimeUnit.MILLISECONDS),
41+
WriteConcern.MAJORITY.withWTimeout(100, TimeUnit.MILLISECONDS),
42+
WriteConcern.W1.withWTimeout(100, TimeUnit.MILLISECONDS),
43+
WriteConcern.W2.withWTimeout(100, TimeUnit.MILLISECONDS),
44+
WriteConcern.W3.withWTimeout(100, TimeUnit.MILLISECONDS),
45+
WriteConcern.UNACKNOWLEDGED.withWTimeout(100, TimeUnit.MILLISECONDS),
46+
WriteConcern.JOURNALED.withWTimeout(100, TimeUnit.MILLISECONDS),
47+
};
48+
}
49+
50+
@MethodSource
51+
@ParameterizedTest
52+
void shouldRemoveWtimeout(final WriteConcern writeConcern){
53+
//when
54+
WriteConcern clonedWithoutTimeout = WriteConcernHelper.cloneWithoutTimeout(writeConcern);
55+
56+
//then
57+
assertEquals(writeConcern.getWObject(), clonedWithoutTimeout.getWObject());
58+
assertEquals(writeConcern.getJournal(), clonedWithoutTimeout.getJournal());
59+
assertNull(clonedWithoutTimeout.getWTimeout(TimeUnit.MILLISECONDS));
60+
}
61+
}

0 commit comments

Comments
 (0)