feat(NODE-5682): set maxTimeMS on commands and preempt I/O #4174

nbbeeken · 2024-07-11T15:45:41Z

Description

What is changing?

add timeoutMS option to onData helper. Makes the iterator throw the same way an error event would.
Add maxTimeMS field to commands when CSOT is enabled
If minRoundTrip defined, add it to the timeoutContext
listDatabases and runCommand respect timeoutMS settings

Is there new documentation needed for these changes?

No

What is the motivation for this change?

Part of the goal of CSOT. This enables the actual interruption of operations, and hints to the server (through maxTimeMS) how long it should permit an operation to run for.

Double check the following

Ran npm run check:lint script
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket

…ver selection

#4157)

src/cmap/connection.ts

baileympearson · 2024-07-24T14:38:46Z

src/cmap/connection.ts

@@ -416,6 +417,11 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
      ...options
    };

+    if (options.timeoutContext?.csotEnabled()) {
+      const { maxTimeMS } = options.timeoutContext;
+      if (maxTimeMS > 0 && Number.isFinite(maxTimeMS)) cmd.maxTimeMS = maxTimeMS;


I'm confused why this logic is necessary - if the remaining timeoutMS is less than minRoundTripTime, we're supposed to throw and exit. So maxTimeMS should never be negative here - right?

Correct, but this is a check for greater than 0, so it confirms that maxTimeMS does not equal 0. The checks for if we should throw are colocated with the write/read steps.

src/cmap/connection.ts

src/sdam/server.ts

src/sdam/topology.ts

src/timeout.ts

test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts

src/cmap/connection.ts

src/timeout.ts

Co-authored-by: Warren James <[email protected]>

nbbeeken changed the title ~~NODE-5682-maxTimeMS~~ WIP feat(NODE-5682): set maxTimeMS Jul 11, 2024

nbbeeken requested a review from W-A-James July 11, 2024 15:46

nbbeeken force-pushed the NODE-5682-maxTimeMS branch from dca1a9c to 723e160 Compare July 11, 2024 18:55

W-A-James removed their request for review July 19, 2024 18:02

nbbeeken force-pushed the NODE-6090 branch from 58d4e7b to 5759f72 Compare July 22, 2024 15:17

W-A-James and others added 16 commits July 22, 2024 11:20

feat(NODE-6090): Implement CSOT logic for connection checkout and ser…

a58ce34

…ver selection

test(NODE-6120): Implement Unified test runner changes for CSOT (#4121)

7b4a1fb

feat(NODE-6090): Implement CSOT logic for connection checkout and ser…

3f4313e

…ver selection

refactor(NODE-6187): refactor to use TimeoutContext abstraction (#4131)

4aa6575

refactor(NODE-6230): executeOperation to use iterative retry mechanism (

898b93f

#4157)

WIP maxTimeMS

6125789

chore: rm stack trace

749ad6d

pass timeoutMS to runCommand

e7dec00

wrap errors correctly

8d83bf5

throw MongoOperationTimeoutError instead of TimeoutError

74ae021

pass timeoutContext through to Connection.command

59eb1b1

remove unused field

ad90766

don't ejsonify logs

6f9925c

pass timeoutMS through to listDatabases

a432320

Remove Error.captureStackTrace calls

f897155

unskip tests

edf63e4

nbbeeken force-pushed the NODE-5682-maxTimeMS branch from de93db9 to 4c04b2a Compare July 22, 2024 15:22

fix: connect ignores timeoutMS setting from client

7c79517

nbbeeken force-pushed the NODE-5682-maxTimeMS branch from 4c04b2a to 7c79517 Compare July 23, 2024 17:59

nbbeeken added 3 commits July 23, 2024 14:14

chore: unset timeoutMS not 0

2fb3226

test: skip more connect tests

9ee5112

chore: add ticket

f5f9bc9

nbbeeken changed the title ~~WIP feat(NODE-5682): set maxTimeMS~~ feat(NODE-5682): set maxTimeMS on commands and preempt I/O Jul 23, 2024

nbbeeken marked this pull request as ready for review July 23, 2024 18:54

baileympearson self-assigned this Jul 24, 2024

baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jul 24, 2024

baileympearson requested changes Jul 24, 2024

View reviewed changes

nbbeeken added 2 commits July 24, 2024 12:01

comments 1

2fdeab8

chore: move socket timeouts into method

520fc51

nbbeeken requested a review from baileympearson July 24, 2024 17:29

baileympearson requested changes Jul 24, 2024

View reviewed changes

src/cmap/connection.ts Show resolved Hide resolved

src/timeout.ts Outdated Show resolved Hide resolved

chore: move start

89ddaae

nbbeeken requested a review from baileympearson July 25, 2024 17:57

chore: linttttttttttttttttttttttttttttttttttttttttttttttt

5e975d7

baileympearson merged commit 7baefd1 into NODE-6090 Jul 26, 2024
26 checks passed

baileympearson deleted the NODE-5682-maxTimeMS branch July 26, 2024 13:55

nbbeeken added a commit that referenced this pull request Aug 13, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

8374a5c

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Aug 20, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

aa96cb0

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Aug 21, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

8599cb3

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Sep 12, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

22082c9

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Sep 25, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

acfb4fc

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Sep 27, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

4999789

Co-authored-by: Warren James <[email protected]>

baileympearson pushed a commit that referenced this pull request Oct 1, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

8d1a2d3

Co-authored-by: Warren James <[email protected]>

baileympearson pushed a commit that referenced this pull request Oct 1, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

df025f4

Co-authored-by: Warren James <[email protected]>

W-A-James added a commit that referenced this pull request Oct 1, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

867d35e

Co-authored-by: Warren James <[email protected]>

W-A-James added a commit that referenced this pull request Oct 1, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

72ccc6a

Co-authored-by: Warren James <[email protected]>

baileympearson pushed a commit that referenced this pull request Oct 10, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

f3e190f

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Oct 14, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

c333723

Co-authored-by: Warren James <[email protected]>

baileympearson pushed a commit that referenced this pull request Oct 21, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

6ed89cc

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Nov 1, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

10b02ba

Co-authored-by: Warren James <[email protected]>

nbbeeken added a commit that referenced this pull request Nov 5, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

3c8e63e

Co-authored-by: Warren James <[email protected]>

dariakp pushed a commit that referenced this pull request Nov 6, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O (#4174)

e4e6a5e

Co-authored-by: Warren James <[email protected]>

github-actions bot mentioned this pull request Nov 6, 2024

chore(main): release 6.11.0 [skip-ci] #4317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(NODE-5682): set maxTimeMS on commands and preempt I/O #4174

feat(NODE-5682): set maxTimeMS on commands and preempt I/O #4174

nbbeeken commented Jul 11, 2024 •

edited

Loading

baileympearson Jul 24, 2024

nbbeeken Jul 24, 2024

feat(NODE-5682): set maxTimeMS on commands and preempt I/O #4174

feat(NODE-5682): set maxTimeMS on commands and preempt I/O #4174

Conversation

nbbeeken commented Jul 11, 2024 • edited Loading

Description

What is changing?

Is there new documentation needed for these changes?

What is the motivation for this change?

Double check the following

baileympearson Jul 24, 2024

Choose a reason for hiding this comment

nbbeeken Jul 24, 2024

Choose a reason for hiding this comment

nbbeeken commented Jul 11, 2024 •

edited

Loading