From 71d6ff0a6b3ffc3d5c290bdaae6b9befa8bd7016 Mon Sep 17 00:00:00 2001 From: Ben Page Date: Mon, 10 Nov 2014 20:18:50 -0600 Subject: [PATCH] acquireTimeout naming collision test acquire err --- README.md | 22 +++++++--- lib/connection-pool.js | 82 +++++++++++++++++++++++++++--------- test/connection-pool.test.js | 45 +++++++++++++++----- test/load-test.js | 7 ++- test/naming.test.js | 12 ++++++ 5 files changed, 130 insertions(+), 38 deletions(-) create mode 100644 test/naming.test.js diff --git a/README.md b/README.md index 7e77f98..ea2b715 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ When the connection is released it is returned to the pool and is available to b * `max` {Number} The maximum number of connections there can be in the pool. Default = `50` * `idleTimeout` {Number} The number of milliseconds before closing an unused connection. Default = `300000` * `retryDelay` {Number} The number of milliseconds to wait after a connection fails, before trying again. Default = `5000` + * `aquireTimeout` {Number} The number of milliseconds to wait for a connection, before returning an error. Default = `60000` * `log` {Boolean|Function} Set to true to have debug log written to the console or pass a function to receive the log messages. Default = `undefined` * `connectionConfig` {Object} The same configuration that would be used to [create a @@ -64,7 +65,9 @@ When the connection is released it is returned to the pool and is available to b ### connectionPool.acquire(callback) Acquire a Tedious Connection object from the pool. -* `callback` {Function} Callback function + + * `callback(err, connection)` {Function} Callback function + * `err` {Object} An Error object is an error occurred trying to acquire a connection, otherwise null. * `connection` {Object} A [Connection](http://pekim.github.com/tedious/api-connection.html) ### connectionPool.drain() @@ -79,10 +82,17 @@ The following method is added to the Tedious [Connection](http://pekim.github.co ### Connection.release() Release the connect back to the pool to be used again -## Version 0.3.x Breaking Changes -* The err parameter has been removed from the callback passed to acquire(). Connection errors can happen at many at times other than during acquire(). Subscribe to the 'error' event to be notified of connection errors. +## Version 0.3.x Changes + * Removed dependency on the `generic-pool` node module. + * Added `poolConfig` options `retryDelay` + * Added `poolConfig` options `aquireTimeout` **(Possibly Breaking)** + * Added `poolConfig` options `log` + * `idleTimeoutMillis` renamed to `idleTimeout` **(Possibly Breaking)** + * The `ConnectionPool` `'error'` event added + * The behavior of the err parameter of the callback passed to `acquire()` has changed. It only returns errors related to acquiring a connection not Tedious Connection errors. Connection errors can happen anytime the pool is being filled and could go unnoticed if only passed the the callback. Subscribe to the `'error'` event on the pool to be notified of all connection errors. **(Possibly Breaking)** + * `PooledConnection` object removed. ## Version 0.2.x Breaking Changes -* To acquire a connection, call on acquire() on a ConnectionPool rather than requestConnection(). -* After acquiring a PooledConnection, do not wait for the 'connected' event. The connection is received connected. -* Call release() on a PooledConnection to release the it back to the pool. close() permenantly closes the connection (as close() behaves in in tedious). \ No newline at end of file +* To acquire a connection, call on `acquire()` on a `ConnectionPool` rather than `requestConnection()`. +* After acquiring a `PooledConnection`, do not wait for the `'connected'` event. The connection is received connected. +* Call `release()` on a `PooledConnection` to release the it back to the pool. `close()` permenantly closes the connection (as `close()` behaves in in tedious). \ No newline at end of file diff --git a/lib/connection-pool.js b/lib/connection-pool.js index 45b97bf..d602554 100644 --- a/lib/connection-pool.js +++ b/lib/connection-pool.js @@ -14,13 +14,24 @@ var RETRY = 3; function ConnectionPool(poolConfig, connectionConfig) { this.connections = []; - this.waitingForConnection = []; + this.waiting = []; this.connectionConfig = connectionConfig; this.max = poolConfig.max || 50; + this.min = poolConfig.min || 10; - this.idleTimeout = poolConfig.idleTimeout || poolConfig.idletimeoutMillis || 300000; //5 min - this.retryDelay = poolConfig.retryDelay || 5000; + + this.idleTimeout = !poolConfig.idleTimeout && poolConfig.idleTimeout !== false + ? 300000 //5 min + : poolConfig.idleTimeout; + + this.retryDelay = !poolConfig.retryDelay && poolConfig.retryDelay !== false + ? 5000 + : poolConfig.retryDelay; + + this.acquireTimeout = !poolConfig.acquireTimeout && poolConfig.acquireTimeout !== false + ? 60000 //1 min + : poolConfig.acquireTimeout; if (poolConfig.log) { if (Object.prototype.toString.call(poolConfig.log) == '[object Function]') @@ -87,9 +98,9 @@ function createConnection(pooled) { return; } - var callback = self.waitingForConnection.shift(); - if (callback !== undefined) - setUsed.call(self, pooled, callback); + var waiter = self.waiting.shift(); + if (waiter !== undefined) + setUsed.call(self, pooled, waiter); else setFree.call(self, pooled); }); @@ -124,7 +135,7 @@ function fill() { var amount = Math.min( this.max - this.connections.length, //max that can be created - this.waitingForConnection.length - available); //how many are needed, minus how many are available + this.waiting.length - available); //how many are needed, minus how many are available amount = Math.max( this.min - this.connections.length, //amount to create to reach min @@ -133,15 +144,15 @@ function fill() { if (amount > 0) this.log('filling ' + amount); - for (i = 0; i < amount; i++) { + for (i = 0; i < amount; i++) createConnection.call(this); - } } ConnectionPool.prototype.acquire = function (callback) { if (this.drained) //pool has been drained return; + var self = this; var free; //look for free connection @@ -155,19 +166,40 @@ ConnectionPool.prototype.acquire = function (callback) { } } + var waiter = { + callback: callback + }; + if (free === undefined) { //no valid connection found - this.waitingForConnection.push(callback); + if (this.acquireTimeout) { + + waiter.timeout = setTimeout(function () { + for (var i = self.waiting.length - 1; i >= 0; i--) { + var waiter2 = self.waiting[i]; + + if (waiter2.timeout === waiter.timeout) { + self.waiting.splice(i, 1); + waiter.callback(new Error('Acquire Timeout')); + return; + } + } + }, this.acquireTimeout); + } + + this.waiting.push(waiter); fill.call(this); } else { - setUsed.call(this, free, callback); + setUsed.call(this, free, waiter); } }; -function setUsed(pooled, callback) { +function setUsed(pooled, waiter) { pooled.status = USED; if (pooled.timeout) clearTimeout(pooled.timeout); - callback(pooled.con); + if (waiter.timeout) + clearTimeout(waiter.timeout); + waiter.callback(null, pooled.con); } function setFree(pooled) { @@ -184,10 +216,12 @@ ConnectionPool.prototype.release = function(connection) { if (this.drained) //pool has been drained return; - var callback = this.waitingForConnection.shift(); + var waiter = this.waiting.shift(); - if (callback !== undefined) { - callback(connection); + if (waiter !== undefined) { + if (waiter.timeout) + clearTimeout(waiter.timeout); + waiter.callback(null, connection); } else { for (var i = this.connections.length - 1; i >= 0; i--) { var pooled = this.connections[i]; @@ -206,11 +240,21 @@ ConnectionPool.prototype.drain = function () { this.drained = true; - for (var i = this.connections.length - 1; i >= 0; i--) - this.connections[i].con.close(); + for (var i = this.connections.length - 1; i >= 0; i--) { + var pooled = this.connections[i]; + pooled.con.close(); + if (pooled.timeout) + clearTimeout(pooled.timeout); + } + + for (i = this.waiting.length - 1; i >= 0; i--) { + var waiter = this.waiting[i]; + if (waiter.timeout) + clearTimeout(waiter.timeout); + } this.connections = null; - this.waitingForConnection = null; + this.waiting = null; }; module.exports = ConnectionPool; diff --git a/test/connection-pool.test.js b/test/connection-pool.test.js index 7dd9538..a1ad9a7 100644 --- a/test/connection-pool.test.js +++ b/test/connection-pool.test.js @@ -1,6 +1,6 @@ var assert = require('assert'); -var ConnectionPool = require('../lib/connection-pool'); var Request = require('tedious').Request; +var ConnectionPool = require('../lib/connection-pool'); var connectionConfig = { userName: 'test', @@ -32,6 +32,7 @@ ALTER LOGIN test DISABLE */ describe('ConnectionPool', function () { + it('min', function (done) { this.timeout(10000); @@ -54,7 +55,9 @@ describe('ConnectionPool', function () { var count = 20; var run = 0; - var createRequest = function (connection) { + var createRequest = function (err, connection) { + assert(!err); + var request = new Request('select 42', function (err, rowCount) { assert.strictEqual(rowCount, 1); setTimeout(function() { @@ -82,8 +85,7 @@ describe('ConnectionPool', function () { } }); - it('connection error event', function (done) { - + it('pool error event', function (done) { var poolConfig = {min: 2, max: 5}; var pool = new ConnectionPool(poolConfig, {}); pool.on('error', function(err) { @@ -93,7 +95,7 @@ describe('ConnectionPool', function () { }); }); - it('connection error retry', function (done) { + it('connection retry', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5, retryDelay: 5}; var pool = new ConnectionPool(poolConfig, {}); @@ -119,13 +121,33 @@ describe('ConnectionPool', function () { setTimeout(testConnected, 100); }); + it('acquire timeout', function (done) { + this.timeout(10000); + + var poolConfig = {min: 1, max: 1, acquireTimeout: 2000}; + var pool = new ConnectionPool(poolConfig, connectionConfig); + + pool.acquire(function(err, connection) { + assert(!err); + assert(!!connection); + }); + + pool.acquire(function(err, connection) { + assert(!!err); + assert(!connection); + done(); + }); + }); + it('idle timeout', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5, idleTimeout: 100}; var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { - pool.acquire(function (connection) { + pool.acquire(function (err, connection) { + assert(!err); + var request = new Request('select 42', function (err, rowCount) { assert.strictEqual(rowCount, 1); done(); @@ -142,7 +164,7 @@ describe('ConnectionPool', function () { }, 300); }); - it('lost connection error', function (done) { + it('lost connection handling', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5}; var pool = new ConnectionPool(poolConfig, connectionConfig); @@ -153,9 +175,10 @@ describe('ConnectionPool', function () { pool.drain(); }); - //This simulates a lost connections by creating a job that kills the current session and then deleting the job. - //The user must have the SQLAgentOperatorRole permission on the msdb database and ALTER ANY CONNECTION on master - pool.acquire(function (connection) { + //This simulates a lost connections by creating a job that kills the current session and then deletesthe job. + pool.acquire(function (err, connection) { + assert(!err); + var command = 'DECLARE @jobName VARCHAR(68) = \'pool\' + CONVERT(VARCHAR(64),NEWID()), @jobId UNIQUEIDENTIFIER;' + 'EXECUTE msdb..sp_add_job @jobName, @owner_login_name=\'' + connectionConfig.userName + '\', @job_id=@jobId OUTPUT;' + 'EXECUTE msdb..sp_add_jobserver @job_id=@jobId;' + @@ -164,7 +187,7 @@ describe('ConnectionPool', function () { 'SELECT @cmd = \'kill \' + CONVERT(VARCHAR(10), @@SPID);' + 'EXECUTE msdb..sp_add_jobstep @job_id=@jobId, @step_name=\'Step1\', @command = @cmd, @database_name = \'' + connectionConfig.options.database + '\', @on_success_action = 3;' + - 'DECLARE @deleteCommand varchar(200);' + + 'DECLARE @deleteCommand VARCHAR(200);' + 'SET @deleteCommand = \'execute msdb..sp_delete_job @job_name=\'\'\'+@jobName+\'\'\'\';' + 'EXECUTE msdb..sp_add_jobstep @job_id=@jobId, @step_name=\'Step2\', @command = @deletecommand;' + diff --git a/test/load-test.js b/test/load-test.js index 4bd490c..e5ec7fe 100644 --- a/test/load-test.js +++ b/test/load-test.js @@ -1,7 +1,7 @@ var ConnectionPool = require('../lib/connection-pool'); var Request = require('tedious').Request; -var poolConfig = {min: 20, max: 100, log: true}; +var poolConfig = {min: 20, max: 100}; var pool = new ConnectionPool(poolConfig, { userName: 'test', password: 'test', @@ -15,7 +15,10 @@ var total = clients * connections; var c = 0; var p = 0; -var createRequest = function (connection) { +var createRequest = function (err, connection) { + if (err) + console.err(err); + if (c >= total) return; diff --git a/test/naming.test.js b/test/naming.test.js new file mode 100644 index 0000000..4ce6711 --- /dev/null +++ b/test/naming.test.js @@ -0,0 +1,12 @@ +var assert = require('assert'); +var Connection = require('tedious').Connection; + +describe('Connection', function () { + it('Name Collision', function () { + assert(!Connection.prototype.release); + + var con = new Connection({}); + assert(!con.pool); + con.close(); + }); +});