Skip to content

Commit

Permalink
fix: Change socket timeout default to 0 (#2572)
Browse files Browse the repository at this point in the history
We make an implicit assumption with our default socket timeout that
most operations will complete within the current default of 5min. This is
not the case for services like Atlas Data Lake, which may regularly take
far longer to complete operations. In general we can't make any
assumptions about how long an operation will take to complete, and so
the default socket timeout is now 0 (infinity) unless otherwise indicated.

NODE-2835
  • Loading branch information
nbbeeken authored Nov 6, 2020
1 parent 033b6e7 commit 89b77ed
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 44 deletions.
2 changes: 1 addition & 1 deletion lib/cmap/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Connection extends EventEmitter {
this.id = options.id;
this.address = streamIdentifier(stream);
this.bson = options.bson;
this.socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 360000;
this.socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 0;
this.host = options.host || 'localhost';
this.port = options.port || 27017;
this.monitorCommands =
Expand Down
2 changes: 1 addition & 1 deletion lib/core/connection/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ function makeConnection(family, options, cancellationToken, _callback) {
: typeof options.connectTimeoutMS === 'number'
? options.connectTimeoutMS
: 30000;
const socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 360000;
const socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 0;
const rejectUnauthorized =
typeof options.rejectUnauthorized === 'boolean' ? options.rejectUnauthorized : true;

Expand Down
7 changes: 3 additions & 4 deletions lib/core/connection/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Connection extends EventEmitter {
* @param {boolean} [options.keepAlive=true] TCP Connection keep alive enabled
* @param {number} [options.keepAliveInitialDelay=120000] Initial delay before TCP keep alive enabled
* @param {number} [options.connectionTimeout=30000] TCP Connection timeout setting
* @param {number} [options.socketTimeout=360000] TCP Socket timeout setting
* @param {number} [options.socketTimeout=0] TCP Socket timeout setting
* @param {boolean} [options.promoteLongs] Convert Long values from the db into Numbers if they fit into 53 bits
* @param {boolean} [options.promoteValues] Promotes BSON values to native types where possible, set to false to only receive wrapper types.
* @param {boolean} [options.promoteBuffers] Promotes Binary BSON values to native Node Buffers.
Expand All @@ -92,7 +92,7 @@ class Connection extends EventEmitter {

this.port = options.port || 27017;
this.host = options.host || 'localhost';
this.socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 360000;
this.socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 0;

// These values are inspected directly in tests, but maybe not necessary to keep around
this.keepAlive = typeof options.keepAlive === 'boolean' ? options.keepAlive : true;
Expand Down Expand Up @@ -316,8 +316,7 @@ class Connection extends EventEmitter {
if (typeof options === 'function') (callback = options), (options = {});

const conn = this;
const socketTimeout =
typeof options.socketTimeout === 'number' ? options.socketTimeout : 360000;
const socketTimeout = typeof options.socketTimeout === 'number' ? options.socketTimeout : 0;
const bson = conn.options.bson;
const query = new Query(bson, ns, command, {
numberToSkip: 0,
Expand Down
6 changes: 3 additions & 3 deletions lib/core/connection/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ var _id = 0;
* @param {number} [options.keepAliveInitialDelay=120000] Initial delay before TCP keep alive enabled
* @param {boolean} [options.noDelay=true] TCP Connection no delay
* @param {number} [options.connectionTimeout=30000] TCP Connection timeout setting
* @param {number} [options.socketTimeout=360000] TCP Socket timeout setting
* @param {number} [options.monitoringSocketTimeout=30000] TCP Socket timeout setting for replicaset monitoring socket
* @param {number} [options.socketTimeout=0] TCP Socket timeout setting
* @param {number} [options.monitoringSocketTimeout=0] TCP Socket timeout setting for replicaset monitoring socket
* @param {boolean} [options.ssl=false] Use SSL for connection
* @param {boolean|function} [options.checkServerIdentity=true] Ensure we check server identify during SSL, set to false to disable checking. Only works for Node 0.12.x or higher. You can pass in a boolean or your own checkServerIdentity override function.
* @param {Buffer} [options.ca] SSL Certificate store binary buffer
Expand Down Expand Up @@ -111,7 +111,7 @@ var Pool = function(topology, options) {
minSize: 0,
// socket settings
connectionTimeout: 30000,
socketTimeout: 360000,
socketTimeout: 0,
keepAlive: true,
keepAliveInitialDelay: 120000,
noDelay: true,
Expand Down
2 changes: 1 addition & 1 deletion lib/core/topologies/replset.js
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ ReplSet.prototype.connect = function(options) {
);
});

// Error out as high availbility interval must be < than socketTimeout
// Error out as high availability interval must be < than socketTimeout
if (
this.s.options.socketTimeout > 0 &&
this.s.options.socketTimeout <= this.s.options.haInterval
Expand Down
2 changes: 1 addition & 1 deletion lib/core/topologies/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function topologyId(server) {
* @param {number} [options.keepAliveInitialDelay=120000] Initial delay before TCP keep alive enabled
* @param {boolean} [options.noDelay=true] TCP Connection no delay
* @param {number} [options.connectionTimeout=30000] TCP Connection timeout setting
* @param {number} [options.socketTimeout=360000] TCP Socket timeout setting
* @param {number} [options.socketTimeout=0] TCP Socket timeout setting
* @param {boolean} [options.ssl=false] Use SSL for connection
* @param {boolean|function} [options.checkServerIdentity=true] Ensure we check server identify during SSL, set to false to disable checking. Only works for Node 0.12.x or higher. You can pass in a boolean or your own checkServerIdentity override function.
* @param {Buffer} [options.ca] SSL Certificate store binary buffer
Expand Down
4 changes: 2 additions & 2 deletions lib/mongo_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const validOptions = require('./operations/connect').validOptions;
* @param {boolean} [options.keepAlive=true] TCP Connection keep alive enabled
* @param {number} [options.keepAliveInitialDelay=120000] The number of milliseconds to wait before initiating keepAlive on the TCP socket
* @param {number} [options.connectTimeoutMS=10000] How long to wait for a connection to be established before timing out
* @param {number} [options.socketTimeoutMS=360000] How long a send or receive on a socket can take before timing out
* @param {number} [options.socketTimeoutMS=0] How long a send or receive on a socket can take before timing out
* @param {number} [options.family] Version of IP stack. Can be 4, 6 or null (default).
* If null, will attempt to connect with IPv6, and will fall back to IPv4 on failure
* @param {number} [options.reconnectTries=30] Server attempt to reconnect #times
Expand Down Expand Up @@ -372,7 +372,7 @@ MongoClient.prototype.isConnected = function(options) {
* @param {boolean} [options.keepAlive=true] TCP Connection keep alive enabled
* @param {number} [options.keepAliveInitialDelay=120000] The number of milliseconds to wait before initiating keepAlive on the TCP socket
* @param {number} [options.connectTimeoutMS=10000] How long to wait for a connection to be established before timing out
* @param {number} [options.socketTimeoutMS=360000] How long a send or receive on a socket can take before timing out
* @param {number} [options.socketTimeoutMS=0] How long a send or receive on a socket can take before timing out
* @param {number} [options.family] Version of IP stack. Can be 4, 6 or null (default).
* If null, will attempt to connect with IPv6, and will fall back to IPv4 on failure
* @param {number} [options.reconnectTries=30] Server attempt to reconnect #times
Expand Down
4 changes: 2 additions & 2 deletions lib/operations/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ function connect(mongoClient, url, options, callback) {
const _finalOptions = createUnifiedOptions(object, options);

// Check if we have connection and socket timeout set
if (_finalOptions.socketTimeoutMS == null) _finalOptions.socketTimeoutMS = 360000;
if (_finalOptions.socketTimeoutMS == null) _finalOptions.socketTimeoutMS = 0;
if (_finalOptions.connectTimeoutMS == null) _finalOptions.connectTimeoutMS = 10000;
if (_finalOptions.retryWrites == null) _finalOptions.retryWrites = true;
if (_finalOptions.useRecoveryToken == null) _finalOptions.useRecoveryToken = true;
Expand Down Expand Up @@ -788,7 +788,7 @@ function translateOptions(options, translationOptions) {
}

// Set the socket and connection timeouts
if (options.socketTimeoutMS == null) options.socketTimeoutMS = 360000;
if (options.socketTimeoutMS == null) options.socketTimeoutMS = 0;
if (options.connectTimeoutMS == null) options.connectTimeoutMS = 10000;

if (!translationOptions.createServers) {
Expand Down
2 changes: 1 addition & 1 deletion lib/topologies/mongos.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ var legalOptionNames = [
* @param {boolean} [options.socketOptions.keepAlive=true] TCP Connection keep alive enabled
* @param {number} [options.socketOptions.keepAliveInitialDelay=120000] The number of milliseconds to wait before initiating keepAlive on the TCP socket
* @param {number} [options.socketOptions.connectTimeoutMS=10000] How long to wait for a connection to be established before timing out
* @param {number} [options.socketOptions.socketTimeoutMS=360000] How long a send or receive on a socket can take before timing out
* @param {number} [options.socketOptions.socketTimeoutMS=0] How long a send or receive on a socket can take before timing out
* @param {boolean} [options.domainsEnabled=false] Enable the wrapping of the callback in the current domain, disabled by default to avoid perf hit.
* @param {boolean} [options.monitorCommands=false] Enable command monitoring for this topology
* @fires Mongos#connect
Expand Down
2 changes: 1 addition & 1 deletion lib/topologies/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ var legalOptionNames = [
* @param {boolean} [options.socketOptions.keepAlive=true] TCP Connection keep alive enabled
* @param {number} [options.socketOptions.keepAliveInitialDelay=120000] The number of milliseconds to wait before initiating keepAlive on the TCP socket
* @param {number} [options.socketOptions.connectTimeoutMS=10000] How long to wait for a connection to be established before timing out
* @param {number} [options.socketOptions.socketTimeoutMS=360000] How long a send or receive on a socket can take before timing out
* @param {number} [options.socketOptions.socketTimeoutMS=0] How long a send or receive on a socket can take before timing out
* @param {number} [options.reconnectTries=30] Server attempt to reconnect #times
* @param {number} [options.reconnectInterval=1000] Server will wait # milliseconds between retries
* @param {boolean} [options.monitoring=true] Triggers the server instance to call ismaster
Expand Down
2 changes: 1 addition & 1 deletion test/functional/collection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ describe('Collection', function() {

db.listCollections().toArray((err, documents) => {
expect(err).to.not.exist;
expect(documents.length > 1).to.be.true;
expect(documents.length >= 1).to.be.true;
let found = false;

documents.forEach(document => {
Expand Down
7 changes: 5 additions & 2 deletions test/functional/mongo_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ describe('MongoClient', function() {

for (var i = 0; i < connections.length; i++) {
test.equal(10000, connections[i].connectionTimeout);
test.equal(360000, connections[i].socketTimeout);
expect(connections[i].socketTimeout).to.equal(0);
}

client.close();
Expand Down Expand Up @@ -518,7 +518,10 @@ describe('MongoClient', function() {
{},
{
keepAlive: true,
keepAliveInitialDelay: 100
keepAliveInitialDelay: 100,
// keepAliveInitialDelay is clamped to half the size of socketTimeout
// if socketTimeout is less than keepAliveInitialDelay
socketTimeout: 101
}
);

Expand Down
20 changes: 17 additions & 3 deletions test/functional/mongo_client_options.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';
const test = require('./shared').assert,
setupDatabase = require('./shared').setupDatabase,
expect = require('chai').expect;
const test = require('./shared').assert;
const setupDatabase = require('./shared').setupDatabase;
const expect = require('chai').expect;

describe('MongoClient Options', function() {
before(function() {
Expand Down Expand Up @@ -96,6 +96,20 @@ describe('MongoClient Options', function() {
}
});

it('should default socketTimeout to infinity', function(done) {
const client = this.configuration.newClient();
client.connect(() => {
expect(client.s.options.socketTimeoutMS).to.deep.equal(0);
const connections = client.topology.s.coreTopology
? client.topology.s.coreTopology.connections()
: [];
for (const connection of connections) {
expect(connection.socketTimeout).to.deep.equal(0);
}
client.close(done);
});
});

/**
* @ignore
*/
Expand Down
16 changes: 9 additions & 7 deletions test/functional/operation_example.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3936,15 +3936,17 @@ describe('Operation Examples', function() {
// REMOVE-LINE done();
// REMOVE-LINE var db = client.db(configuration.db);
// BEGIN
expect(err).to.not.exist;
var db = client.db(configuration.db);
test.equal(null, err);

// Retry to get the collection, should work as it's now created
db.collections(function(err, collections) {
test.equal(null, err);
test.ok(collections.length > 0);
db.createCollection('example', err => {
expect(err).to.not.exist;
// Retry to get the collection, should work as it's now created
db.collections(function(err, collections) {
expect(err).to.not.exist;
test.ok(collections.length > 0);

client.close(done);
client.close(done);
});
});
});
// END
Expand Down
1 change: 1 addition & 0 deletions test/functional/operation_generators_example.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2931,6 +2931,7 @@ describe('Operation (Generators)', function() {
.newClient(configuration.writeConcernMax(), { poolSize: 1 })
.connect();
var db = client.db(configuration.db);
yield db.createCollection('example');
// LINE var MongoClient = require('mongodb').MongoClient,
// LINE co = require('co');
// LINE test = require('assert');
Expand Down
32 changes: 19 additions & 13 deletions test/functional/operation_promises_example.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3149,22 +3149,28 @@ describe('Operation (Promises)', function() {
auto_reconnect: false
});

return client.connect().then(function(client) {
var db = client.db(configuration.db);
// LINE var MongoClient = require('mongodb').MongoClient,
// LINE test = require('assert');
// LINE const client = new MongoClient('mongodb://localhost:27017/test');
// LINE client.connect().then(() => {
// LINE var db = client.db('test);
// REPLACE configuration.writeConcernMax() WITH {w:1}
// REMOVE-LINE done();
// BEGIN
// Retry to get the collection, should work as it's now created
return db.collections().then(function(collections) {
return client
.connect()
.then(function(client) {
var db = client.db(configuration.db);
return db.createCollection('example');
})
.then(() => {
// LINE var MongoClient = require('mongodb').MongoClient,
// LINE test = require('assert');
// LINE const client = new MongoClient('mongodb://localhost:27017/test');
// LINE client.connect().then(() => {
// LINE var db = client.db('test);
// REPLACE configuration.writeConcernMax() WITH {w:1}
// REMOVE-LINE done();
// BEGIN
// Retry to get the collection, should work as it's now created
return client.db(configuration.db).collections();
})
.then(function(collections) {
test.ok(collections.length > 0);
return client.close();
});
});
// END
}
});
Expand Down
3 changes: 2 additions & 1 deletion test/functional/replset_connection.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
var f = require('util').format;
var test = require('./shared').assert;
const expect = require('chai').expect;
var setupDatabase = require('./shared').setupDatabase;

var restartAndDone = function(configuration, done) {
Expand Down Expand Up @@ -595,7 +596,7 @@ describe.skip('ReplSet (Connection)', function() {
var db = client.db(configuration.db);

test.equal(500, client.topology.connections()[0].connectionTimeout);
test.equal(360000, client.topology.connections()[0].socketTimeout);
expect(client.topology.connections()[0].socketTimeout).to.equal(0);

db.collection('replicaset_mongo_client_collection').update(
{ a: 1 },
Expand Down

0 comments on commit 89b77ed

Please sign in to comment.