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-6304): Add CSOT support for non-tailable cursors #4195

Merged
merged 53 commits into from
Sep 12, 2024

Conversation

W-A-James
Copy link
Contributor

@W-A-James W-A-James commented Aug 14, 2024

Description

What is changing?

Operations (direct user-facing changes)
  • Added support for timeoutMS and timeoutMode to all cursor-creating Collection and Db methods.
Cursors
AbstractCursor
  • Added support for timeoutMS and timeoutMode options to all AbstractCursor
  • Created CursorInitializeOptions type and updated AbstractCursor._initialize to take this type as its options. Updated overrides of this type to match this signature and utilize the new options.
  • Added protected timeoutContext property to AbstractCursor that is constructed inside AbstractCursor.cursorInit()
  • Added CSOT timeout logic to AbstractCursor.next(), AbstractCursor.tryNext() and AbstractCursor.hasNext()
  • Added optional timeoutMS argument to AbstractCursor.close and added CSOT timeout logic to AbstractCursor.cleanup
  • Added optional timeoutMS argument to AbstractCursor.cleanup
  • Added logic to omit maxTimeMS field on getMore when CSOT is enabled
AggregationCursor
  • Added logic to throw if $out or $merge stages are provided to an AggregationCursor with timeoutMode: ITERATION
ChangeStreamCursor (note, this implementation is not complete. Will be finished in NODE-6305)
  • Updated _initialize to make use of aformentioned CursorInitializeOptions
FindCursor
  • Updated _initialize to make use of aformentioned CursorInitializeOptions
ListCollectionsCursor
  • Updated _initialize to make use of aformentioned CursorInitializeOptions
ListIndexesCursor
  • Updated _initialize to make use of aformentioned CursorInitializeOptions
RunCommandCursor
  • Updated _initialize to make use of aformentioned CursorInitializeOptions
Timeout and TimeoutContext
  • Added TimeoutContext.refresh() method to reset active timeouts for cursor iteration and reuse a TimeoutContext
  • Added TimeoutContext.clear() method to clear active timeouts associated with a TimeoutContext
Testing
  • Implemented and skipped CSOT Prose Test 5 (will be unskipped in NODE-6305)
  • Synced CSOT spec tests to be up-to-date with latest tests
  • Unskipped CSOT non-tailable cursor tests, as well as other CSOT tests (see changes for details)
  • Updated CSOT spec test skip logic to keep better track of which tests are skipped and where they will be addressed
  • Added integration tests to ensure that ITERATION and CURSOR_LIFETIME modes work correctly for non-tailable cursors
  • Updated UTR to ensure timeoutMode and timeoutMS options are passed through for runCursorCommand
  • Added unit test for AggregationCursor to ensure that $out and $merge helpers throw if in ITERATION mode
Is there new documentation needed for these changes?

What is the motivation for this change?

Release Highlight

Fill in title or leave empty for no highlight

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

src/collection.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Sep 5, 2024
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/cursor/abstract_cursor.ts Show resolved Hide resolved
src/cursor/abstract_cursor.ts Outdated Show resolved Hide resolved
src/cursor/abstract_cursor.ts Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/timeout.ts Show resolved Hide resolved
src/cursor/abstract_cursor.ts Show resolved Hide resolved
src/cursor/aggregation_cursor.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken merged commit a4d15ab into NODE-6090 Sep 12, 2024
25 of 27 checks passed
@nbbeeken nbbeeken deleted the NODE-6304 branch September 12, 2024 15:35
@W-A-James W-A-James restored the NODE-6304 branch September 16, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants