From e0789d7800dbc1988a2dc708759b77cafc2cb883 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Mon, 27 Mar 2023 22:33:20 +0800 Subject: [PATCH 1/6] feat(pool): added acquireTimeout supports --- lib/connection_config.js | 3 +- lib/pool.js | 19 ++++++++ lib/pool_config.js | 3 ++ test/integration/test-pool-acquire-timeout.js | 48 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 test/integration/test-pool-acquire-timeout.js diff --git a/lib/connection_config.js b/lib/connection_config.js index a18b30b330..d5d4f7b9a0 100644 --- a/lib/connection_config.js +++ b/lib/connection_config.js @@ -65,7 +65,8 @@ const validOptions = { idleTimeout: 1, Promise: 1, queueLimit: 1, - waitForConnections: 1 + waitForConnections: 1, + acquireTimeout: 1 }; class ConnectionConfig { diff --git a/lib/pool.js b/lib/pool.js index 1b4993ff1a..4fef6e53fb 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -78,6 +78,25 @@ class Pool extends EventEmitter { return cb(new Error('Queue limit reached.')); } this.emit('enqueue'); + + if (this.config.acquireTimeout > 0) { + + const timeout = setTimeout(() => { + + spliceConnection(this._connectionQueue, cb); + cb(new Error('Timeout acquiring connection')); + + }, this.config.acquireTimeout); + + return this._connectionQueue.push((err, connection) => { + + clearTimeout(timeout); + + cb(err, connection); + + }, this.config.acquireTimeout); + } + return this._connectionQueue.push(cb); } diff --git a/lib/pool_config.js b/lib/pool_config.js index 0a4a26097f..3d8f01a5ce 100644 --- a/lib/pool_config.js +++ b/lib/pool_config.js @@ -15,6 +15,9 @@ class PoolConfig { this.connectionLimit = isNaN(options.connectionLimit) ? 10 : Number(options.connectionLimit); + this.acquireTimeout = Number.isInteger(options.acquireTimeout) && options.acquireTimeout >= 0 + ? options.acquireTimeout + : 10000; this.maxIdle = isNaN(options.maxIdle) ? this.connectionLimit : Number(options.maxIdle); diff --git a/test/integration/test-pool-acquire-timeout.js b/test/integration/test-pool-acquire-timeout.js new file mode 100644 index 0000000000..ec17a5a9f1 --- /dev/null +++ b/test/integration/test-pool-acquire-timeout.js @@ -0,0 +1,48 @@ +'use strict'; + +const mysql = require('../..'); +const test = require('utest'); +const assert = require('assert'); +const common = require('../common'); + +const poolConfig = common.getConfig(); + +const ACQUITE_TIMEOUT = 500; +const poolWithAcquireTimeout = new mysql.createPool({ + ...poolConfig, + acquireTimeout: ACQUITE_TIMEOUT, + connectionLimit: 2 +}); + +poolWithAcquireTimeout.getConnection(function (err, c1) { + assert.equal(!!c1, true); + assert.ifError(err); + poolWithAcquireTimeout.getConnection(function (err, c2) { + assert.ifError(err); + assert.equal(!!c2, true); + const C3_STARTED_AT = Date.now(); + poolWithAcquireTimeout.getConnection(function (e3, c3) { + const C3_DONE_AT = Date.now(); + + poolWithAcquireTimeout.releaseConnection(c1); + + poolWithAcquireTimeout.getConnection(function (e4, c4) { + + test('Pool With Acquire Timeout', { + 'timeout of pool is full': () => { + assert.equal(e3 !== null, true); + assert.equal(!c3, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUITE_TIMEOUT, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUITE_TIMEOUT * 2, true); + }, + 'ok if pool is not full': () => { + assert.equal(e4 === null, true); + assert.equal(!!c4, true); + } + }); + + poolWithAcquireTimeout.releaseConnection(c4); + }); + }); + }); +}); From f6a5fa8378ce53b0d72f372bfc67ae195a5143a3 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Tue, 28 Mar 2023 14:29:33 +0800 Subject: [PATCH 2/6] fix(lint): fixed linting on test suites --- test/integration/test-pool-acquire-timeout.js | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/test/integration/test-pool-acquire-timeout.js b/test/integration/test-pool-acquire-timeout.js index ec17a5a9f1..2ad09e257f 100644 --- a/test/integration/test-pool-acquire-timeout.js +++ b/test/integration/test-pool-acquire-timeout.js @@ -9,40 +9,40 @@ const poolConfig = common.getConfig(); const ACQUITE_TIMEOUT = 500; const poolWithAcquireTimeout = new mysql.createPool({ - ...poolConfig, - acquireTimeout: ACQUITE_TIMEOUT, - connectionLimit: 2 + ...poolConfig, + acquireTimeout: ACQUITE_TIMEOUT, + connectionLimit: 2 }); -poolWithAcquireTimeout.getConnection(function (err, c1) { - assert.equal(!!c1, true); +poolWithAcquireTimeout.getConnection((err, c1) => { + assert.equal(!!c1, true); + assert.ifError(err); + poolWithAcquireTimeout.getConnection((err, c2) => { assert.ifError(err); - poolWithAcquireTimeout.getConnection(function (err, c2) { - assert.ifError(err); - assert.equal(!!c2, true); - const C3_STARTED_AT = Date.now(); - poolWithAcquireTimeout.getConnection(function (e3, c3) { - const C3_DONE_AT = Date.now(); + assert.equal(!!c2, true); + const C3_STARTED_AT = Date.now(); + poolWithAcquireTimeout.getConnection((e3, c3) => { + const C3_DONE_AT = Date.now(); - poolWithAcquireTimeout.releaseConnection(c1); + poolWithAcquireTimeout.releaseConnection(c1); - poolWithAcquireTimeout.getConnection(function (e4, c4) { + poolWithAcquireTimeout.getConnection((e4, c4) => { - test('Pool With Acquire Timeout', { - 'timeout of pool is full': () => { - assert.equal(e3 !== null, true); - assert.equal(!c3, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUITE_TIMEOUT, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUITE_TIMEOUT * 2, true); - }, - 'ok if pool is not full': () => { - assert.equal(e4 === null, true); - assert.equal(!!c4, true); - } - }); - - poolWithAcquireTimeout.releaseConnection(c4); - }); + test('Pool With Acquire Timeout', { + 'timeout of pool is full': () => { + assert.equal(e3 !== null, true); + assert.equal(!c3, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUITE_TIMEOUT, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUITE_TIMEOUT * 2, true); + }, + 'ok if pool is not full': () => { + assert.equal(e4 === null, true); + assert.equal(!!c4, true); + } }); + + poolWithAcquireTimeout.releaseConnection(c4); + }); }); + }); }); From a97ff4250544eff9b88d131cdfe2b5e4ed779d30 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Wed, 10 Apr 2024 09:06:31 +0800 Subject: [PATCH 3/6] fix(test): spelling mistakes of ACQUIRE_TIMEOUT constant --- test/integration/test-pool-acquire-timeout.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/test-pool-acquire-timeout.js b/test/integration/test-pool-acquire-timeout.js index 2ad09e257f..0a03d4e70c 100644 --- a/test/integration/test-pool-acquire-timeout.js +++ b/test/integration/test-pool-acquire-timeout.js @@ -7,10 +7,10 @@ const common = require('../common'); const poolConfig = common.getConfig(); -const ACQUITE_TIMEOUT = 500; +const ACQUIRE_TIMEOUT = 500; const poolWithAcquireTimeout = new mysql.createPool({ ...poolConfig, - acquireTimeout: ACQUITE_TIMEOUT, + acquireTimeout: ACQUIRE_TIMEOUT, connectionLimit: 2 }); @@ -32,8 +32,8 @@ poolWithAcquireTimeout.getConnection((err, c1) => { 'timeout of pool is full': () => { assert.equal(e3 !== null, true); assert.equal(!c3, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUITE_TIMEOUT, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUITE_TIMEOUT * 2, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUIRE_TIMEOUT, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUIRE_TIMEOUT * 2, true); }, 'ok if pool is not full': () => { assert.equal(e4 === null, true); From 1e3cf9b0cf248a72ec7645053e572075aacc21c1 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Wed, 10 Apr 2024 11:09:54 +0800 Subject: [PATCH 4/6] fix(pool): getConnection callback should be called only once --- lib/pool.js | 28 +++++------ .../test-promise-wrappers.test.cjs | 47 ++++++++++++++++++ test/integration/test-pool-acquire-timeout.js | 48 ------------------- .../test-pool-acquire-timeout.test.js | 46 ++++++++++++++++++ 4 files changed, 107 insertions(+), 62 deletions(-) delete mode 100644 test/integration/test-pool-acquire-timeout.js create mode 100644 test/integration/test-pool-acquire-timeout.test.js diff --git a/lib/pool.js b/lib/pool.js index 4fef6e53fb..6fbd86553a 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -56,7 +56,7 @@ class Pool extends EventEmitter { config: this.config.connectionConfig }); this._allConnections.push(connection); - return connection.connect(err => { + return connection.connect((err) => { if (this._closed) { return cb(new Error('Pool is closed.')); } @@ -80,21 +80,20 @@ class Pool extends EventEmitter { this.emit('enqueue'); if (this.config.acquireTimeout > 0) { + // eslint-disable-next-line prefer-const + let timer; - const timeout = setTimeout(() => { + const wrappedCb = (err, connection) => { + clearTimeout(timer); + cb(err, connection); + }; - spliceConnection(this._connectionQueue, cb); + timer = setTimeout(() => { + spliceConnection(this._connectionQueue, wrappedCb); cb(new Error('Timeout acquiring connection')); - }, this.config.acquireTimeout); - return this._connectionQueue.push((err, connection) => { - - clearTimeout(timeout); - - cb(err, connection); - - }, this.config.acquireTimeout); + return this._connectionQueue.push(wrappedCb); } return this._connectionQueue.push(cb); @@ -121,7 +120,7 @@ class Pool extends EventEmitter { this._closed = true; clearTimeout(this._removeIdleTimeoutConnectionsTimer); if (typeof cb !== 'function') { - cb = function(err) { + cb = function (err) { if (err) { throw err; } @@ -130,7 +129,7 @@ class Pool extends EventEmitter { let calledBack = false; let closedConnections = 0; let connection; - const endCB = function(err) { + const endCB = function (err) { if (calledBack) { return; } @@ -158,7 +157,8 @@ class Pool extends EventEmitter { this.config.connectionConfig ); if (typeof cmdQuery.namedPlaceholders === 'undefined') { - cmdQuery.namedPlaceholders = this.config.connectionConfig.namedPlaceholders; + cmdQuery.namedPlaceholders = + this.config.connectionConfig.namedPlaceholders; } this.getConnection((err, conn) => { if (err) { diff --git a/test/integration/promise-wrappers/test-promise-wrappers.test.cjs b/test/integration/promise-wrappers/test-promise-wrappers.test.cjs index 6f3a2b1f45..96053d091c 100644 --- a/test/integration/promise-wrappers/test-promise-wrappers.test.cjs +++ b/test/integration/promise-wrappers/test-promise-wrappers.test.cjs @@ -75,6 +75,52 @@ function testErrors() { }); } +function testPoolAcquireTimeout() { + + (async () => { + const ACQUIRE_TIMEOUT = 500; + const pool = createPool({ + ...config, + connectionLimit: 2, + acquireTimeout: ACQUIRE_TIMEOUT, + }); + + const c1 = await pool.getConnection(); + + assert.ok(c1); + + const c2 = await pool.getConnection(); + + assert.ok(c2); + + const c3StartedAt = Date.now(); + try { + + await pool.getConnection(); + assert.fail('should not reach here'); + } + catch (e) { + + assert.equal(Date.now() - c3StartedAt >= ACQUIRE_TIMEOUT, true); + assert.equal(e.message, 'Timeout acquiring connection'); + } + + c2.release(); + + const c4 = await pool.getConnection(); + + c1.release(); + c4.release(); + + await pool.end(); + + assert.ok(c4, 'acquireTimeout is working correctly'); + + })().catch((err) => { + assert.fail(err); + }); +} + function testObjParams() { let connResolved; createConnection(config) @@ -473,6 +519,7 @@ testChangeUser(); testConnectionProperties(); testPoolConnectionDestroy(); testPromiseLibrary(); +testPoolAcquireTimeout(); process.on('exit', () => { assert.equal(doneCalled, true, 'done not called'); diff --git a/test/integration/test-pool-acquire-timeout.js b/test/integration/test-pool-acquire-timeout.js deleted file mode 100644 index 0a03d4e70c..0000000000 --- a/test/integration/test-pool-acquire-timeout.js +++ /dev/null @@ -1,48 +0,0 @@ -'use strict'; - -const mysql = require('../..'); -const test = require('utest'); -const assert = require('assert'); -const common = require('../common'); - -const poolConfig = common.getConfig(); - -const ACQUIRE_TIMEOUT = 500; -const poolWithAcquireTimeout = new mysql.createPool({ - ...poolConfig, - acquireTimeout: ACQUIRE_TIMEOUT, - connectionLimit: 2 -}); - -poolWithAcquireTimeout.getConnection((err, c1) => { - assert.equal(!!c1, true); - assert.ifError(err); - poolWithAcquireTimeout.getConnection((err, c2) => { - assert.ifError(err); - assert.equal(!!c2, true); - const C3_STARTED_AT = Date.now(); - poolWithAcquireTimeout.getConnection((e3, c3) => { - const C3_DONE_AT = Date.now(); - - poolWithAcquireTimeout.releaseConnection(c1); - - poolWithAcquireTimeout.getConnection((e4, c4) => { - - test('Pool With Acquire Timeout', { - 'timeout of pool is full': () => { - assert.equal(e3 !== null, true); - assert.equal(!c3, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUIRE_TIMEOUT, true); - assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUIRE_TIMEOUT * 2, true); - }, - 'ok if pool is not full': () => { - assert.equal(e4 === null, true); - assert.equal(!!c4, true); - } - }); - - poolWithAcquireTimeout.releaseConnection(c4); - }); - }); - }); -}); diff --git a/test/integration/test-pool-acquire-timeout.test.js b/test/integration/test-pool-acquire-timeout.test.js new file mode 100644 index 0000000000..0fc5a17c9a --- /dev/null +++ b/test/integration/test-pool-acquire-timeout.test.js @@ -0,0 +1,46 @@ +'use strict'; + +const mysql = require('../..'); +const assert = require('assert'); +const common = require('../common.test.cjs'); + +const poolConfig = common.getConfig(); + +const ACQUIRE_TIMEOUT = 500; +const pool = new mysql.createPool({ + ...poolConfig, + acquireTimeout: ACQUIRE_TIMEOUT, + connectionLimit: 2 +}); + +pool.getConnection((err, c1) => { + assert.equal(!!c1, true); + assert.ifError(err); + + pool.getConnection((err, c2) => { + assert.ifError(err); + assert.equal(!!c2, true); + + const C3_STARTED_AT = Date.now(); + + pool.getConnection((e3, c3) => { + const C3_DONE_AT = Date.now(); + assert.equal(C3_DONE_AT - C3_STARTED_AT >= ACQUIRE_TIMEOUT, true); + assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUIRE_TIMEOUT * 2, true); + + assert.notEqual(e3, null); + assert.equal(e3.message, 'Timeout acquiring connection', 'Acquire timeout error message is correct'); + assert.equal(!c3, true); + c1.release(); + + pool.getConnection((e4, c4) => { + assert.equal(e4, null); + assert.equal(!!c4, true); + + c4.release(); + c2.release(); + pool.end(); + }); + }); + }); +}); From ec0fde9502abb5c603842ad9b13994689c068fd0 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Wed, 10 Apr 2024 11:13:00 +0800 Subject: [PATCH 5/6] fix: added typings for acquireTimeout option --- .../mysql/createPool/callbacks/createPool.test.ts | 4 ++++ typings/mysql/lib/Pool.d.ts | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts b/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts index 508b08ebc4..bcd7cf39eb 100644 --- a/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts +++ b/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts @@ -9,4 +9,8 @@ import { uriAccess, access } from '../../baseConnection.test.js'; uriPool = mysql.createPool(uriAccess); pool = mysql.createPool(access); + pool = mysql.createPool({ + ...access, + acquireTimeout: 1000, + }); })(); diff --git a/typings/mysql/lib/Pool.d.ts b/typings/mysql/lib/Pool.d.ts index 042495e71f..575c5200db 100644 --- a/typings/mysql/lib/Pool.d.ts +++ b/typings/mysql/lib/Pool.d.ts @@ -22,6 +22,13 @@ export interface PoolOptions extends ConnectionOptions { */ connectionLimit?: number; + /** + * The maximum time (in milliseconds) to wait for getting a connection from the pool. (Default: 10 seconds) + * + * @default 10000 + */ + acquireTimeout?: number; + /** * The maximum number of idle connections. (Default: same as `connectionLimit`) */ From 5fabc6e8ca01f16cb78fc2ce518883754b003807 Mon Sep 17 00:00:00 2001 From: Angus ZENG Date: Wed, 10 Apr 2024 11:43:09 +0800 Subject: [PATCH 6/6] fix: linting of tests --- .../promise-wrappers/test-promise-wrappers.test.cjs | 7 +------ ...timeout.test.js => test-pool-acquire-timeout.test.cjs} | 8 ++++++-- .../mysql/createPool/callbacks/createPool.test.ts | 4 ---- 3 files changed, 7 insertions(+), 12 deletions(-) rename test/integration/{test-pool-acquire-timeout.test.js => test-pool-acquire-timeout.test.cjs} (86%) diff --git a/test/integration/promise-wrappers/test-promise-wrappers.test.cjs b/test/integration/promise-wrappers/test-promise-wrappers.test.cjs index 96053d091c..53d4118b0f 100644 --- a/test/integration/promise-wrappers/test-promise-wrappers.test.cjs +++ b/test/integration/promise-wrappers/test-promise-wrappers.test.cjs @@ -76,7 +76,6 @@ function testErrors() { } function testPoolAcquireTimeout() { - (async () => { const ACQUIRE_TIMEOUT = 500; const pool = createPool({ @@ -95,12 +94,9 @@ function testPoolAcquireTimeout() { const c3StartedAt = Date.now(); try { - await pool.getConnection(); assert.fail('should not reach here'); - } - catch (e) { - + } catch (e) { assert.equal(Date.now() - c3StartedAt >= ACQUIRE_TIMEOUT, true); assert.equal(e.message, 'Timeout acquiring connection'); } @@ -115,7 +111,6 @@ function testPoolAcquireTimeout() { await pool.end(); assert.ok(c4, 'acquireTimeout is working correctly'); - })().catch((err) => { assert.fail(err); }); diff --git a/test/integration/test-pool-acquire-timeout.test.js b/test/integration/test-pool-acquire-timeout.test.cjs similarity index 86% rename from test/integration/test-pool-acquire-timeout.test.js rename to test/integration/test-pool-acquire-timeout.test.cjs index 0fc5a17c9a..c4e89189c3 100644 --- a/test/integration/test-pool-acquire-timeout.test.js +++ b/test/integration/test-pool-acquire-timeout.test.cjs @@ -10,7 +10,7 @@ const ACQUIRE_TIMEOUT = 500; const pool = new mysql.createPool({ ...poolConfig, acquireTimeout: ACQUIRE_TIMEOUT, - connectionLimit: 2 + connectionLimit: 2, }); pool.getConnection((err, c1) => { @@ -29,7 +29,11 @@ pool.getConnection((err, c1) => { assert.equal(C3_DONE_AT - C3_STARTED_AT < ACQUIRE_TIMEOUT * 2, true); assert.notEqual(e3, null); - assert.equal(e3.message, 'Timeout acquiring connection', 'Acquire timeout error message is correct'); + assert.equal( + e3.message, + 'Timeout acquiring connection', + 'Acquire timeout error message is correct', + ); assert.equal(!c3, true); c1.release(); diff --git a/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts b/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts index bcd7cf39eb..508b08ebc4 100644 --- a/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts +++ b/test/tsc-build/mysql/createPool/callbacks/createPool.test.ts @@ -9,8 +9,4 @@ import { uriAccess, access } from '../../baseConnection.test.js'; uriPool = mysql.createPool(uriAccess); pool = mysql.createPool(access); - pool = mysql.createPool({ - ...access, - acquireTimeout: 1000, - }); })();