fix(NODE-3648): run get more ops through server selection #3030
Conversation
durran
force-pushed
the
NODE-3648
branch
2 times, most recently
from
66941be
to
a95b0fd
Compare
durran
added
Primary Review
There was a problem hiding this comment.
Some initial thoughts. This is very close to ready as is, but there's another component to fixing the original bug that we could consider to include here. Only in this version of the driver have we now dropped support for versions of MongoDB that don't support sessions. Part of the issue was that our hasSessionSupport logic was omitting the session because the monitors downgraded it. Now that we run getMores through serverSelection we shouldn't encounter this issue.
But should additionally remove the checks altogether? and in fact maybe we should consider throwing Connection#getMore if we don't have a session at that point. (possibly too far) but IIUC of all operations getMores 100% need to have a session defined. (with our currently supported server vers)
Co-authored-by: Neal Beeken <[email protected]>
nbbeeken
added
Team Review
| describe('GetMoreOperation', function () { | ||
| const ns = 'db.coll'; | ||
| const cursorId = Long.fromNumber(1); | ||
| const options = { |
There was a problem hiding this comment.
can we freeze these so that we can make sure the options aren't mutated? (otherwise that deep equal check below doesn't guarantee as much)
There was a problem hiding this comment.
Now all params are frozen.
Co-authored-by: Daria Pardue <[email protected]>
Description
Runs
getmorecommand operations through server selection, selecting the same server.What is changing?
A new server selector has been added,
sameServerSelector, which takes aServerDescriptionand returns aServerDescription[]. This will always select the same server as the description provided, unless the server has transitioned into an unknown state, where it will return an empty array. In this case, server selection will force monitor checks to potentially update the server and select the same one again.A new
GetMoreOperationhas been added in order for the cursors ongetMoreto execute this operation with server selection. The pinned server's description on the operation will be used in conjunction with the new server selector.Is there new documentation needed for these changes?
No
What is the motivation for this change?
NODE-3648
NODE-3453
HELP-28682
Double check the following
npm run check:lintscript<type>(NODE-xxxx)<!>: <description>