Skip to content

Commit 5dd8ee5

Browse files
baileympearsondariakp
authored andcommitted
fix(NODE-6454): use timeoutcontext for state machine execute() cursor options (#4291)
1 parent f745b99 commit 5dd8ee5

File tree

6 files changed

+153
-52
lines changed

6 files changed

+153
-52
lines changed

src/client-side-encryption/state_machine.ts

+14-16
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
serialize
1212
} from '../bson';
1313
import { type ProxyOptions } from '../cmap/connection';
14+
import { CursorTimeoutContext } from '../cursor/abstract_cursor';
1415
import { getSocks, type SocksLib } from '../deps';
1516
import { MongoOperationTimeoutError } from '../error';
1617
import { type MongoClient, type MongoClientOptions } from '../mongo_client';
@@ -519,16 +520,16 @@ export class StateMachine {
519520
): Promise<Uint8Array | null> {
520521
const { db } = MongoDBCollectionNamespace.fromString(ns);
521522

522-
const collections = await client
523-
.db(db)
524-
.listCollections(filter, {
525-
promoteLongs: false,
526-
promoteValues: false,
527-
...(timeoutContext?.csotEnabled()
528-
? { timeoutMS: timeoutContext?.remainingTimeMS, timeoutMode: 'cursorLifetime' }
529-
: {})
530-
})
531-
.toArray();
523+
const cursor = client.db(db).listCollections(filter, {
524+
promoteLongs: false,
525+
promoteValues: false,
526+
timeoutContext: timeoutContext && new CursorTimeoutContext(timeoutContext, Symbol())
527+
});
528+
529+
// There is always exactly zero or one matching documents, so this should always exhaust the cursor
530+
// in a single batch. We call `toArray()` just to be safe and ensure that the cursor is always
531+
// exhausted and closed.
532+
const collections = await cursor.toArray();
532533

533534
const info = collections.length > 0 ? serialize(collections[0]) : null;
534535
return info;
@@ -582,12 +583,9 @@ export class StateMachine {
582583
return client
583584
.db(dbName)
584585
.collection<DataKey>(collectionName, { readConcern: { level: 'majority' } })
585-
.find(
586-
deserialize(filter),
587-
timeoutContext?.csotEnabled()
588-
? { timeoutMS: timeoutContext?.remainingTimeMS, timeoutMode: 'cursorLifetime' }
589-
: {}
590-
)
586+
.find(deserialize(filter), {
587+
timeoutContext: timeoutContext && new CursorTimeoutContext(timeoutContext, Symbol())
588+
})
591589
.toArray();
592590
}
593591
}

src/operations/list_collections.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Binary, Document } from '../bson';
22
import { CursorResponse } from '../cmap/wire_protocol/responses';
3-
import { type CursorTimeoutMode } from '../cursor/abstract_cursor';
3+
import { type CursorTimeoutContext, type CursorTimeoutMode } from '../cursor/abstract_cursor';
44
import type { Db } from '../db';
55
import type { Server } from '../sdam/server';
66
import type { ClientSession } from '../sessions';
@@ -19,6 +19,9 @@ export interface ListCollectionsOptions extends Omit<CommandOperationOptions, 'w
1919
batchSize?: number;
2020
/** @internal */
2121
timeoutMode?: CursorTimeoutMode;
22+
23+
/** @internal */
24+
timeoutContext?: CursorTimeoutContext;
2225
}
2326

2427
/** @internal */

test/integration/client-side-encryption/driver.test.ts

+67-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { type Binary, EJSON, UUID } from 'bson';
22
import { expect } from 'chai';
33
import * as crypto from 'crypto';
44
import * as sinon from 'sinon';
5+
import { setTimeout } from 'timers/promises';
56

