Skip to content

Commit

Permalink
fix(replset): don't leak servers failing to connect
Browse files Browse the repository at this point in the history
The change to include all connecting servers in the local
`connectingServers` array lead to a "server leak" for nodes that
we are not able to connect to. Now we track them, and only add
new ones on a successful reconnect attempt.

NODE-2270
  • Loading branch information
mbroadst committed Oct 24, 2019
1 parent 295ea4c commit f209160
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions lib/core/topologies/replset.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,26 @@ function rexecuteOperations(self) {
}

function connectNewServers(self, servers, callback) {
// No new servers
if (servers.length === 0) {
return callback();
}

// Count lefts
var count = servers.length;
var error = null;

function done() {
count = count - 1;
if (count === 0) {
callback(error);
}
}

// Handle events
var _handleEvent = function(self, event) {
return function(err) {
var _self = this;
count = count - 1;

// Destroyed
if (self.state === DESTROYED || self.state === UNREFERENCED) {
Expand Down Expand Up @@ -332,17 +343,10 @@ function connectNewServers(self, servers, callback) {

// Rexecute any stalled operation
rexecuteOperations(self);

// Are we done finish up callback
if (count === 0) {
callback(error);
}
done();
};
};

// No new servers
if (count === 0) return callback();

// Execute method
function execute(_server, i) {
setTimeout(function() {
Expand All @@ -351,6 +355,18 @@ function connectNewServers(self, servers, callback) {
return;
}

// remove existing connecting server if it's failed to connect, otherwise
// wait for that server to connect
const existingServerIdx = self.s.connectingServers.findIndex(s => s.name === _server);
if (existingServerIdx >= 0) {
const connectingServer = self.s.connectingServers[existingServerIdx];
connectingServer.destroy({ force: true });

self.s.connectingServers.splice(existingServerIdx, 1);
done();
return;
}

// Create a new server instance
var server = new Server(
Object.assign({}, self.s.options, {
Expand Down

0 comments on commit f209160

Please sign in to comment.