Skip to content

Commit

Permalink
chore: fix a few flaky CSOT tests (#4278)
Browse files Browse the repository at this point in the history
  • Loading branch information
baileympearson authored and nbbeeken committed Nov 1, 2024
1 parent 51d55c0 commit 6d3a864
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 42 deletions.
5 changes: 4 additions & 1 deletion .evergreen/run-resource-management-feature-integration.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
47 changes: 27 additions & 20 deletions test/integration/client-side-operations-timeout/node_csot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,15 +361,15 @@ 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',
mode: 'alwaysOn',
data: {
failCommands: ['find', 'getMore'],
blockConnection: true,
blockTimeMS: 50
blockTimeMS: 150
}
};

Expand Down Expand Up @@ -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
Expand All @@ -457,20 +457,25 @@ 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
);
})
).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');
}
);
});
Expand Down Expand Up @@ -644,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 () {
Expand Down Expand Up @@ -685,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 () {
Expand Down
4 changes: 3 additions & 1 deletion test/integration/crud/client_bulk_write.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,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
Expand Down
54 changes: 39 additions & 15 deletions test/integration/node-specific/abstract_cursor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { inspect } from 'util';
import {
AbstractCursor,
type Collection,
type CommandStartedEvent,
CursorTimeoutContext,
CursorTimeoutMode,
type FindCursor,
Expand All @@ -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 () {
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -473,39 +477,59 @@ 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({
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: {
failCommands: ['getMore'],
blockConnection: true,
blockTimeMS: 5000
}
} as FailPoint);
},
uri
);
});

afterEach(async function () {
await clearFailPoint(this.configuration, uri);
});

it(
'the provided timeoutContext is not modified',
{
requires: {
mongodb: '>=4.4',
topology: '!load-balanced'
}
},
async function () {
const cursor = collection.find(
{},
{
timeoutContext: context,
timeoutMS: 1000,
timeoutMS: 150,
timeoutMode: CursorTimeoutMode.LIFETIME,
batchSize: 1
}
);

const refresh = sinon.spy(context, 'refresh');
const refreshed = sinon.spy(context, 'refreshed');
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);
expect(refresh.called).to.be.false;
expect(refreshed.called).to.be.true;
}
);
});
Expand Down
12 changes: 8 additions & 4 deletions test/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 6d3a864

Please sign in to comment.