Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(NODE-6389): add support for timeoutMS in StateMachine.execute() #4243

Merged
merged 44 commits into from
Oct 7, 2024
Merged
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
3c2ec0a
feat(NODE-6090): Implement CSOT logic for connection checkout and ser…
W-A-James Apr 11, 2024
909578f
test(NODE-6120): Implement Unified test runner changes for CSOT (#4121)
W-A-James Jun 10, 2024
e101750
refactor(NODE-6187): refactor to use TimeoutContext abstraction (#4131)
W-A-James Jun 21, 2024
e4efd3f
refactor(NODE-6230): executeOperation to use iterative retry mechanis…
nbbeeken Jul 22, 2024
22082c9
feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)
nbbeeken Jul 26, 2024
bf95fa4
feat(NODE-6231): Add CSOT behaviour for retryable reads and writes (#…
W-A-James Aug 1, 2024
c63d102
feat(NODE-6312): add error transformation for server timeouts (#4192)
nbbeeken Aug 12, 2024
1eab23d
feat(NODE-6313): add CSOT support to sessions and transactions (#4199)
nbbeeken Sep 9, 2024
4c4b0a9
feat(NODE-6304): add CSOT support for non-tailable cursors (#4195)
W-A-James Sep 12, 2024
558d416
fix(NODE-6374): MongoOperationTimeoutError inherits MongoRuntimeError…
nbbeeken Sep 12, 2024
3ed4a14
test: remove empty skipped context blocks (#4238)
W-A-James Sep 12, 2024
d3438ea
feat(NODE-5844): add iscryptd to ServerDescription (#4239)
nbbeeken Sep 17, 2024
ff561e3
temp
aditi-khare-mongoDB Sep 19, 2024
164780c
temp
aditi-khare-mongoDB Sep 20, 2024
12a7e2e
chore: plumb timeoutMS around more
nbbeeken Sep 24, 2024
999f23d
feat(NODE-6090): Implement CSOT logic for connection checkout and ser…
W-A-James Apr 11, 2024
0355404
test(NODE-6120): Implement Unified test runner changes for CSOT (#4121)
W-A-James Jun 10, 2024
5ef3d69
refactor(NODE-6187): refactor to use TimeoutContext abstraction (#4131)
W-A-James Jun 21, 2024
7139b8f
refactor(NODE-6230): executeOperation to use iterative retry mechanis…
nbbeeken Jul 22, 2024
acfb4fc
feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)
nbbeeken Jul 26, 2024
4efff95
feat(NODE-6231): Add CSOT behaviour for retryable reads and writes (#…
W-A-James Aug 1, 2024
1997f81
feat(NODE-6312): add error transformation for server timeouts (#4192)
nbbeeken Aug 12, 2024
cc3ef8f
feat(NODE-6313): add CSOT support to sessions and transactions (#4199)
nbbeeken Sep 9, 2024
38affae
feat(NODE-6304): add CSOT support for non-tailable cursors (#4195)
W-A-James Sep 12, 2024
738188b
fix(NODE-6374): MongoOperationTimeoutError inherits MongoRuntimeError…
nbbeeken Sep 12, 2024
c4a7c2c
test: remove empty skipped context blocks (#4238)
W-A-James Sep 12, 2024
5aa6d4c
feat(NODE-5844): add iscryptd to ServerDescription (#4239)
nbbeeken Sep 17, 2024
17a2fde
chore: allow clientBulkWrite to use TimeoutContext (#4251)
W-A-James Sep 25, 2024
aead2f1
Merge branch 'NODE-6090' into NODE-6389
aditi-khare-mongoDB Sep 25, 2024
88ca990
half testing
aditi-khare-mongoDB Sep 26, 2024
2e3a84c
revert state machine test changes
aditi-khare-mongoDB Sep 30, 2024
e6e9fb4
requested changes
aditi-khare-mongoDB Oct 1, 2024
3dc383b
Merge branch 'NODE-6090' into NODE-6389
aditi-khare-mongoDB Oct 1, 2024
702a03e
lint fix
aditi-khare-mongoDB Oct 1, 2024
3b6a23b
test fix
aditi-khare-mongoDB Oct 1, 2024
5560a1b
no negative timeouts
aditi-khare-mongoDB Oct 2, 2024
601c159
requested changes
aditi-khare-mongoDB Oct 2, 2024
096f154
fix failing tests
aditi-khare-mongoDB Oct 3, 2024
5aba790
requested changes 3
aditi-khare-mongoDB Oct 3, 2024
903e0d0
limit flaky tests
aditi-khare-mongoDB Oct 3, 2024
709f725
Merge branch 'NODE-6090' into NODE-6389
aditi-khare-mongoDB Oct 4, 2024
cb12f64
lint fix
aditi-khare-mongoDB Oct 7, 2024
01aca89
Merge branch 'NODE-6090' into NODE-6389
baileympearson Oct 7, 2024
6ea56d3
Merge branch 'NODE-6090' into NODE-6389
baileympearson Oct 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(NODE-6231): Add CSOT behaviour for retryable reads and writes (#…
W-A-James authored and nbbeeken committed Sep 25, 2024

Verified

This commit was signed with the committer’s verified signature.
nbbeeken Neal Beeken
commit 4efff95018c4f0f08c3010c18975179b1889ba1e
9 changes: 5 additions & 4 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
@@ -227,12 +227,10 @@ async function tryOperation<
session.incrementTransactionNumber();
}

// TODO(NODE-6231): implement infinite retry within CSOT timeout here
const maxTries = willRetry ? 2 : 1;
const maxTries = willRetry ? (timeoutContext.csotEnabled() ? Infinity : 2) : 1;
let previousOperationError: MongoError | undefined;
let previousServer: ServerDescription | undefined;

// TODO(NODE-6231): implement infinite retry within CSOT timeout here
for (let tries = 0; tries < maxTries; tries++) {
if (previousOperationError) {
if (hasWriteAspect && previousOperationError.code === MMAPv1_RETRY_WRITES_ERROR_CODE) {
@@ -276,7 +274,6 @@ async function tryOperation<
return await operation.execute(server, session, timeoutContext);
} catch (operationError) {
if (!(operationError instanceof MongoError)) throw operationError;

if (
previousOperationError != null &&
operationError.hasErrorLabel(MongoErrorLabel.NoWritesPerformed)
@@ -285,6 +282,10 @@ async function tryOperation<
}
previousServer = server.description;
previousOperationError = operationError;

// Reset timeouts
timeoutContext.serverSelectionTimeout?.clear();
timeoutContext.connectionCheckoutTimeout?.clear();
}
}

26 changes: 17 additions & 9 deletions src/timeout.ts
Original file line number Diff line number Diff line change
@@ -39,6 +39,7 @@ export class Timeout extends Promise<never> {
public ended: number | null = null;
public duration: number;
public timedOut = false;
public cleared = false;

get remainingTime(): number {
if (this.timedOut) return 0;
@@ -53,7 +54,6 @@ export class Timeout extends Promise<never> {
/** Create a new timeout that expires in `duration` ms */
private constructor(executor: Executor = () => null, duration: number, unref = true) {
let reject!: Reject;

if (duration < 0) {
throw new MongoInvalidArgumentError('Cannot create a Timeout with a negative duration');
}
@@ -86,6 +86,7 @@ export class Timeout extends Promise<never> {
clear(): void {
clearTimeout(this.id);
this.id = undefined;
this.cleared = true;
}

throwIfExpired(): void {
@@ -213,16 +214,20 @@ export class CSOTTimeoutContext extends TimeoutContext {

get serverSelectionTimeout(): Timeout | null {
// check for undefined
if (typeof this._serverSelectionTimeout !== 'object') {
if (typeof this._serverSelectionTimeout !== 'object' || this._serverSelectionTimeout?.cleared) {
const { remainingTimeMS, serverSelectionTimeoutMS } = this;
if (remainingTimeMS <= 0)
throw new MongoOperationTimeoutError(
`Timed out in server selection after ${this.timeoutMS}ms`
);
const usingServerSelectionTimeoutMS =
this.serverSelectionTimeoutMS !== 0 &&
csotMin(this.timeoutMS, this.serverSelectionTimeoutMS) === this.serverSelectionTimeoutMS;

serverSelectionTimeoutMS !== 0 &&
csotMin(remainingTimeMS, serverSelectionTimeoutMS) === serverSelectionTimeoutMS;
if (usingServerSelectionTimeoutMS) {
this._serverSelectionTimeout = Timeout.expires(this.serverSelectionTimeoutMS);
this._serverSelectionTimeout = Timeout.expires(serverSelectionTimeoutMS);
} else {
if (this.timeoutMS > 0) {
this._serverSelectionTimeout = Timeout.expires(this.timeoutMS);
if (remainingTimeMS > 0 && Number.isFinite(remainingTimeMS)) {
this._serverSelectionTimeout = Timeout.expires(remainingTimeMS);
} else {
this._serverSelectionTimeout = null;
}
@@ -233,7 +238,10 @@ export class CSOTTimeoutContext extends TimeoutContext {
}

get connectionCheckoutTimeout(): Timeout | null {
if (typeof this._connectionCheckoutTimeout !== 'object') {
if (
typeof this._connectionCheckoutTimeout !== 'object' ||
this._connectionCheckoutTimeout?.cleared
) {
if (typeof this._serverSelectionTimeout === 'object') {
// null or Timeout
this._connectionCheckoutTimeout = this._serverSelectionTimeout;
Original file line number Diff line number Diff line change
@@ -6,7 +6,9 @@ import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
const enabled = [
'override-collection-timeoutMS',
'override-database-timeoutMS',
'override-operation-timeoutMS'
'override-operation-timeoutMS',
'retryability-legacy-timeouts',
'retryability-timeoutMS'
];

const cursorOperations = [
@@ -18,6 +20,11 @@ const cursorOperations = [
'listCollectionNames'
];

const bulkWriteOperations = [
'timeoutMS applies to whole operation, not individual attempts - bulkWrite on collection',
'timeoutMS applies to whole operation, not individual attempts - insertMany on collection'
];

describe('CSOT spec tests', function () {
const specs = loadSpecTests(join('client-side-operations-timeout'));
for (const spec of specs) {
@@ -30,6 +37,10 @@ describe('CSOT spec tests', function () {
// Cursor operation
if (test.operations.find(operation => cursorOperations.includes(operation.name)))
test.skipReason = 'TODO(NODE-5684): Not working yet';

if (bulkWriteOperations.includes(test.description))
test.skipReason =
'TODO(NODE-6274): update test runner to check errorResponse field of MongoBulkWriteError in isTimeoutError assertion';
}
}
runUnifiedSuite(specs);
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@
import { expect } from 'chai';
import * as sinon from 'sinon';

import { ConnectionPool, type MongoClient, Timeout, Topology } from '../../mongodb';
import { ConnectionPool, type MongoClient, Timeout, TimeoutContext, Topology } from '../../mongodb';

// TODO(NODE-5824): Implement CSOT prose tests
describe('CSOT spec unit tests', function () {
@@ -22,10 +22,16 @@ describe('CSOT spec unit tests', function () {
it('Operations should ignore waitQueueTimeoutMS if timeoutMS is also set.', async function () {
client = this.configuration.newClient({ waitQueueTimeoutMS: 999999, timeoutMS: 10000 });
sinon.spy(Timeout, 'expires');
const timeoutContextSpy = sinon.spy(TimeoutContext, 'create');

await client.db('db').collection('collection').insertOne({ x: 1 });

expect(Timeout.expires).to.have.been.calledWith(10000);
const createCalls = timeoutContextSpy.getCalls().filter(
// @ts-expect-error accessing concrete field
call => call.args[0].timeoutMS === 10000
);

expect(createCalls).to.have.length.greaterThanOrEqual(1);
expect(Timeout.expires).to.not.have.been.calledWith(999999);
});

Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* Anything javascript specific relating to timeouts */
import { expect } from 'chai';
import * as sinon from 'sinon';

import {
type ClientSession,
@@ -13,10 +12,6 @@ import {
} from '../../mongodb';

describe('CSOT driver tests', () => {
afterEach(() => {
sinon.restore();
});

describe('timeoutMS inheritance', () => {
let client: MongoClient;
let db: Db;
2 changes: 2 additions & 0 deletions test/tools/unified-spec-runner/match.ts
Original file line number Diff line number Diff line change
@@ -789,6 +789,8 @@ export function expectErrorCheck(
expect(error).to.be.instanceof(MongoOperationTimeoutError);
}

// TODO(NODE-6274): Check for MongoBulkWriteErrors that have a MongoOperationTimeoutError in their
// errorResponse field
if (expected.isTimeoutError === false) {
expect(error).to.not.be.instanceof(MongoOperationTimeoutError);
} else if (expected.isTimeoutError === true) {