From c0b36c5dccae41a958eaf098b47f44055cf30a0f Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Mon, 24 Mar 2025 14:45:21 -0400 Subject: [PATCH 1/3] JAVA-5667 Eliminate unnecessary killCursors command when batchSize == limit --- .../internal/operation/Operations.java | 9 ++- .../unified-test-format/crud/find.json | 62 +++++++++++++++++++ .../client/internal/FindPublisherImpl.java | 9 ++- .../internal/FindPublisherImplTest.java | 2 +- .../client/internal/FindIterableImpl.java | 9 ++- .../GridFSFindIterableSpecification.groovy | 2 +- .../internal/FindIterableSpecification.groovy | 4 +- 7 files changed, 90 insertions(+), 7 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/operation/Operations.java b/driver-core/src/main/com/mongodb/internal/operation/Operations.java index 88af67a1204..daffa9e12fb 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/Operations.java +++ b/driver-core/src/main/com/mongodb/internal/operation/Operations.java @@ -188,11 +188,18 @@ FindOperation find(final MongoNamespace findNamespace, @Nulla private FindOperation createFindOperation(final MongoNamespace findNamespace, @Nullable final Bson filter, final Class resultClass, final FindOptions options) { + final int effectiveBatchSize; + if (options.getBatchSize() > 0 && options.getBatchSize() == options.getLimit()) { + // Eliminate unnecessary killCursors command when batchSize == limit + effectiveBatchSize = options.getBatchSize() + 1; + } else { + effectiveBatchSize = options.getBatchSize(); + } FindOperation operation = new FindOperation<>( findNamespace, codecRegistry.get(resultClass)) .retryReads(retryReads) .filter(filter == null ? new BsonDocument() : filter.toBsonDocument(documentClass, codecRegistry)) - .batchSize(options.getBatchSize()) + .batchSize(effectiveBatchSize) .skip(options.getSkip()) .limit(options.getLimit()) .projection(toBsonDocument(options.getProjection())) diff --git a/driver-core/src/test/resources/unified-test-format/crud/find.json b/driver-core/src/test/resources/unified-test-format/crud/find.json index 6bf1e4e4453..325cd96c218 100644 --- a/driver-core/src/test/resources/unified-test-format/crud/find.json +++ b/driver-core/src/test/resources/unified-test-format/crud/find.json @@ -237,6 +237,68 @@ ] } ] + }, + { + "description": "Find with batchSize equal to limit", + "operations": [ + { + "object": "collection0", + "name": "find", + "arguments": { + "filter": { + "_id": { + "$gt": 1 + } + }, + "sort": { + "_id": 1 + }, + "limit": 4, + "batchSize": 4 + }, + "expectResult": [ + { + "_id": 2, + "x": 22 + }, + { + "_id": 3, + "x": 33 + }, + { + "_id": 4, + "x": 44 + }, + { + "_id": 5, + "x": 55 + } + ] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "coll0", + "filter": { + "_id": { + "$gt": 1 + } + }, + "limit": 4, + "batchSize": 5 + }, + "commandName": "find", + "databaseName": "find-tests" + } + } + ] + } + ] } ] } diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java index ff9fb3a8036..b266d873481 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java @@ -233,6 +233,13 @@ Function, TimeoutSettings> getTimeoutSettings() { @Override AsyncReadOperation> asAsyncFirstReadOperation() { - return getOperations().findFirst(filter, getDocumentClass(), findOptions); + final FindOptions effectiveFindOptions; + if (findOptions.getBatchSize() > 0 && findOptions.getBatchSize() == findOptions.getLimit()) { + // Eliminate unnecessary killCursors command when batchSize == limit + effectiveFindOptions = findOptions.withBatchSize(findOptions.getBatchSize() + 1); + } else { + effectiveFindOptions = findOptions; + } + return getOperations().findFirst(filter, getDocumentClass(), effectiveFindOptions); } } diff --git a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/FindPublisherImplTest.java b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/FindPublisherImplTest.java index eab28373f2a..95bb6d43eca 100644 --- a/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/FindPublisherImplTest.java +++ b/driver-reactive-streams/src/test/unit/com/mongodb/reactivestreams/client/internal/FindPublisherImplTest.java @@ -88,7 +88,7 @@ void shouldBuildTheExpectedOperation() { .retryReads(true) .filter(new BsonDocument()) .allowDiskUse(false) - .batchSize(100) + .batchSize(101) .collation(COLLATION) .comment(new BsonString("my comment")) .cursorType(CursorType.NonTailable) diff --git a/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java b/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java index fbead0d7911..7888c72aa5f 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java @@ -245,7 +245,14 @@ private E executeExplain(final Class explainResultClass, @Nullable final } public ExplainableReadOperation> asReadOperation() { - return operations.find(filter, resultClass, findOptions); + final FindOptions effectiveFindOptions; + if (findOptions.getBatchSize() > 0 && findOptions.getBatchSize() == findOptions.getLimit()) { + // Eliminate unnecessary killCursors command when batchSize == limit + effectiveFindOptions = findOptions.withBatchSize(findOptions.getBatchSize() + 1); + } else { + effectiveFindOptions = findOptions; + } + return operations.find(filter, resultClass, effectiveFindOptions); } } diff --git a/driver-sync/src/test/unit/com/mongodb/client/gridfs/GridFSFindIterableSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/gridfs/GridFSFindIterableSpecification.groovy index 632e59a16d0..0da3d130d7a 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/gridfs/GridFSFindIterableSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/gridfs/GridFSFindIterableSpecification.groovy @@ -88,7 +88,7 @@ class GridFSFindIterableSpecification extends Specification { expect operation, isTheSameAs(new FindOperation(namespace, gridFSFileCodec) .filter(new BsonDocument('filter', new BsonInt32(2))) .sort(new BsonDocument('sort', new BsonInt32(2))) - .batchSize(99) + .batchSize(100) .limit(99) .skip(9) .noCursorTimeout(true) diff --git a/driver-sync/src/test/unit/com/mongodb/client/internal/FindIterableSpecification.groovy b/driver-sync/src/test/unit/com/mongodb/client/internal/FindIterableSpecification.groovy index e2f7cae2d62..fec8b9882c8 100644 --- a/driver-sync/src/test/unit/com/mongodb/client/internal/FindIterableSpecification.groovy +++ b/driver-sync/src/test/unit/com/mongodb/client/internal/FindIterableSpecification.groovy @@ -87,7 +87,7 @@ class FindIterableSpecification extends Specification { .filter(new BsonDocument('filter', new BsonInt32(1))) .sort(new BsonDocument('sort', new BsonInt32(1))) .projection(new BsonDocument('projection', new BsonInt32(1))) - .batchSize(100) + .batchSize(101) .limit(100) .skip(10) .cursorType(CursorType.NonTailable) @@ -132,7 +132,7 @@ class FindIterableSpecification extends Specification { .filter(new BsonDocument('filter', new BsonInt32(2))) .sort(new BsonDocument('sort', new BsonInt32(2))) .projection(new BsonDocument('projection', new BsonInt32(2))) - .batchSize(99) + .batchSize(100) .limit(99) .skip(9) .cursorType(CursorType.Tailable) From 84a175b3abd241bc6593d967ed206c513263fac6 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 25 Mar 2025 10:51:32 -0400 Subject: [PATCH 2/3] refactor by centralizing change in FindOptions#getBatchSize() --- .../com/mongodb/internal/client/model/FindOptions.java | 4 +++- .../main/com/mongodb/internal/operation/Operations.java | 9 +-------- .../client/internal/FindPublisherImpl.java | 9 +-------- .../com/mongodb/client/internal/FindIterableImpl.java | 9 +-------- 4 files changed, 6 insertions(+), 25 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java b/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java index 1c7f3ef9858..d44b39137b9 100644 --- a/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java +++ b/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java @@ -215,7 +215,9 @@ public FindOptions maxAwaitTime(final long maxAwaitTime, final TimeUnit timeUnit * @mongodb.driver.manual reference/method/cursor.batchSize/#cursor.batchSize Batch Size */ public int getBatchSize() { - return batchSize; + return batchSize > 0 && batchSize == limit + ? batchSize + 1 // avoid an open cursor on server side when batchSize and limit are equal + : batchSize; } /** diff --git a/driver-core/src/main/com/mongodb/internal/operation/Operations.java b/driver-core/src/main/com/mongodb/internal/operation/Operations.java index daffa9e12fb..88af67a1204 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/Operations.java +++ b/driver-core/src/main/com/mongodb/internal/operation/Operations.java @@ -188,18 +188,11 @@ FindOperation find(final MongoNamespace findNamespace, @Nulla private FindOperation createFindOperation(final MongoNamespace findNamespace, @Nullable final Bson filter, final Class resultClass, final FindOptions options) { - final int effectiveBatchSize; - if (options.getBatchSize() > 0 && options.getBatchSize() == options.getLimit()) { - // Eliminate unnecessary killCursors command when batchSize == limit - effectiveBatchSize = options.getBatchSize() + 1; - } else { - effectiveBatchSize = options.getBatchSize(); - } FindOperation operation = new FindOperation<>( findNamespace, codecRegistry.get(resultClass)) .retryReads(retryReads) .filter(filter == null ? new BsonDocument() : filter.toBsonDocument(documentClass, codecRegistry)) - .batchSize(effectiveBatchSize) + .batchSize(options.getBatchSize()) .skip(options.getSkip()) .limit(options.getLimit()) .projection(toBsonDocument(options.getProjection())) diff --git a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java index b266d873481..ff9fb3a8036 100644 --- a/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java +++ b/driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/FindPublisherImpl.java @@ -233,13 +233,6 @@ Function, TimeoutSettings> getTimeoutSettings() { @Override AsyncReadOperation> asAsyncFirstReadOperation() { - final FindOptions effectiveFindOptions; - if (findOptions.getBatchSize() > 0 && findOptions.getBatchSize() == findOptions.getLimit()) { - // Eliminate unnecessary killCursors command when batchSize == limit - effectiveFindOptions = findOptions.withBatchSize(findOptions.getBatchSize() + 1); - } else { - effectiveFindOptions = findOptions; - } - return getOperations().findFirst(filter, getDocumentClass(), effectiveFindOptions); + return getOperations().findFirst(filter, getDocumentClass(), findOptions); } } diff --git a/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java b/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java index 7888c72aa5f..fbead0d7911 100644 --- a/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java +++ b/driver-sync/src/main/com/mongodb/client/internal/FindIterableImpl.java @@ -245,14 +245,7 @@ private E executeExplain(final Class explainResultClass, @Nullable final } public ExplainableReadOperation> asReadOperation() { - final FindOptions effectiveFindOptions; - if (findOptions.getBatchSize() > 0 && findOptions.getBatchSize() == findOptions.getLimit()) { - // Eliminate unnecessary killCursors command when batchSize == limit - effectiveFindOptions = findOptions.withBatchSize(findOptions.getBatchSize() + 1); - } else { - effectiveFindOptions = findOptions; - } - return operations.find(filter, resultClass, effectiveFindOptions); + return operations.find(filter, resultClass, findOptions); } } From f7377813de9629e285cd119a0cb5d92467ca3025 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 25 Mar 2025 17:26:48 -0400 Subject: [PATCH 3/3] refactor code logic from FindOptions to FindOperation --- .../com/mongodb/internal/client/model/FindOptions.java | 4 +--- .../main/com/mongodb/internal/operation/FindOperation.java | 7 ++++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java b/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java index d44b39137b9..1c7f3ef9858 100644 --- a/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java +++ b/driver-core/src/main/com/mongodb/internal/client/model/FindOptions.java @@ -215,9 +215,7 @@ public FindOptions maxAwaitTime(final long maxAwaitTime, final TimeUnit timeUnit * @mongodb.driver.manual reference/method/cursor.batchSize/#cursor.batchSize Batch Size */ public int getBatchSize() { - return batchSize > 0 && batchSize == limit - ? batchSize + 1 // avoid an open cursor on server side when batchSize and limit are equal - : batchSize; + return batchSize; } /** 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 abdbc328a14..4ef5c2907e1 100644 --- a/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java +++ b/driver-core/src/main/com/mongodb/internal/operation/FindOperation.java @@ -390,7 +390,12 @@ private BsonDocument getCommand(final OperationContext operationContext, final i if (batchSize < 0 && Math.abs(batchSize) < limit) { commandDocument.put("limit", new BsonInt32(Math.abs(batchSize))); } else if (batchSize != 0) { - commandDocument.put("batchSize", new BsonInt32(Math.abs(batchSize))); + int effectiveBatchSize = Math.abs(batchSize); + if (effectiveBatchSize == limit) { + // avoid an open cursor on server side when batchSize and limit are equal + effectiveBatchSize++; + } + commandDocument.put("batchSize", new BsonInt32(effectiveBatchSize)); } } if (limit < 0 || batchSize < 0) {