diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0dea9ef1..549798c0 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -3,6 +3,8 @@ v6.0.0 (2019-0?-??) [change] Upgraded tedious to v6 ([#818](https://github.com/tediousjs/node-mssql/pull/818)) [change] `options.encrypt` is now set to true by default [change] Upgraded `debug` dependency to v4 +[change] Replaced pool library (`generic-pool`) with `tarn.js` ([#808](https://github.com/tediousjs/node-mssql/pull/808)) +[removed] Backoff try strategy for creating connections removed, `tarn.js` built-in retry strategy used instead v5.0.0 (2019-03-07) ------------------- diff --git a/README.md b/README.md index 9da235e7..918b4188 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ If you're on Windows Azure, add `?encrypt=true` to your connection string. See [ * [SQL injection](#sql-injection) * [Known Issues](#known-issues) * [Contributing](https://github.com/tediousjs/node-mssql/wiki/Contributing) +* [4.x to 5.x changes](#4x-to-5x-changes) * [3.x to 4.x changes](#3x-to-4x-changes) * [3.x Documentation](https://github.com/tediousjs/node-mssql/blob/1893969195045a250f0fdeeb2de7f30dcf6689ad/README.md) @@ -393,7 +394,7 @@ const config = { - **pool.min** - The minimum of connections there can be in the pool (default: `0`). - **pool.idleTimeoutMillis** - The Number of milliseconds before closing an unused connection (default: `30000`). -Complete list of pool options can be found [here](https://github.com/coopernurse/node-pool). +Complete list of pool options can be found [here](https://github.com/vincit/tarn.js/#usage). ### Formats @@ -1554,6 +1555,14 @@ request.query('select @myval as myval', (err, result) => { - msnodesqlv8 has problem with errors during transactions - [reported](https://github.com/tediousjs/node-mssql/issues/77). - msnodesqlv8 doesn't support [detailed SQL errors](#detailed-sql-errors). +## 4.x to 5.x changes + +- Moved pool library from `node-pool` to `tarn.js` +- `ConnectionPool.pool.size` deprecated, use `ConnectionPool.size` instead +- `ConnectionPool.pool.available` deprecated, use `ConnectionPool.available` instead +- `ConnectionPool.pool.pending` deprecated, use `ConnectionPool.pending` instead +- `ConnectionPool.pool.borrowed` deprecated, use `ConnectionPool.borrowed` instead + ## 3.x to 4.x changes - Library & tests are rewritten to ES6. diff --git a/lib/base.js b/lib/base.js index fc745129..a5abcd2b 100644 --- a/lib/base.js +++ b/lib/base.js @@ -2,7 +2,7 @@ const EventEmitter = require('events').EventEmitter const debug = require('debug')('mssql:base') -const gp = require('generic-pool') +const tarn = require('tarn') const TYPES = require('./datatypes').TYPES const declare = require('./datatypes').declare @@ -170,12 +170,16 @@ class ConnectionPool extends EventEmitter { */ acquire (requester, callback) { + const acquirePromise = this._acquire().promise.catch(err => { + this.emit('error', err) + throw err + }) if (typeof callback === 'function') { - this._acquire().then(connection => callback(null, connection, this.config)).catch(callback) + acquirePromise.then(connection => callback(null, connection, this.config)).catch(callback) return this } - return this._acquire() + return acquirePromise } _acquire () { @@ -244,81 +248,84 @@ class ConnectionPool extends EventEmitter { this._poolCreate().then((connection) => { debug('pool(%d): connected', IDS.get(this)) - return this._poolDestroy(connection).then(() => { - if (!this._connecting) { - debug('pool(%d): not connecting, exiting silently (was close called before connection established?)', IDS.get(this)) - return - } - - let createAttempt = 0 - - function create () { - // Create an "ID" to help track debugging messages - const createId = createAttempt++ - const timeout = this.config.pool ? this.config.pool.acquireTimeoutMillis : undefined - - return this._poolCreateRetry(createId, 0, timeout) - } + this._poolDestroy(connection) + if (!this._connecting) { + debug('pool(%d): not connecting, exiting silently (was close called before connection established?)', IDS.get(this)) + return + } - // prepare pool - this.pool = gp.createPool({ - create: create.bind(this), + // prepare pool + this.pool = new tarn.Pool( + Object.assign({ + create: this._poolCreate.bind(this), validate: this._poolValidate.bind(this), - destroy: this._poolDestroy.bind(this) - }, Object.assign({ + destroy: this._poolDestroy.bind(this), max: 10, min: 0, - evictionRunIntervalMillis: 1000, idleTimeoutMillis: 30000, - testOnBorrow: true - }, this.config.pool)) - - this.pool.on('factoryCreateError', this.emit.bind(this, 'error')) - this.pool.on('factoryDestroyError', this.emit.bind(this, 'error')) + propagateCreateError: true + }, this.config.pool) + ) + const self = this + Object.defineProperties(this.pool, { + size: { + get: () => { + const message = 'the `size` property on pool is deprecated, access it directly on the `ConnectionPool`' + self.emit('debug', message) + process.emitWarning(message) + return self.size + } + }, + available: { + get: () => { + const message = 'the `available` property on pool is deprecated, access it directly on the `ConnectionPool`' + self.emit('debug', message) + process.emitWarning(message) + return self.available + } + }, + pending: { + get: () => { + const message = 'the `pending` property on pool is deprecate, access it directly on the `ConnectionPool`' + self.emit('debug', message) + process.emitWarning(message) + return self.pending + } + }, + borrowed: { + get: () => { + const message = 'the `borrowed` property on pool is deprecated, access it directly on the `ConnectionPool`' + self.emit('debug', message) + process.emitWarning(message) + return self.borrowed + } + } + }) - this._connecting = false - this._connected = true + this._connecting = false + this._connected = true - callback(null) - }) + callback(null) }).catch(err => { this._connecting = false callback(err) }) } - _poolCreateRetry (createId, retryCount, timeout) { - const self = this - let backoff - debug('pool(%d): attempting to create connection resource(%d), attempt #%d', IDS.get(this), createId, retryCount) - // increment our retry count on error and calculate a new backoff - retryCount++ - return this._poolCreate().catch((e) => { - // don't bother calculating backoffs > 8, this caps us at ~30s per retry - backoff = retryCount > 8 ? backoff : Math.pow(2, retryCount) * 100 - if (typeof timeout !== 'undefined') { - timeout -= backoff - if (timeout <= 0) { - throw e - } - } - return new Promise((resolve, reject) => { - // construct a timer-based promise to retry the connection attempt - const timer = setTimeout(() => { - debug('pool(%d): backoff timer complete resource(%d)', IDS.get(self), createId) - // if the connection has been closed, reject - if (!self.connecting && !self.connected) { - debug('pool(%d): pool closed while trying to acquire a connection resource(%d)', IDS.get(self), createId) - reject(new ConnectionError('Connection is closed.', 'ECONNCLOSED')) - } else { - resolve() - } - }, backoff) - // don't let this timer block node from exiting - timer.unref() - debug('pool(%d): failed to create connection resource(%d), retrying with backoff of %dms', IDS.get(self), createId, backoff) - }).then(this._poolCreateRetry.bind(this, createId, retryCount, timeout)) - }) + get size () { + return this.pool.numFree() + this.pool.numUsed() + this.pool.numPendingCreates() + } + + get available () { + return this.pool.numFree() + } + + get pending () { + return this.pool.numPendingAcquires() + } + + get borrowed () { + return this.pool.numUsed() } /** @@ -352,13 +359,9 @@ class ConnectionPool extends EventEmitter { if (!this.pool) return setImmediate(callback, null) - const pool = this.pool + this.pool.destroy() this.pool = null - pool.drain().then(() => { - return pool.clear() - }).then(() => { - callback(null) - }) + callback(null) } /** diff --git a/lib/msnodesqlv8.js b/lib/msnodesqlv8.js index 9fa0862e..e0ae1f5c 100644 --- a/lib/msnodesqlv8.js +++ b/lib/msnodesqlv8.js @@ -201,9 +201,7 @@ class ConnectionPool extends base.ConnectionPool { } _poolValidate (tds) { - return new base.Promise((resolve, reject) => { - resolve(tds && !tds.hasError) - }) + return tds && !tds.hasError } _poolDestroy (tds) { diff --git a/lib/tedious.js b/lib/tedious.js index 49cd471d..587d19a0 100644 --- a/lib/tedious.js +++ b/lib/tedious.js @@ -193,6 +193,14 @@ const parameterCorrection = function (value) { class ConnectionPool extends base.ConnectionPool { _poolCreate () { return new base.Promise((resolve, reject) => { + const resolveOnce = (v) => { + resolve(v) + resolve = reject = () => {} + } + const rejectOnce = (e) => { + reject(e) + resolve = reject = () => {} + } const cfg = { server: this.config.server, options: Object.assign({ @@ -233,7 +241,6 @@ class ConnectionPool extends base.ConnectionPool { payload: true } } - const tedious = new tds.Connection(cfg) IDS.add(tedious, 'Connection') debug('pool(%d): connection #%d created', IDS.get(this), IDS.get(tedious)) @@ -242,22 +249,24 @@ class ConnectionPool extends base.ConnectionPool { tedious.once('connect', err => { if (err) { err = new base.ConnectionError(err) - return reject(err) + return rejectOnce(err) } debug('connection(%d): established', IDS.get(tedious)) - resolve(tedious) + resolveOnce(tedious) }) + tedious.on('end', () => { + const err = new base.ConnectionError('The connection ended without ever completing the connection') + rejectOnce(err) + }) tedious.on('error', err => { if (err.code === 'ESOCKET') { tedious.hasError = true - reject(err) - return + } else { + this.emit('error', err) } - - this.emit('error', err) - reject(err) + rejectOnce(err) }) if (this.config.debug) { @@ -270,31 +279,24 @@ class ConnectionPool extends base.ConnectionPool { } _poolValidate (tedious) { - return new base.Promise((resolve, reject) => { - resolve(tedious && !tedious.closed && !tedious.hasError) - }) + return tedious && !tedious.closed && !tedious.hasError } _poolDestroy (tedious) { - return new base.Promise((resolve, reject) => { - if (!tedious) { - resolve() - return - } - debug('connection(%d): destroying', IDS.get(tedious)) - - if (tedious.closed) { - debug('connection(%d): already closed', IDS.get(tedious)) - resolve() - } else { - tedious.once('end', () => { - debug('connection(%d): destroyed', IDS.get(tedious)) - resolve() - }) + if (!tedious) { + return + } + debug('connection(%d): destroying', IDS.get(tedious)) - tedious.close() - } - }) + if (tedious.closed) { + debug('connection(%d): already closed', IDS.get(tedious)) + } else { + tedious.once('end', () => { + debug('connection(%d): destroyed', IDS.get(tedious)) + }) + + tedious.close() + } } } diff --git a/package-lock.json b/package-lock.json index 7c0ee1f3..eb29c5e7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1446,11 +1446,6 @@ "integrity": "sha1-GwqzvVU7Kg1jmdKcDj6gslIHgyc=", "dev": true }, - "generic-pool": { - "version": "3.6.1", - "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.6.1.tgz", - "integrity": "sha512-iMmD/pY4q0+V+f8o4twE9JPeqfNuX+gJAaIPB3B0W1lFkBOtTxBo6B0HxHPgGhzQA8jego7EWopcYq/UDJO2KA==" - }, "get-caller-file": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-1.0.3.tgz", @@ -3465,6 +3460,11 @@ "string-width": "^2.1.1" } }, + "tarn": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/tarn/-/tarn-1.1.4.tgz", + "integrity": "sha512-j4samMCQCP5+6Il9/cxCqBd3x4vvlLeVdoyGex0KixPKl4F8LpNbDSC6NDhjianZgUngElRr9UI1ryZqJDhwGg==" + }, "tedious": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/tedious/-/tedious-6.0.0.tgz", diff --git a/package.json b/package.json index 68161186..5677b060 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "license": "MIT", "dependencies": { "debug": "^4", - "generic-pool": "^3.6.1", + "tarn": "^1.1.4", "tedious": "^6" }, "devDependencies": { diff --git a/test/common/tests.js b/test/common/tests.js index 9eafa191..35271b4b 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -808,10 +808,10 @@ module.exports = (sql, driver) => { const complete = () => setTimeout(() => { // this must be delayed because destroying connection take some time - assert.strictEqual(connection.pool.size, 3) - assert.strictEqual(connection.pool.available, 3) - assert.strictEqual(connection.pool.pending, 0) - assert.strictEqual(connection.pool.borrowed, 0) + assert.strictEqual(connection.size, 3) + assert.strictEqual(connection.available, 3) + assert.strictEqual(connection.pending, 0) + assert.strictEqual(connection.borrowed, 0) done() }, 500) @@ -874,10 +874,10 @@ module.exports = (sql, driver) => { }) setImmediate(() => { - assert.strictEqual(connection.pool.size, 1) - assert.strictEqual(connection.pool.available, 0) - assert.strictEqual(connection.pool.pending, 3) - assert.strictEqual(connection.pool.borrowed, 0) + assert.strictEqual(connection.size, 1) + assert.strictEqual(connection.available, 0) + assert.strictEqual(connection.pending, 3) + assert.strictEqual(connection.borrowed, 0) }) }, @@ -892,10 +892,10 @@ module.exports = (sql, driver) => { r3.query('select 1', function (err, result) { if (err) return done(err) - assert.strictEqual(connection2.pool.size, 1) - assert.strictEqual(connection2.pool.available, 1) - assert.strictEqual(connection2.pool.pending, 0) - assert.strictEqual(connection2.pool.borrowed, 0) + assert.strictEqual(connection2.size, 1) + assert.strictEqual(connection2.available, 1) + assert.strictEqual(connection2.pending, 0) + assert.strictEqual(connection2.borrowed, 0) done() })