From cda416a30662ca84745cf1dd139f56d1cf51421b Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 18 Sep 2024 14:47:21 -0700 Subject: [PATCH 1/3] Set maxTimeMS explicitly for commands being explained when CSOT is not enabled. JAVA-5580 --- .../com/mongodb/internal/TimeoutContext.java | 14 +++- .../internal/connection/CommandHelper.java | 16 +++++ .../operation/AggregateOperation.java | 8 ++- .../internal/operation/FindOperation.java | 10 ++- .../MapReduceToCollectionOperation.java | 9 ++- .../MapReduceWithInlineResultsOperation.java | 10 ++- .../internal/operation/OperationHelper.java | 2 +- .../mongodb/client/AbstractExplainTest.java | 72 ++++++++++++++++++- 8 files changed, 126 insertions(+), 15 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/TimeoutContext.java b/driver-core/src/main/com/mongodb/internal/TimeoutContext.java index 0b4907c2ff1..93df2a09922 100644 --- a/driver-core/src/main/com/mongodb/internal/TimeoutContext.java +++ b/driver-core/src/main/com/mongodb/internal/TimeoutContext.java @@ -17,6 +17,7 @@ import com.mongodb.MongoClientException; import com.mongodb.MongoOperationTimeoutException; +import com.mongodb.internal.connection.CommandMessage; import com.mongodb.internal.time.StartTime; import com.mongodb.internal.time.Timeout; import com.mongodb.lang.Nullable; @@ -213,14 +214,23 @@ public void resetToDefaultMaxTime() { /** * The override will be provided as the remaining value in - * {@link #runMaxTimeMS}, where 0 is ignored. + * {@link #runMaxTimeMS}, where 0 is ignored. This is useful for setting timeout + * in {@link CommandMessage} as an extra element before we send it to the server. + * *

* NOTE: Suitable for static user-defined values only (i.e MaxAwaitTimeMS), - * not for running timeouts that adjust dynamically. + * not for running timeouts that adjust dynamically (CSOT). */ public void setMaxTimeOverride(final long maxTimeMS) { this.maxTimeSupplier = () -> maxTimeMS; } + /** + * Disable the maxTimeMS override. This way the maxTimeMS will not + * be appended to the command in the {@link CommandMessage}. + */ + public void disableMaxTimeOverride() { + this.maxTimeSupplier = () -> 0; + } /** * The override will be provided as the remaining value in diff --git a/driver-core/src/main/com/mongodb/internal/connection/CommandHelper.java b/driver-core/src/main/com/mongodb/internal/connection/CommandHelper.java index 11dfd94e935..fa7c1f0739d 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/CommandHelper.java +++ b/driver-core/src/main/com/mongodb/internal/connection/CommandHelper.java @@ -20,10 +20,12 @@ import com.mongodb.MongoServerException; import com.mongodb.ServerApi; import com.mongodb.connection.ClusterConnectionMode; +import com.mongodb.internal.TimeoutContext; import com.mongodb.internal.async.SingleResultCallback; import com.mongodb.internal.validator.NoOpFieldNameValidator; import com.mongodb.lang.Nullable; import org.bson.BsonDocument; +import org.bson.BsonInt64; import org.bson.BsonValue; import org.bson.codecs.BsonDocumentCodec; @@ -117,6 +119,20 @@ private static CommandMessage getCommandMessage(final String database, final Bso clusterConnectionMode, serverApi); } + + /** + * Appends a user-defined maxTimeMS to the command if CSOT is not enabled. + * This is necessary when maxTimeMS must be explicitly set on the command being explained, + * rather than appending it lazily to the explain command in the {@link CommandMessage} via {@link TimeoutContext#setMaxTimeOverride(long)}. + * This ensures backwards compatibility with pre-CSOT behavior. + */ + public static void applyMaxTimeMS(final TimeoutContext timeoutContext, final BsonDocument command) { + if (!timeoutContext.hasTimeoutMS()) { + command.append("maxTimeMS", new BsonInt64(timeoutContext.getTimeoutSettings().getMaxTimeMS())); + timeoutContext.disableMaxTimeOverride(); + } + } + private CommandHelper() { } } diff --git a/driver-core/src/main/com/mongodb/internal/operation/AggregateOperation.java b/driver-core/src/main/com/mongodb/internal/operation/AggregateOperation.java index 07943560b40..f50304480b5 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/AggregateOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/AggregateOperation.java @@ -32,6 +32,7 @@ import java.util.List; +import static com.mongodb.internal.connection.CommandHelper.applyMaxTimeMS; import static com.mongodb.internal.operation.ExplainHelper.asExplainCommand; import static com.mongodb.internal.operation.ServerVersionHelper.MIN_WIRE_VERSION; @@ -155,8 +156,11 @@ public AsyncReadOperation asAsyncExplainableOperation(@Nullable final Exp CommandReadOperation createExplainableOperation(@Nullable final ExplainVerbosity verbosity, final Decoder resultDecoder) { return new CommandReadOperation<>(getNamespace().getDatabaseName(), - (operationContext, serverDescription, connectionDescription) -> - asExplainCommand(wrapped.getCommand(operationContext, MIN_WIRE_VERSION), verbosity), resultDecoder); + (operationContext, serverDescription, connectionDescription) -> { + BsonDocument command = wrapped.getCommand(operationContext, MIN_WIRE_VERSION); + applyMaxTimeMS(operationContext.getTimeoutContext(), command); + return asExplainCommand(command, verbosity); + }, resultDecoder); } MongoNamespace getNamespace() { diff --git a/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java b/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java index 514e48b4db8..abdbc328a14 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java @@ -42,6 +42,7 @@ import static com.mongodb.assertions.Assertions.notNull; import static com.mongodb.internal.async.ErrorHandlingResultCallback.errorHandlingCallback; +import static com.mongodb.internal.connection.CommandHelper.applyMaxTimeMS; import static com.mongodb.internal.operation.AsyncOperationHelper.CommandReadTransformerAsync; import static com.mongodb.internal.operation.AsyncOperationHelper.createReadCommandAndExecuteAsync; import static com.mongodb.internal.operation.AsyncOperationHelper.decorateReadWithRetriesAsync; @@ -364,8 +365,11 @@ public AsyncReadOperation asAsyncExplainableOperation(@Nullable final Exp CommandReadOperation createExplainableOperation(@Nullable final ExplainVerbosity verbosity, final Decoder resultDecoder) { return new CommandReadOperation<>(getNamespace().getDatabaseName(), - (operationContext, serverDescription, connectionDescription) -> - asExplainCommand(getCommand(operationContext, MIN_WIRE_VERSION), verbosity), resultDecoder); + (operationContext, serverDescription, connectionDescription) -> { + BsonDocument command = getCommand(operationContext, MIN_WIRE_VERSION); + applyMaxTimeMS(operationContext.getTimeoutContext(), command); + return asExplainCommand(command, verbosity); + }, resultDecoder); } private BsonDocument getCommand(final OperationContext operationContext, final int maxWireVersion) { @@ -397,7 +401,7 @@ private BsonDocument getCommand(final OperationContext operationContext, final i if (isAwaitData()) { commandDocument.put("awaitData", BsonBoolean.TRUE); } else { - operationContext.getTimeoutContext().setMaxTimeOverride(0L); + operationContext.getTimeoutContext().disableMaxTimeOverride(); } } else { setNonTailableCursorMaxTimeSupplier(timeoutMode, operationContext); diff --git a/driver-core/src/main/com/mongodb/internal/operation/MapReduceToCollectionOperation.java b/driver-core/src/main/com/mongodb/internal/operation/MapReduceToCollectionOperation.java index b93be56d6f2..327aa5e5fa7 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/MapReduceToCollectionOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/MapReduceToCollectionOperation.java @@ -35,6 +35,7 @@ import static com.mongodb.assertions.Assertions.isTrue; import static com.mongodb.assertions.Assertions.notNull; +import static com.mongodb.internal.connection.CommandHelper.applyMaxTimeMS; import static com.mongodb.internal.operation.AsyncOperationHelper.CommandWriteTransformerAsync; import static com.mongodb.internal.operation.AsyncOperationHelper.executeCommandAsync; import static com.mongodb.internal.operation.CommandOperationHelper.CommandCreator; @@ -243,9 +244,11 @@ public AsyncReadOperation asExplainableOperationAsync(final Explai private CommandReadOperation createExplainableOperation(final ExplainVerbosity explainVerbosity) { return new CommandReadOperation<>(getNamespace().getDatabaseName(), - (operationContext, serverDescription, connectionDescription) -> - asExplainCommand(getCommandCreator().create(operationContext, serverDescription, connectionDescription), - explainVerbosity), new BsonDocumentCodec()); + (operationContext, serverDescription, connectionDescription) -> { + BsonDocument command = getCommandCreator().create(operationContext, serverDescription, connectionDescription); + applyMaxTimeMS(operationContext.getTimeoutContext(), command); + return asExplainCommand(command, explainVerbosity); + }, new BsonDocumentCodec()); } private CommandWriteTransformer transformer(final TimeoutContext timeoutContext) { diff --git a/driver-core/src/main/com/mongodb/internal/operation/MapReduceWithInlineResultsOperation.java b/driver-core/src/main/com/mongodb/internal/operation/MapReduceWithInlineResultsOperation.java index 695053e8845..273d8595ec8 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/MapReduceWithInlineResultsOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/MapReduceWithInlineResultsOperation.java @@ -32,6 +32,7 @@ import static com.mongodb.assertions.Assertions.notNull; import static com.mongodb.internal.async.ErrorHandlingResultCallback.errorHandlingCallback; +import static com.mongodb.internal.connection.CommandHelper.applyMaxTimeMS; import static com.mongodb.internal.operation.AsyncOperationHelper.CommandReadTransformerAsync; import static com.mongodb.internal.operation.AsyncOperationHelper.executeRetryableReadAsync; import static com.mongodb.internal.operation.CommandOperationHelper.CommandCreator; @@ -188,9 +189,12 @@ public AsyncReadOperation asExplainableOperationAsync(final Explai private CommandReadOperation createExplainableOperation(final ExplainVerbosity explainVerbosity) { return new CommandReadOperation<>(namespace.getDatabaseName(), - (operationContext, serverDescription, connectionDescription) -> - asExplainCommand(getCommandCreator().create(operationContext, serverDescription, connectionDescription), - explainVerbosity), new BsonDocumentCodec()); + (operationContext, serverDescription, connectionDescription) -> { + BsonDocument command = getCommandCreator().create(operationContext, serverDescription, connectionDescription); + applyMaxTimeMS(operationContext.getTimeoutContext(), command); + return asExplainCommand(command, + explainVerbosity); + }, new BsonDocumentCodec()); } private CommandReadTransformer> transformer() { 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 ac69f8742c7..04318635a06 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java +++ b/driver-core/src/main/com/mongodb/internal/operation/OperationHelper.java @@ -198,7 +198,7 @@ static boolean canRetryRead(final ServerDescription serverDescription, final Ope static void setNonTailableCursorMaxTimeSupplier(final TimeoutMode timeoutMode, final OperationContext operationContext) { if (timeoutMode == TimeoutMode.ITERATION) { - operationContext.getTimeoutContext().setMaxTimeOverride(0L); + operationContext.getTimeoutContext().disableMaxTimeOverride(); } } diff --git a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java index 7db4a079a5e..941e86748f4 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java @@ -18,8 +18,11 @@ import com.mongodb.ExplainVerbosity; import com.mongodb.MongoClientSettings; +import com.mongodb.MongoCommandException; import com.mongodb.client.model.Aggregates; import com.mongodb.client.model.Filters; +import com.mongodb.event.CommandStartedEvent; +import com.mongodb.internal.connection.TestCommandListener; import org.bson.BsonDocument; import org.bson.BsonInt32; import org.bson.Document; @@ -27,29 +30,36 @@ import org.junit.Before; import org.junit.Test; +import java.util.concurrent.TimeUnit; + import static com.mongodb.ClusterFixture.serverVersionAtLeast; import static com.mongodb.ClusterFixture.serverVersionLessThan; import static com.mongodb.client.Fixture.getDefaultDatabaseName; import static java.util.Collections.singletonList; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; public abstract class AbstractExplainTest { private MongoClient client; + private TestCommandListener commandListener; protected abstract MongoClient createMongoClient(MongoClientSettings settings); @Before public void setUp() { - client = createMongoClient(Fixture.getMongoClientSettings()); + commandListener = new TestCommandListener(); + client = createMongoClient(Fixture.getMongoClientSettingsBuilder().addCommandListener(commandListener).build()); } @After public void tearDown() { client.close(); + commandListener.reset(); } @Test @@ -83,6 +93,56 @@ public void testExplainOfFind() { assertFalse(explainBsonDocument.containsKey("executionStats")); } + @Test + public void testFindContainsMaxTimeMsInExplain() { + //given + MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) + .getCollection("explainTest", BsonDocument.class); + + FindIterable iterable = collection.find() + .maxTime(500, TimeUnit.MILLISECONDS); + + //when + iterable.explain(); + + //then + assertExplainableCommandContainMaxTimeMS(); + } + + @Test + public void testAggregateContainsMaxTimeMsInExplain() { + //given + MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) + .getCollection("explainTest", BsonDocument.class); + + AggregateIterable iterable = collection.aggregate( + singletonList(Aggregates.match(Filters.eq("_id", 1)))) + .maxTime(500, TimeUnit.MILLISECONDS); + + //when + iterable.explain(); + + //then + assertExplainableCommandContainMaxTimeMS(); + } + + @Test + public void testListSearchIndexesContainsMaxTimeMsInExplain() { + //given + assumeTrue(serverVersionAtLeast(6, 0)); + MongoCollection collection = client.getDatabase(getDefaultDatabaseName()) + .getCollection("explainTest", BsonDocument.class); + + ListSearchIndexesIterable iterable = collection.listSearchIndexes() + .maxTime(500, TimeUnit.MILLISECONDS); + + //when (we expect exception as listSearchIndexes is only supported in Atlas Search) + assertThrows(MongoCommandException.class, iterable::explain); + + //then + assertExplainableCommandContainMaxTimeMS(); + } + @Test public void testExplainOfAggregateWithNewResponseStructure() { // Aggregate explain is supported on earlier versions, but the structure of the response on which we're asserting in this test @@ -167,4 +227,14 @@ public void testExplainOfAggregateWithOldResponseStructure() { explainBsonDocument = iterable.explain(BsonDocument.class, ExplainVerbosity.QUERY_PLANNER); assertNotNull(explainBsonDocument); } + + private void assertExplainableCommandContainMaxTimeMS() { + assertEquals(1, commandListener.getCommandStartedEvents().size()); + CommandStartedEvent explain = commandListener.getCommandStartedEvent("explain"); + BsonDocument explainCommand = explain.getCommand(); + BsonDocument explainableCommand = explainCommand.getDocument("explain"); + + assertFalse(explainCommand.containsKey("maxTimeMS")); + assertTrue(explainableCommand.containsKey("maxTimeMS")); + } } From 71e9fc95473356bc35b19d0f03e1afca0a04c646 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 18 Sep 2024 17:49:50 -0700 Subject: [PATCH 2/3] Change assertion. JAVA-5580 --- .../com/mongodb/client/AbstractExplainTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java index 941e86748f4..b8e9e76c406 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java @@ -136,8 +136,12 @@ public void testListSearchIndexesContainsMaxTimeMsInExplain() { ListSearchIndexesIterable iterable = collection.listSearchIndexes() .maxTime(500, TimeUnit.MILLISECONDS); - //when (we expect exception as listSearchIndexes is only supported in Atlas Search) - assertThrows(MongoCommandException.class, iterable::explain); + //when + try { + iterable.explain(); + } catch (MongoCommandException throwable) { + //we expect listSearchIndexes is only supported in Atlas Search in some deployments. + } //then assertExplainableCommandContainMaxTimeMS(); From d3c8a18a24ffa5a9e71341a4985f581c554a1e36 Mon Sep 17 00:00:00 2001 From: "slav.babanin" Date: Wed, 18 Sep 2024 18:10:18 -0700 Subject: [PATCH 3/3] Fix static checks. JAVA-5580 --- .../test/functional/com/mongodb/client/AbstractExplainTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java index b8e9e76c406..d9df697b3ed 100644 --- a/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java +++ b/driver-sync/src/test/functional/com/mongodb/client/AbstractExplainTest.java @@ -39,7 +39,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue;