Skip to content

Commit d18dbec

Browse files
sanych-sunsanych-sun
authored
CSHARP-5377: Eliminate unnecessary killCursors command when batchSize == limit (#1729)
1 parent 67752b3 commit d18dbec

File tree

4 files changed

+224
-63
lines changed

4 files changed

+224
-63
lines changed

‎specifications/crud/tests/unified/find.json

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,154 @@
151151
]
152152
}
153153
]
154+
},
155+
{
156+
"description": "Find with filter",
157+
"operations": [
158+
{
159+
"object": "collection0",
160+
"name": "find",
161+
"arguments": {
162+
"filter": {
163+
"_id": 1
164+
}
165+
},
166+
"expectResult": [
167+
{
168+
"_id": 1,
169+
"x": 11
170+
}
171+
]
172+
}
173+
]
174+
},
175+
{
176+
"description": "Find with filter, sort, skip, and limit",
177+
"operations": [
178+
{
179+
"object": "collection0",
180+
"name": "find",
181+
"arguments": {
182+
"filter": {
183+
"_id": {
184+
"$gt": 2
185+
}
186+
},
187+
"sort": {
188+
"_id": 1
189+
},
190+
"skip": 2,
191+
"limit": 2
192+
},
193+
"expectResult": [
194+
{
195+
"_id": 5,
196+
"x": 55
197+
},
198+
{
199+
"_id": 6,
200+
"x": 66
201+
}
202+
]
203+
}
204+
]
205+
},
206+
{
207+
"description": "Find with limit, sort, and batchsize",
208+
"operations": [
209+
{
210+
"object": "collection0",
211+
"name": "find",
212+
"arguments": {
213+
"filter": {},
214+
"sort": {
215+
"_id": 1
216+
},
217+
"limit": 4,
218+
"batchSize": 2
219+
},
220+
"expectResult": [
221+
{
222+
"_id": 1,
223+
"x": 11
224+
},
225+
{
226+
"_id": 2,
227+
"x": 22
228+
},
229+
{
230+
"_id": 3,
231+
"x": 33
232+
},
233+
{
234+
"_id": 4,
235+
"x": 44
236+
}
237+
]
238+
}
239+
]
240+
},
241+
{
242+
"description": "Find with batchSize equal to limit",
243+
"operations": [
244+
{
245+
"object": "collection0",
246+
"name": "find",
247+
"arguments": {
248+
"filter": {
249+
"_id": {
250+
"$gt": 1
251+
}
252+
},
253+
"sort": {
254+
"_id": 1
255+
},
256+
"limit": 4,
257+
"batchSize": 4
258+
},
259+
"expectResult": [
260+
{
261+
"_id": 2,
262+
"x": 22
263+
},
264+
{
265+
"_id": 3,
266+
"x": 33
267+
},
268+
{
269+
"_id": 4,
270+
"x": 44
271+
},
272+
{
273+
"_id": 5,
274+
"x": 55
275+
}
276+
]
277+
}
278+
],
279+
"expectEvents": [
280+
{
281+
"client": "client0",
282+
"events": [
283+
{
284+
"commandStartedEvent": {
285+
"command": {
286+
"find": "coll0",
287+
"filter": {
288+
"_id": {
289+
"$gt": 1
290+
}
291+
},
292+
"limit": 4,
293+
"batchSize": 5
294+
},
295+
"commandName": "find",
296+
"databaseName": "find-tests"
297+
}
298+
}
299+
]
300+
}
301+
]
154302
}
155303
]
156304
}

‎specifications/crud/tests/unified/find.yml

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,71 @@ tests:
6565
batchSize: 2
6666
commandName: getMore
6767
databaseName: *database0Name
68-
68+
-
69+
description: 'Find with filter'
70+
operations:
71+
-
72+
object: *collection0
73+
name: find
74+
arguments:
75+
filter: { _id: 1 }
76+
expectResult:
77+
- { _id: 1, x: 11 }
78+
-
79+
description: 'Find with filter, sort, skip, and limit'
80+
operations:
81+
-
82+
object: *collection0
83+
name: find
84+
arguments:
85+
filter: { _id: { $gt: 2 } }
86+
sort: { _id: 1 }
87+
skip: 2
88+
limit: 2
89+
expectResult:
90+
- { _id: 5, x: 55 }
91+
- { _id: 6, x: 66 }
92+
-
93+
description: 'Find with limit, sort, and batchsize'
94+
operations:
95+
-
96+
object: *collection0
97+
name: find
98+
arguments:
99+
filter: { }
100+
sort: { _id: 1 }
101+
limit: 4
102+
batchSize: 2
103+
expectResult:
104+
- { _id: 1, x: 11 }
105+
- { _id: 2, x: 22 }
106+
- { _id: 3, x: 33 }
107+
- { _id: 4, x: 44 }
108+
-
109+
description: 'Find with batchSize equal to limit'
110+
operations:
111+
-
112+
object: *collection0
113+
name: find
114+
arguments:
115+
filter: { _id: { $gt: 1 } }
116+
sort: { _id: 1 }
117+
limit: 4
118+
batchSize: 4
119+
expectResult:
120+
- { _id: 2, x: 22 }
121+
- { _id: 3, x: 33 }
122+
- { _id: 4, x: 44 }
123+
- { _id: 5, x: 55 }
124+
expectEvents:
125+
- client: *client0
126+
events:
127+
- commandStartedEvent:
128+
command:
129+
find: *collection0Name
130+
filter: { _id: { $gt: 1 } }
131+
limit: 4
132+
# Drivers use limit + 1 for batchSize to ensure the server closes the cursor
133+
batchSize: 5
134+
commandName: find
135+
databaseName: *database0Name

‎src/MongoDB.Driver/Core/Operations/FindOperation.cs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ internal sealed class FindOperation<TDocument> : IReadOperation<IAsyncCursor<TDo
4646
private BsonValue _comment;
4747
private CursorType _cursorType;
4848
private BsonDocument _filter;
49-
private int? _firstBatchSize;
5049
private BsonValue _hint;
5150
private BsonDocument _let;
5251
private int? _limit;
@@ -125,12 +124,6 @@ public BsonDocument Filter
125124
set { _filter = value; }
126125
}
127126

