Skip to content

Commit

Permalink
fix(pool): support a drain event for use with unified topology
Browse files Browse the repository at this point in the history
Until we integrate the new connection pool conforming to the CMAP
specification, any type of error/close/timeout coming from a pool
signals pool invalidation. If the pool is configured for unified
then it should invalidate the server by emitting a `drain` event
which is translated to an `error in the Server instance, causing
the topology to cycle the server.

NODE-2332
  • Loading branch information
mbroadst committed Nov 25, 2019
1 parent c24eb91 commit 3471c28
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 2 deletions.
9 changes: 9 additions & 0 deletions lib/core/connection/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,15 @@ function connectionFailureHandler(pool, event, err, conn) {
const workItem = conn.workItems.shift();
if (workItem.cb) workItem.cb(err);
}

if (pool.state !== DRAINING && pool.options.legacyCompatMode === false) {
// since an error/close/timeout means pool invalidation in a
// pre-CMAP world, we will issue a custom `drain` event here to
// signal that the server should be recycled
stateTransition(pool, DRAINING);
pool.emit('drain', err);
return;
}
}

// Did we catch a timeout, increment the numberOfConsecutiveTimeouts
Expand Down
4 changes: 3 additions & 1 deletion lib/core/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,9 @@ function isNodeShuttingDownError(err) {
* @param {Server} server
*/
function isSDAMUnrecoverableError(error, server) {
if (error instanceof MongoParseError) {
// NOTE: null check is here for a strictly pre-CMAP world, a timeout or
// close event are considered unrecoverable
if (error instanceof MongoParseError || error == null) {
return true;
}

Expand Down
4 changes: 4 additions & 0 deletions lib/core/sdam/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ class Server extends EventEmitter {
// setup listeners
this.s.pool.on('parseError', parseErrorEventHandler(this));

this.s.pool.on('drain', err => {
this.emit('error', err);
});

// it is unclear whether consumers should even know about these events
// this.s.pool.on('timeout', timeoutEventHandler(this));
// this.s.pool.on('reconnect', reconnectEventHandler(this));
Expand Down
2 changes: 1 addition & 1 deletion test/functional/spec-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ function parseSessionOptions(options) {
return result;
}

const IGNORED_COMMANDS = new Set(['ismaster', 'configureFailPoint']);
const IGNORED_COMMANDS = new Set(['ismaster', 'configureFailPoint', 'endSessions']);

let displayCommands = false;
function runTestSuiteTest(configuration, spec, context) {
Expand Down

0 comments on commit 3471c28

Please sign in to comment.