67
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
78
import { ClientEncryption } from '../../../src/client-side-encryption/client_encryption';
@@ -15,7 +16,9 @@ import {
1516
MongoCryptCreateDataKeyError,
1617
MongoCryptCreateEncryptedCollectionError,
1718
MongoOperationTimeoutError,
18-
StateMachine
19+
resolveTimeoutOptions,
20+
StateMachine,
21+
TimeoutContext
1922
} from '../../mongodb';
2023
import {
2124
clearFailPoint,
@@ -25,6 +28,7 @@ import {
2528
measureDuration,
2629
sleep
2730
} from '../../tools/utils';
31+
import { filterForCommands } from '../shared';
2832

2933
const metadata: MongoDBMetadataUI = {
3034
requires: {
@@ -950,6 +954,68 @@ describe('CSOT', function () {
950954
}
951955
);
952956

957+
context('when the cursor times out and a killCursors is executed', function () {
958+
let client: MongoClient;
959+
let commands: (CommandStartedEvent & { command: { maxTimeMS?: number } })[] = [];
960+
961+
beforeEach(async function () {
962+
client = this.configuration.newClient({}, { monitorCommands: true });
963+
commands = [];
964+
client.on('commandStarted', filterForCommands('killCursors', commands));
965+
966+
await client.connect();
967+
const docs = Array.from({ length: 1200 }, (_, i) => ({ i }));
968+
969+
await client.db('test').collection('test').insertMany(docs);
970+
971+
await configureFailPoint(this.configuration, {
972+
configureFailPoint: 'failCommand',
973+
mode: 'alwaysOn',
974+
data: {
975+
failCommands: ['getMore'],
976+
blockConnection: true,
977+
blockTimeMS: 2000
978+
}
979+
});
980+
});
981+
982+
afterEach(async function () {
983+
await clearFailPoint(this.configuration);
984+
await client.close();
985+
});
986+
987+
it(
988+
'refreshes timeoutMS to the full timeout',
989+
{
990+
requires: {
991+
...metadata.requires,
992+
topology: '!load-balanced'
993+
}
994+
},
995+
async function () {
996+
const timeoutContext = TimeoutContext.create(
997+
resolveTimeoutOptions(client, { timeoutMS: 1900 })
998+
);
999+
1000+
await setTimeout(1500);
1001+
1002+
const { result: error } = await measureDuration(() =>
1003+
stateMachine
1004+
.fetchKeys(client, 'test.test', BSON.serialize({}), timeoutContext)
1005+
.catch(e => e)
1006+
);
1007+
expect(error).to.be.instanceOf(MongoOperationTimeoutError);
1008+
1009+
const [
1010+
{
1011+
command: { maxTimeMS }
1012+
}
1013+
] = commands;
1014+
expect(maxTimeMS).to.be.greaterThan(1800);
1015+
}
1016+
);
1017+
});
1018+
9531019
context('when csot is not enabled and fetchKeys() is delayed', function () {
9541020
let encryptedClient;
9551021

test/integration/crud/client_bulk_write.test.ts

+30-19
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import {
1414
clearFailPoint,
1515
configureFailPoint,
1616
makeMultiBatchWrite,
17-
makeMultiResponseBatchModelArray
17+
makeMultiResponseBatchModelArray,
18+
mergeTestMetadata
1819
} from '../../tools/utils';
1920
import { filterForCommands } from '../shared';
2021

@@ -268,7 +269,7 @@ describe('Client Bulk Write', function () {
268269

269270
beforeEach(async function () {
270271
client = this.configuration.newClient({}, { monitorCommands: true, minPoolSize: 5 });
271-
client.on('commandStarted', filterForCommands(['getMore'], commands));
272+
client.on('commandStarted', filterForCommands(['getMore', 'killCursors'], commands));
272273
await client.connect();
273274

274275
await configureFailPoint(this.configuration, {
@@ -278,25 +279,35 @@ describe('Client Bulk Write', function () {
278279
});
279280
});
280281

281-
it('the bulk write operation times out', metadata, async function () {
282-
const models = await makeMultiResponseBatchModelArray(this.configuration);
283-
const start = now();
284-
const timeoutError = await client
285-
.bulkWrite(models, {
286-
verboseResults: true,
287-
timeoutMS: 1500
288-
})
289-
.catch(e => e);
282+
it(
283+
'the bulk write operation times out',
284+
mergeTestMetadata(metadata, {
285+
requires: {
286+
// this test has timing logic that depends on killCursors being executed, which does
287+
// not happen in load balanced mode
288+
topology: '!load-balanced'
289+
}
290+
}),
291+
async function () {
292+
const models = await makeMultiResponseBatchModelArray(this.configuration);
293+
const start = now();
294+
const timeoutError = await client
295+
.bulkWrite(models, {
296+
verboseResults: true,
297+
timeoutMS: 1500
298+
})
299+
.catch(e => e);
290300

291-
const end = now();
292-
expect(timeoutError).to.be.instanceOf(MongoOperationTimeoutError);
301+
const end = now();
302+
expect(timeoutError).to.be.instanceOf(MongoOperationTimeoutError);
293303

294-
// DRIVERS-3005 - killCursors causes cursor cleanup to extend past timeoutMS.
295-
// The amount of time killCursors takes is wildly variable and can take up to almost
296-
// 600-700ms sometimes.
297-
expect(end - start).to.be.within(1500, 1500 + 800);
298-
expect(commands).to.have.lengthOf(1);
299-
});
304+
// DRIVERS-3005 - killCursors causes cursor cleanup to extend past timeoutMS.
305+
// The amount of time killCursors takes is wildly variable and can take up to almost
306+
// 600-700ms sometimes.
307+
expect(end - start).to.be.within(1500, 1500 + 800);
308+
expect(commands.map(({ commandName }) => commandName)).to.have.lengthOf(2);
309+
}
310+
);
300311
});
301312

302313
describe('if the cursor encounters an error and a killCursors is sent', function () {

test/tools/utils.ts

+16
Original file line numberDiff line numberDiff line change
@@ -689,3 +689,19 @@ export async function measureDuration<T>(f: () => Promise<T>): Promise<{
689689
result
690690
};
691691
}
692+
693+
export function mergeTestMetadata(
694+
metadata: MongoDBMetadataUI,
695+
newMetadata: MongoDBMetadataUI
696+
): MongoDBMetadataUI {
697+
return {
698+
requires: {
699+
...metadata.requires,
700+
...newMetadata.requires
701+
},
702+
sessions: {
703+
...metadata.sessions,
704+
...newMetadata.sessions
705+
}
706+
};
707+
}

test/unit/client-side-encryption/state_machine.test.ts

+22-15
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import {
1616
BSON,
1717
Collection,
1818
CSOTTimeoutContext,
19+
CursorTimeoutContext,
20+
type FindOptions,
1921
Int32,
2022
Long,
2123
MongoClient,
@@ -484,26 +486,29 @@ describe('StateMachine', function () {
484486
});
485487

486488
context('when StateMachine.fetchKeys() is passed a `CSOTimeoutContext`', function () {
487-
it('collection.find runs with its timeoutMS property set to remainingTimeMS', async function () {
488-
const timeoutContext = new CSOTTimeoutContext({
489+
it('collection.find uses the provided timeout context', async function () {
490+
const context = new CSOTTimeoutContext({
489491
timeoutMS: 500,
490492
serverSelectionTimeoutMS: 30000
491493
});
492-
await sleep(300);
494+
493495
await stateMachine
494-
.fetchKeys(client, 'keyVault', BSON.serialize({ a: 1 }), timeoutContext)
496+
.fetchKeys(client, 'keyVault', BSON.serialize({ a: 1 }), context)
495497
.catch(e => squashError(e));
496-
expect(findSpy.getCalls()[0].args[1].timeoutMS).to.not.be.undefined;
497-
expect(findSpy.getCalls()[0].args[1].timeoutMS).to.be.lessThanOrEqual(205);
498+
499+
const { timeoutContext } = findSpy.getCalls()[0].args[1] as FindOptions;
500+
expect(timeoutContext).to.be.instanceOf(CursorTimeoutContext);
501+
expect(timeoutContext.timeoutContext).to.equal(context);
498502
});
499503
});
500504

501505
context('when StateMachine.fetchKeys() is not passed a `CSOTimeoutContext`', function () {
502-
it('collection.find runs with an undefined timeoutMS property', async function () {
506+
it('a timeoutContext is not provided to the find cursor', async function () {
503507
await stateMachine
504508
.fetchKeys(client, 'keyVault', BSON.serialize({ a: 1 }))
505509
.catch(e => squashError(e));
506-
expect(findSpy.getCalls()[0].args[1].timeoutMS).to.be.undefined;
510+
const { timeoutContext } = findSpy.getCalls()[0].args[1] as FindOptions;
511+
expect(timeoutContext).to.be.undefined;
507512
});
508513
});
509514
});
@@ -564,29 +569,31 @@ describe('StateMachine', function () {
564569
context(
565570
'when StateMachine.fetchCollectionInfo() is passed a `CSOTimeoutContext`',
566571
function () {
567-
it('listCollections runs with its timeoutMS property set to remainingTimeMS', async function () {
568-
const timeoutContext = new CSOTTimeoutContext({
572+
it('listCollections uses the provided timeoutContext', async function () {
573+
const context = new CSOTTimeoutContext({
569574
timeoutMS: 500,
570575
serverSelectionTimeoutMS: 30000
571576
});
572577
await sleep(300);
573578
await stateMachine
574-
.fetchCollectionInfo(client, 'keyVault', BSON.serialize({ a: 1 }), timeoutContext)
579+
.fetchCollectionInfo(client, 'keyVault', BSON.serialize({ a: 1 }), context)
575580
.catch(e => squashError(e));
576-
expect(listCollectionsSpy.getCalls()[0].args[1].timeoutMS).to.not.be.undefined;
577-
expect(listCollectionsSpy.getCalls()[0].args[1].timeoutMS).to.be.lessThanOrEqual(205);
581+
const [_filter, { timeoutContext }] = listCollectionsSpy.getCalls()[0].args;
582+
expect(timeoutContext).to.exist;
583+
expect(timeoutContext.timeoutContext).to.equal(context);
578584
});
579585
}
580586
);
581587

582588
context(
583589
'when StateMachine.fetchCollectionInfo() is not passed a `CSOTimeoutContext`',
584590
function () {
585-
it('listCollections runs with an undefined timeoutMS property', async function () {
591+
it('no timeoutContext is provided to listCollections', async function () {
586592
await stateMachine
587593
.fetchCollectionInfo(client, 'keyVault', BSON.serialize({ a: 1 }))
588594
.catch(e => squashError(e));
589-
expect(listCollectionsSpy.getCalls()[0].args[1].timeoutMS).to.be.undefined;
595+
const [_filter, { timeoutContext }] = listCollectionsSpy.getCalls()[0].args;
596+
expect(timeoutContext).not.to.exist;
590597
});
591598
}
592599
);

0 commit comments

Comments
 (0)