128-
public int? FirstBatchSize
129-
{
130-
get { return _firstBatchSize; }
131-
set { _firstBatchSize = Ensure.IsNullOrGreaterThanOrEqualToZero(value, nameof(value)); }
132-
}
133-
134127
public BsonValue Hint
135128
{
136129
get { return _hint; }
@@ -249,7 +242,13 @@ public BsonDocument CreateCommand(ConnectionDescription connectionDescription, I
249242
var wireVersion = connectionDescription.MaxWireVersion;
250243
FindProjectionChecker.ThrowIfAggregationExpressionIsUsedWhenNotSupported(_projection, wireVersion);
251244

252-
var firstBatchSize = _firstBatchSize ?? (_batchSize > 0 ? _batchSize : null);
245+
var batchSize = _batchSize;
246+
// https://github.com/mongodb/specifications/blob/668992950d975d3163e538849dd20383a214fc37/source/crud/crud.md?plain=1#L803
247+
if (batchSize.HasValue && batchSize == _limit)
248+
{
249+
batchSize = _limit + 1;
250+
}
251+
253252
var isShardRouter = connectionDescription.HelloResult.ServerType == ServerType.ShardRouter;
254253

255254
var effectiveComment = _comment;
@@ -271,7 +270,7 @@ public BsonDocument CreateCommand(ConnectionDescription connectionDescription, I
271270
{ "hint", effectiveHint, effectiveHint != null },
272271
{ "skip", () => _skip.Value, _skip.HasValue },
273272
{ "limit", () => Math.Abs(_limit.Value), _limit.HasValue && _limit != 0 },
274-
{ "batchSize", () => firstBatchSize.Value, firstBatchSize.HasValue },
273+
{ "batchSize", () => batchSize.Value, batchSize.HasValue && batchSize > 0 },
275274
{ "singleBatch", () => _limit < 0 || _singleBatch.Value, _limit < 0 || _singleBatch.HasValue },
276275
{ "comment", effectiveComment, effectiveComment != null },
277276
{ "maxTimeMS", () => MaxTimeHelper.ToMaxTimeMS(effectiveMaxTime.Value), effectiveMaxTime.HasValue },

‎tests/MongoDB.Driver.Tests/Core/Operations/FindOperationTests.cs

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ public void constructor_should_initialize_instance()
136136
subject.Comment.Should().BeNull();
137137
subject.CursorType.Should().Be(CursorType.NonTailable);
138138
subject.Filter.Should().BeNull();
139-
subject.FirstBatchSize.Should().NotHaveValue();
140139
subject.Hint.Should().BeNull();
141140
subject.Limit.Should().NotHaveValue();
142141
subject.Max.Should().BeNull();
@@ -352,30 +351,6 @@ public void CreateCommand_should_return_expected_result_when_Filter_is_set(
352351
result.Should().Be(expectedResult);
353352
}
354353

355-
[Theory]
356-
[ParameterAttributeData]
357-
public void CreateCommand_should_return_expected_result_when_FirstBatchSize_is_set(
358-
[Values(null, 0, 1)]
359-
int? firstBatchSize)
360-
{
361-
var subject = new FindOperation<BsonDocument>(_collectionNamespace, BsonDocumentSerializer.Instance, _messageEncoderSettings)
362-
{
363-
FirstBatchSize = firstBatchSize
364-
};
365-
366-
var connectionDescription = OperationTestHelper.CreateConnectionDescription();
367-
var session = OperationTestHelper.CreateSession();
368-
369-
var result = subject.CreateCommand(connectionDescription, session);
370-
371-
var expectedResult = new BsonDocument
372-
{
373-
{ "find", _collectionNamespace.CollectionName },
374-
{ "batchSize", () => firstBatchSize.Value, firstBatchSize.HasValue }
375-
};
376-
result.Should().Be(expectedResult);
377-
}
378-
379354
[Theory]
380355
[ParameterAttributeData]
381356
public void CreateCommand_should_return_expected_result_when_Hint_is_set(
@@ -937,34 +912,6 @@ public void Filter_get_and_set_should_work(
937912
result.Should().BeSameAs(value);
938913
}
939914

940-
[Theory]
941-
[ParameterAttributeData]
942-
public void FirstBatchSize_get_and_set_should_work(
943-
[Values(null, 0, 1, 2)]
944-
int? value)
945-
{
946-
var subject = new FindOperation<BsonDocument>(_collectionNamespace, BsonDocumentSerializer.Instance, _messageEncoderSettings);
947-
948-
subject.FirstBatchSize = value;
949-
var result = subject.FirstBatchSize;
950-
951-
result.Should().Be(value);
952-
}
953-
954-
[Theory]
955-
[ParameterAttributeData]
956-
public void FirstBatchSize_set_should_throw_when_value_is_invalid(
957-
[Values(-2, -1)]
958-
int value)
959-
{
960-
var subject = new FindOperation<BsonDocument>(_collectionNamespace, BsonDocumentSerializer.Instance, _messageEncoderSettings);
961-
962-
var exception = Record.Exception(() => { subject.FirstBatchSize = value; });
963-
964-
var argumentOutOfRangeException = exception.Should().BeOfType<ArgumentOutOfRangeException>().Subject;
965-
argumentOutOfRangeException.ParamName.Should().Be("value");
966-
}
967-
968915
[Theory]
969916
[ParameterAttributeData]
970917
public void Hint_get_and_set_should_work(

0 commit comments

Comments
 (0)