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

Updated connection cleanup process to handle expired connections and those exceeding config.maxIdle #3022

Merged
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
7 changes: 4 additions & 3 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,10 @@ class Pool extends EventEmitter {
this._removeIdleTimeoutConnectionsTimer = setTimeout(() => {
try {
while (
this._freeConnections.length > this.config.maxIdle &&
Date.now() - this._freeConnections.get(0).lastActiveTime >
this.config.idleTimeout
this._freeConnections.length > this.config.maxIdle ||
(this._freeConnections.length > 0 &&
Date.now() - this._freeConnections.get(0).lastActiveTime >
this.config.idleTimeout)
) {
this._freeConnections.get(0).destroy();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
'use strict';
const createPool = require('../common.test.cjs').createPool;
const { assert } = require('poku');

/**
* This test case tests the scenario where released connections are not
* being destroyed after the idle timeout has passed, to do this we setup
* a pool with a connection limit of 3, and a max idle of 2, and an idle
* timeout of 1 second. We then create 3 connections, and release them
* after 1 second, we then check that the pool has 0 connections, and 0
* free connections.
*
* @see https://github.com/sidorares/node-mysql2/issues/3020
*/

/**
* This test case
*/
const pool = new createPool({
connectionLimit: 3,
maxIdle: 2,
idleTimeout: 1000,
});

/**
* Create the first connection and ensure it's in the pool as expected
*/
pool.getConnection((err1, connection1) => {
assert.ifError(err1);
assert.ok(connection1);

/**
* Create the second connection and ensure it's in the pool as expected
*/
pool.getConnection((err2, connection2) => {
assert.ifError(err2);
assert.ok(connection2);

/**
* Create the third connection and ensure it's in the pool as expected
*/
pool.getConnection((err3, connection3) => {
assert.ifError(err3);
assert.ok(connection3);

/**
* Release all the connections
*/
connection1.release();
connection2.release();
connection3.release();

/**
* After the idle timeout has passed, check that all items in the in the pool
* that have been released are destroyed as expected.
*/
setTimeout(() => {
assert(
pool._allConnections.length === 0,
`Expected all connections to be closed, but found ${pool._allConnections.length}`,
);
assert(
pool._freeConnections.length === 0,
`Expected all free connections to be closed, but found ${pool._freeConnections.length}`,
);

pool.end();
}, 5000);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const createPool = require('../common.test.cjs').createPool;
const { assert } = require('poku');

const pool = new createPool({
connectionLimit: 3, // 5 connections
maxIdle: 1, // 1 idle connection
idleTimeout: 1000, // remove idle connections after 1 second
});

pool.getConnection((err1, connection1) => {
assert.ifError(err1);
assert.ok(connection1);
pool.getConnection((err2, connection2) => {
assert.ifError(err2);
assert.ok(connection2);
assert.notStrictEqual(connection1, connection2);
pool.getConnection((err3, connection3) => {
assert.ifError(err3);
assert.ok(connection3);
assert.notStrictEqual(connection1, connection3);
assert.notStrictEqual(connection2, connection3);
connection1.release();
connection2.release();
connection3.release();
assert(pool._allConnections.length === 3);
assert(pool._freeConnections.length === 3);
// after two seconds, the above 3 connection should have been destroyed
setTimeout(() => {
assert(pool._allConnections.length === 0);
assert(pool._freeConnections.length === 0);
// Creating a new connection should create a fresh one
pool.getConnection((err4, connection4) => {
assert.ifError(err4);
assert.ok(connection4);
assert(pool._allConnections.length === 1);
assert(pool._freeConnections.length === 0);
connection4.release();
connection4.destroy();
pool.end();
});
}, 2000);
});
});
});
4 changes: 3 additions & 1 deletion test/integration/test-pool-release-idle-connection.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ pool.getConnection((err1, connection1) => {
connection4.destroy();
pool.end();
});
}, 7000);
// Setting the time to a lower value than idleTimeout will ensure that the connection is not considered idle
// during our assertions
}, 4000);
});
});
});
Loading