From 25192e145d2d079d8172df606d14cf11491893a5 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 14 Oct 2024 16:22:06 -0600 Subject: [PATCH 1/8] thresholds --- ...ient_side_operations_timeout.prose.test.ts | 4 +- .../crud/client_bulk_write.test.ts | 4 +- .../node-specific/abstract_cursor.test.ts | 51 +++++++++++++------ test/tools/utils.ts | 12 +++-- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts index 458447a437c..3ec0f23f794 100644 --- a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts +++ b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts @@ -21,7 +21,7 @@ import { promiseWithResolvers, squashError } from '../../mongodb'; -import { type FailPoint, makeMultiBatchWrite } from '../../tools/utils'; +import { type FailPoint, makeMultiBatchWrite, waitUntilPoolsFilled } from '../../tools/utils'; import { filterForCommands } from '../shared'; // TODO(NODE-5824): Implement CSOT prose tests @@ -343,7 +343,7 @@ describe('CSOT spec prose tests', function () { client = this.configuration.newClient(undefined, { monitorCommands: true, - timeoutMS: 100, + timeoutMS: 150, minPoolSize: 20 }); await client.connect(); diff --git a/test/integration/crud/client_bulk_write.test.ts b/test/integration/crud/client_bulk_write.test.ts index 6177077b632..d9b5512b76e 100644 --- a/test/integration/crud/client_bulk_write.test.ts +++ b/test/integration/crud/client_bulk_write.test.ts @@ -320,7 +320,9 @@ describe('Client Bulk Write', function () { it( 'timeoutMS is refreshed to the timeoutMS passed to the bulk write for the killCursors command', - metadata, + { + requires: { ...metadata.requires, topology: '!load-balanced' } + }, async function () { const models = await makeMultiResponseBatchModelArray(this.configuration); const timeoutError = await client diff --git a/test/integration/node-specific/abstract_cursor.test.ts b/test/integration/node-specific/abstract_cursor.test.ts index 136e72a3499..57b9fbf0370 100644 --- a/test/integration/node-specific/abstract_cursor.test.ts +++ b/test/integration/node-specific/abstract_cursor.test.ts @@ -7,6 +7,7 @@ import { inspect } from 'util'; import { AbstractCursor, type Collection, + type CommandStartedEvent, CursorTimeoutContext, CursorTimeoutMode, type FindCursor, @@ -17,7 +18,8 @@ import { MongoServerError, TimeoutContext } from '../../mongodb'; -import { type FailPoint } from '../../tools/utils'; +import { clearFailPoint, configureFailPoint } from '../../tools/utils'; +import { filterForCommands } from '../shared'; describe('class AbstractCursor', function () { describe('regression tests NODE-5372', function () { @@ -405,9 +407,11 @@ describe('class AbstractCursor', function () { let client: MongoClient; let collection: Collection; let context: CursorTimeoutContext; + const commands: CommandStartedEvent[] = []; beforeEach(async function () { - client = this.configuration.newClient(); + client = this.configuration.newClient({}, { monitorCommands: true }); + client.on('commandStarted', filterForCommands('killCursors', commands)); collection = client.db('abstract_cursor_integration').collection('test'); @@ -472,16 +476,18 @@ describe('class AbstractCursor', function () { }); }); - describe('when the cursor refreshes the timeout for killCursors', function () { - it( - 'the provided timeoutContext is not modified', - { - requires: { - mongodb: '>=4.4' - } - }, - async function () { - await client.db('admin').command({ + describe('when the cursor refreshes the timeout for killCursors' + i, function () { + let uri: string; + + before(function () { + uri = this.configuration.url({ useMultipleMongoses: false }); + }); + + beforeEach(async function () { + commands.length = 0; + await configureFailPoint( + this.configuration, + { configureFailPoint: 'failCommand', mode: { times: 1 }, data: { @@ -489,13 +495,29 @@ describe('class AbstractCursor', function () { blockConnection: true, blockTimeMS: 5000 } - } as FailPoint); + }, + uri + ); + }); + afterEach(async function () { + await clearFailPoint(this.configuration, uri); + }); + + it( + 'the provided timeoutContext is not modified' + i, + { + requires: { + mongodb: '>=4.4', + topology: '!load-balanced' + } + }, + async function () { const cursor = collection.find( {}, { timeoutContext: context, - timeoutMS: 1000, + timeoutMS: 150, timeoutMode: CursorTimeoutMode.LIFETIME, batchSize: 1 } @@ -504,7 +526,6 @@ describe('class AbstractCursor', function () { const error = await cursor.toArray().catch(e => e); expect(error).to.be.instanceof(MongoOperationTimeoutError); - // @ts-expect-error We know we have a CSOT timeout context but TS does not. expect(context.timeoutContext.remainingTimeMS).to.be.lessThan(0); } ); diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 8ebc5e8f532..38c0da6c092 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -601,8 +601,12 @@ export async function waitUntilPoolsFilled( await Promise.all([wait$(), client.connect()]); } -export async function configureFailPoint(configuration: TestConfiguration, failPoint: FailPoint) { - const utilClient = configuration.newClient(); +export async function configureFailPoint( + configuration: TestConfiguration, + failPoint: FailPoint, + uri = configuration.url() +) { + const utilClient = configuration.newClient(uri); await utilClient.connect(); try { @@ -612,8 +616,8 @@ export async function configureFailPoint(configuration: TestConfiguration, failP } } -export async function clearFailPoint(configuration: TestConfiguration) { - const utilClient = configuration.newClient(); +export async function clearFailPoint(configuration: TestConfiguration, uri = configuration.url()) { + const utilClient = configuration.newClient(uri); await utilClient.connect(); try { From 698c924fe5cd64f96ee15768efc69e01d25005c8 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 14 Oct 2024 16:26:04 -0600 Subject: [PATCH 2/8] undefined i --- test/integration/node-specific/abstract_cursor.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/node-specific/abstract_cursor.test.ts b/test/integration/node-specific/abstract_cursor.test.ts index 57b9fbf0370..ad7fbe5e654 100644 --- a/test/integration/node-specific/abstract_cursor.test.ts +++ b/test/integration/node-specific/abstract_cursor.test.ts @@ -476,7 +476,7 @@ describe('class AbstractCursor', function () { }); }); - describe('when the cursor refreshes the timeout for killCursors' + i, function () { + describe('when the cursor refreshes the timeout for killCursors', function () { let uri: string; before(function () { @@ -505,7 +505,7 @@ describe('class AbstractCursor', function () { }); it( - 'the provided timeoutContext is not modified' + i, + 'the provided timeoutContext is not modified', { requires: { mongodb: '>=4.4', From 93574eeb77f5d79d39c3ff5b081a822a03349177 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 15 Oct 2024 09:58:34 -0600 Subject: [PATCH 3/8] fix lint --- .../client_side_operations_timeout.prose.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts index 3ec0f23f794..146a2585c52 100644 --- a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts +++ b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts @@ -21,7 +21,7 @@ import { promiseWithResolvers, squashError } from '../../mongodb'; -import { type FailPoint, makeMultiBatchWrite, waitUntilPoolsFilled } from '../../tools/utils'; +import { type FailPoint, makeMultiBatchWrite } from '../../tools/utils'; import { filterForCommands } from '../shared'; // TODO(NODE-5824): Implement CSOT prose tests From da470c8da2dad9be0ccd8b4ac17fa4275bcd1be7 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 15 Oct 2024 10:04:54 -0600 Subject: [PATCH 4/8] hopefully make test more reliable --- test/integration/node-specific/abstract_cursor.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/integration/node-specific/abstract_cursor.test.ts b/test/integration/node-specific/abstract_cursor.test.ts index ad7fbe5e654..b68d00155d8 100644 --- a/test/integration/node-specific/abstract_cursor.test.ts +++ b/test/integration/node-specific/abstract_cursor.test.ts @@ -523,10 +523,15 @@ describe('class AbstractCursor', function () { } ); + const refresh = sinon.spy(context, 'refresh'); + const clear = sinon.spy(context, 'clear'); + const refreshed = sinon.spy(context, 'refreshed'); const error = await cursor.toArray().catch(e => e); expect(error).to.be.instanceof(MongoOperationTimeoutError); - expect(context.timeoutContext.remainingTimeMS).to.be.lessThan(0); + expect(refresh.called).to.be.false; + expect(clear.called).to.be.false; + expect(refreshed.called).to.be.true; } ); }); From 85e09ca0b766f87fefa0ee77173a36b1a1ff7288 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 15 Oct 2024 10:46:37 -0600 Subject: [PATCH 5/8] fix failing tests? --- test/integration/node-specific/abstract_cursor.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/node-specific/abstract_cursor.test.ts b/test/integration/node-specific/abstract_cursor.test.ts index b68d00155d8..8e154e1dc3e 100644 --- a/test/integration/node-specific/abstract_cursor.test.ts +++ b/test/integration/node-specific/abstract_cursor.test.ts @@ -524,13 +524,11 @@ describe('class AbstractCursor', function () { ); const refresh = sinon.spy(context, 'refresh'); - const clear = sinon.spy(context, 'clear'); const refreshed = sinon.spy(context, 'refreshed'); const error = await cursor.toArray().catch(e => e); expect(error).to.be.instanceof(MongoOperationTimeoutError); expect(refresh.called).to.be.false; - expect(clear.called).to.be.false; expect(refreshed.called).to.be.true; } ); From f049cf77b1f95019cc5b7024f89ecd2cfb418b27 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 15 Oct 2024 11:11:48 -0600 Subject: [PATCH 6/8] more flaky tests? --- .../node_csot.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/client-side-operations-timeout/node_csot.test.ts b/test/integration/client-side-operations-timeout/node_csot.test.ts index a981a9113df..68a52f80160 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -361,7 +361,7 @@ describe('CSOT driver tests', metadata, () => { describe('Non-Tailable cursors', () => { let client: MongoClient; let internalClient: MongoClient; - let commandStarted: CommandStartedEvent[]; + let commandStarted: (CommandStartedEvent & { command: { maxTimeMS?: number } })[]; let commandSucceeded: CommandSucceededEvent[]; const failpoint: FailPoint = { configureFailPoint: 'failCommand', @@ -369,7 +369,7 @@ describe('CSOT driver tests', metadata, () => { data: { failCommands: ['find', 'getMore'], blockConnection: true, - blockTimeMS: 50 + blockTimeMS: 150 } }; @@ -435,7 +435,7 @@ describe('CSOT driver tests', metadata, () => { const cursor = client .db('db') .collection('coll') - .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 100 }) + .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 200 }) .project({ _id: 0 }); // Iterating over 3 documents in the collection, each artificially taking ~50 ms due to failpoint. If timeoutMS is not refreshed, then we'd expect to error @@ -457,17 +457,17 @@ describe('CSOT driver tests', metadata, () => { const cursor = client .db('db') .collection('coll') - .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 100 }) + .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 200 }) .project({ _id: 0 }); await cursor.toArray(); expect(commandStarted).to.have.length.gte(3); // Find and 2 getMores + expect( commandStarted.filter(ev => { return ( - ev.command.find != null && - ev.command.getMore != null && - ev.command.maxTimeMS != null + (ev.command.find != null && ev.command.maxTimeMS != null) || + (ev.command.getMore != null && ev.command.maxTimeMS != null) ); }) ).to.have.lengthOf(0); From 55ede467250f19826bf755aa4149d495ffcaff63 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 15 Oct 2024 13:56:34 -0600 Subject: [PATCH 7/8] comments --- .../node_csot.test.ts | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/test/integration/client-side-operations-timeout/node_csot.test.ts b/test/integration/client-side-operations-timeout/node_csot.test.ts index 68a52f80160..4ab28e0bb31 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -461,16 +461,21 @@ describe('CSOT driver tests', metadata, () => { .project({ _id: 0 }); await cursor.toArray(); - expect(commandStarted).to.have.length.gte(3); // Find and 2 getMores - - expect( - commandStarted.filter(ev => { - return ( - (ev.command.find != null && ev.command.maxTimeMS != null) || - (ev.command.getMore != null && ev.command.maxTimeMS != null) - ); - }) - ).to.have.lengthOf(0); + const commands = commandStarted.filter(c => + ['find', 'getMore'].includes(c.commandName) + ); + expect(commands).to.have.lengthOf(4); // Find and 2 getMores + + const [ + { command: aggregate }, + { command: getMore1 }, + { command: getMore2 }, + { command: getMore3 } + ] = commands; + expect(aggregate).not.to.have.property('maxTimeMS'); + expect(getMore1).not.to.have.property('maxTimeMS'); + expect(getMore2).not.to.have.property('maxTimeMS'); + expect(getMore3).not.to.have.property('maxTimeMS'); } ); }); From d8d6d21d68ebc9f6d16bcda08dc772179ca73890 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 17 Oct 2024 12:39:42 -0600 Subject: [PATCH 8/8] attempt to address flakes --- .../run-resource-management-feature-integration.sh | 5 ++++- .../node_csot.test.ts | 14 ++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/.evergreen/run-resource-management-feature-integration.sh b/.evergreen/run-resource-management-feature-integration.sh index 093a4749d78..71756d96141 100644 --- a/.evergreen/run-resource-management-feature-integration.sh +++ b/.evergreen/run-resource-management-feature-integration.sh @@ -1,6 +1,9 @@ #! /bin/bash -source $DRIVERS_TOOLS/.evergreen/init-node-and-npm-env.sh +# source $DRIVERgit addS_TOOLS/.evergreen/init-node-and-npm-env.sh + +echo "node: $(node --version)" +echo "npm: $(npm --version)" echo "Building driver..." npm pack diff --git a/test/integration/client-side-operations-timeout/node_csot.test.ts b/test/integration/client-side-operations-timeout/node_csot.test.ts index 4ab28e0bb31..12b380d8f1a 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -649,7 +649,7 @@ describe('CSOT driver tests', metadata, () => { client = this.configuration.newClient(undefined, { monitorCommands: true, minPoolSize }); commandStarted = []; client.on('commandStarted', ev => commandStarted.push(ev)); - await client.connect(); + await waitUntilPoolsFilled(client, AbortSignal.timeout(30_000), minPoolSize); }); afterEach(async function () { @@ -690,11 +690,13 @@ describe('CSOT driver tests', metadata, () => { .db('db') .collection('coll') .find({}, { timeoutMS: 150, tailable: true, awaitData: true, batchSize: 1 }); - for (let i = 0; i < 5; i++) { - // Iterate cursor 5 times (server would have blocked for 500ms overall, but client - // should not throw - await cursor.next(); - } + // Iterate cursor 5 times (server would have blocked for 500ms overall, but client + // should not throw + await cursor.next(); + await cursor.next(); + await cursor.next(); + await cursor.next(); + await cursor.next(); }); it('does not use timeoutMS to compute maxTimeMS for getMores', metadata, async function () {