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.

Sign up for GitHub

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

Jump to bottom

refactor(NODE-6057): implement CursorResponse for lazy document parsing #4085

Merged

baileympearson merged 25 commits into main from chore-cursor-v2-final

May 1, 2024

Contributor

nbbeeken commented Apr 17, 2024 •

edited

Description

What is changing?

Implement a CursorResponse subclass of MongoDBResponse
- Implements logic to abstract List vs CursorResponse handling
Make FindCursors use CursorResponse

Is there new documentation needed for these changes?

TBD. Is this something we want to apply to all cursors? Should we progressively migrate to this?

What is the motivation for this change?

An attempt at increasing performance. A sucessful decoupling of driver BSON and user BSON settings.

Only for FindCursor, BSON documents from the current batch are now parsed on each iteration.

One clear improvement is that any cursor that is closed before exhaustion will take significantly less run time.

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

nbbeeken force-pushed the chore-cursor-v2-final branch 2 times, most recently from 98a105b to a537db1 Compare

April 18, 2024 19:42

nbbeeken marked this pull request as ready for review

April 19, 2024 18:01

nbbeeken commented

View reviewed changes

src/client-side-encryption/auto_encrypter.ts Outdated Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cmap/wire_protocol/responses.ts Outdated Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cursor/abstract_cursor.ts Outdated Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cursor/find_cursor.ts Show resolved Hide resolved

nbbeeken commented

View reviewed changes

test/unit/assorted/collations.test.js Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cmap/wire_protocol/responses.ts Outdated Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cmap/wire_protocol/responses.ts Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cursor/abstract_cursor.ts Show resolved Hide resolved

nbbeeken commented

View reviewed changes

src/cmap/wire_protocol/responses.ts Show resolved Hide resolved

baileympearson self-assigned this

baileympearson added the Primary Review label

baileympearson reviewed

View reviewed changes

src/cmap/wire_protocol/responses.ts Outdated Show resolved Hide resolved

src/cursor/abstract_cursor.ts Outdated Show resolved Hide resolved

src/cmap/wire_protocol/responses.ts Show resolved Hide resolved

src/cmap/wire_protocol/responses.ts Show resolved Hide resolved

src/cmap/wire_protocol/responses.ts Outdated Show resolved Hide resolved

src/cursor/find_cursor.ts Show resolved Hide resolved

test/unit/assorted/collations.test.js Show resolved Hide resolved

src/client-side-encryption/auto_encrypter.ts Outdated Show resolved Hide resolved

addaleax reviewed

View reviewed changes

src/client-side-encryption/auto_encrypter.ts Outdated Show resolved Hide resolved

src/client-side-encryption/state_machine.ts Outdated Show resolved Hide resolved

src/cmap/wire_protocol/responses.ts Outdated Show resolved Hide resolved

nbbeeken requested review from addaleax and baileympearson

April 23, 2024 19:02

nbbeeken added 12 commits

April 25, 2024 13:22


          refactor(NODE-6057): implement CursorResponse for lazy document parsing

c5b8d78


          chore: remove the need for unshift

0e22782


          chore: lint

3144b11


          chore: fix FLE, may revert.

78f9068


          chore: fix ts

f42584c


          fix: old servers

8bd2a47


          fix: return consistent type

68077e9


          fix: unit

5fb54d3


          perf: define methods on cursor response

df5121a


          refactor: replace isBuffer check with isUint8Array

053f01f


          fix: add correct ts to execute

7edb948


          docs: add comment for base64 string

4631e34

nbbeeken added 8 commits

April 25, 2024 13:22


          feat: remove valuesAs just use indexing

ac5118e


          feat: required fields by moving error check out of constructor

9f80b5b


          fix: ns is not a required field


          fix: rename hasNext

ea365c1


          fix: shift: false wrong for stream, stream uses same settings as `cur…

24b7678

…sor.next`


          fix: stream cannot use cursor's next because it reaches exhaustion er…

195bd62

…ror in changestream prose test


          test: add unit tests for cursor response ctor

2f10f0e


          fix: batch always exists and emptyGetMore made simpler

ca88168

nbbeeken force-pushed the chore-cursor-v2-final branch from 70df234 to ca88168 Compare

April 25, 2024 17:22

nbbeeken added 3 commits

April 25, 2024 17:32


          test: demonstrate internal decorate feature breakage


          fix: revert changes to FLE and exempt from using responseType

3ee316d


          test: fixes

c0b0a86

baileympearson requested changes

View reviewed changes

src/client-side-encryption/auto_encrypter.ts Outdated Show resolved Hide resolved

test/integration/client-side-encryption/driver.test.ts Show resolved Hide resolved

test/integration/client-side-encryption/driver.test.ts Outdated Show resolved Hide resolved

src/operations/find.ts Outdated Show resolved Hide resolved

nbbeeken added 2 commits

April 29, 2024 13:22


          test: lower server version for decorateDecryptionResult test

26be067


          fix: remove encryption flag from find

0d801a6

nbbeeken requested a review from baileympearson

April 29, 2024 18:32

baileympearson approved these changes

View reviewed changes

baileympearson approved these changes

View reviewed changes

baileympearson added Team Review and removed Primary Review labels

baileympearson merged commit 9d73f45 into main

21 of 26 checks passed

baileympearson deleted the chore-cursor-v2-final branch

May 1, 2024 20:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment