Skip to content

Commit

Permalink
feat: perform selection before cursor operation execution if needed
Browse files Browse the repository at this point in the history
This is the counterpart to the previous commit which is temporarily
required because cursors do not use `executeOperation`. This fix
should no longer be required in a very short time.
  • Loading branch information
mbroadst authored and daprahamian committed Aug 13, 2019
1 parent 1a25876 commit 808cf37
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
15 changes: 15 additions & 0 deletions lib/core/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const MongoNetworkError = require('./error').MongoNetworkError;
const mongoErrorContextSymbol = require('./error').mongoErrorContextSymbol;
const f = require('util').format;
const collationNotSupported = require('./utils').collationNotSupported;
const ReadPreference = require('./topologies/read_preference');
const BSON = retrieveBSON();
const Long = BSON.Long;

Expand Down Expand Up @@ -584,6 +585,20 @@ var nextFunction = function(self, callback) {
Cursor.prototype._initializeCursor = function(callback) {
const cursor = this;

// NOTE: this goes away once cursors use `executeOperation`
if (cursor.topology.shouldCheckForSessionSupport()) {
cursor.topology.selectServer(ReadPreference.primaryPreferred, err => {
if (err) {
callback(err);
return;
}

cursor.next(callback);
});

return;
}

// Very explicitly choose what is passed to selectServer
const serverSelectOptions = {};
if (cursor.cursorState.session) {
Expand Down
11 changes: 11 additions & 0 deletions lib/core/sdam/topology.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,17 @@ class Topology extends EventEmitter {
}

// Sessions related methods

/**
* @return Whether the topology should initiate selection to determine session support
*/
shouldCheckForSessionSupport() {
return (
(this.description.type === TopologyType.Single && !this.description.hasKnownServers) ||
!this.description.hasDataBearingServers
);
}

/**
* @return Whether sessions are supported on the current topology
*/
Expand Down
10 changes: 1 addition & 9 deletions lib/operations/execute_operation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const OperationBase = require('./operation').OperationBase;
const ReadPreference = require('../core').ReadPreference;
const isRetryableError = require('../core/error').isRetryableError;
const maxWireVersion = require('../core/utils').maxWireVersion;
const TopologyType = require('../core/sdam/topology_description').TopologyType;

/**
* Executes the given operation with provided arguments.
Expand All @@ -33,7 +32,7 @@ function executeOperation(topology, operation, callback) {
if (
topology.description != null &&
!operation.hasAspect(Aspect.SKIP_SESSION) &&
shouldCheckForSessionSupport(topology)
topology.shouldCheckForSessionSupport()
) {
// TODO: this is only supported for unified topology, the first part of this check
// should go away when we drop legacy topology types.
Expand Down Expand Up @@ -162,11 +161,4 @@ function executeWithServerSelection(topology, operation, callback) {
});
}

function shouldCheckForSessionSupport(topology) {
return (
(topology.description.type === TopologyType.Single && !topology.description.hasKnownServers) ||
!topology.description.hasDataBearingServers
);
}

module.exports = executeOperation;

0 comments on commit 808cf37

Please sign in to comment.