Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pool): added acquireTimeout supports for pool #1919

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/connection_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ const validOptions = {
idleTimeout: 1,
Promise: 1,
queueLimit: 1,
waitForConnections: 1
waitForConnections: 1,
acquireTimeout: 1
};

class ConnectionConfig {
Expand Down
27 changes: 23 additions & 4 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
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.'));
}
Expand All @@ -78,6 +78,24 @@
return cb(new Error('Queue limit reached.'));
}
this.emit('enqueue');

if (this.config.acquireTimeout > 0) {
// eslint-disable-next-line prefer-const
let timer;

const wrappedCb = (err, connection) => {
clearTimeout(timer);
cb(err, connection);
};

timer = setTimeout(() => {
spliceConnection(this._connectionQueue, wrappedCb);
cb(new Error('Timeout acquiring connection'));
}, this.config.acquireTimeout);

return this._connectionQueue.push(wrappedCb);
}

return this._connectionQueue.push(cb);
}

Expand All @@ -102,7 +120,7 @@
this._closed = true;
clearTimeout(this._removeIdleTimeoutConnectionsTimer);
if (typeof cb !== 'function') {
cb = function(err) {
cb = function (err) {
if (err) {
throw err;
}
Expand All @@ -111,7 +129,7 @@
let calledBack = false;
let closedConnections = 0;
let connection;
const endCB = function(err) {
const endCB = function (err) {
if (calledBack) {
return;
}
Expand Down Expand Up @@ -139,7 +157,8 @@
this.config.connectionConfig
);
if (typeof cmdQuery.namedPlaceholders === 'undefined') {
cmdQuery.namedPlaceholders = this.config.connectionConfig.namedPlaceholders;
cmdQuery.namedPlaceholders =
this.config.connectionConfig.namedPlaceholders;

Check warning on line 161 in lib/pool.js

View check run for this annotation

Codecov / codecov/patch

lib/pool.js#L160-L161

Added lines #L160 - L161 were not covered by tests
}
this.getConnection((err, conn) => {
if (err) {
Expand Down
3 changes: 3 additions & 0 deletions lib/pool_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
42 changes: 42 additions & 0 deletions test/integration/promise-wrappers/test-promise-wrappers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,47 @@ 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)
Expand Down Expand Up @@ -473,6 +514,7 @@ testChangeUser();
testConnectionProperties();
testPoolConnectionDestroy();
testPromiseLibrary();
testPoolAcquireTimeout();

process.on('exit', () => {
assert.equal(doneCalled, true, 'done not called');
Expand Down
50 changes: 50 additions & 0 deletions test/integration/test-pool-acquire-timeout.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'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();
});
});
});
});
7 changes: 7 additions & 0 deletions typings/mysql/lib/Pool.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
*/
Expand Down
Loading