From 21abf3790078756d2a09079d93369beaa82befee Mon Sep 17 00:00:00 2001 From: dimov Date: Mon, 4 Nov 2024 12:10:13 +0200 Subject: [PATCH 1/3] fix: gracefully close pool connections sidorares#3148 --- lib/base/pool.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base/pool.js b/lib/base/pool.js index 47981f1943..019c5f4176 100644 --- a/lib/base/pool.js +++ b/lib/base/pool.js @@ -200,7 +200,7 @@ class BasePool extends EventEmitter { Date.now() - this._freeConnections.get(0).lastActiveTime > this.config.idleTimeout) ) { - this._freeConnections.get(0).destroy(); + this._freeConnections.get(0)._realEnd(); } } finally { this._removeIdleTimeoutConnections(); From c396bd8da8288765843e4b2829a1232de73fc0db Mon Sep 17 00:00:00 2001 From: dimov Date: Mon, 4 Nov 2024 15:21:26 +0200 Subject: [PATCH 2/3] fix: implement pool_connection end --- lib/base/pool.js | 4 ++-- lib/base/pool_connection.js | 16 +++------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/lib/base/pool.js b/lib/base/pool.js index 019c5f4176..3086f0d8b2 100644 --- a/lib/base/pool.js +++ b/lib/base/pool.js @@ -121,7 +121,7 @@ class BasePool extends EventEmitter { } for (let i = 0; i < this._allConnections.length; i++) { connection = this._allConnections.get(i); - connection._realEnd(endCB); + connection.end(endCB); } } @@ -200,7 +200,7 @@ class BasePool extends EventEmitter { Date.now() - this._freeConnections.get(0).lastActiveTime > this.config.idleTimeout) ) { - this._freeConnections.get(0)._realEnd(); + this._freeConnections.get(0).end(); } } finally { this._removeIdleTimeoutConnections(); diff --git a/lib/base/pool_connection.js b/lib/base/pool_connection.js index 74511ce587..5bba18a449 100644 --- a/lib/base/pool_connection.js +++ b/lib/base/pool_connection.js @@ -29,16 +29,9 @@ class BasePoolConnection extends BaseConnection { this._pool.releaseConnection(this); } - end() { - const err = new Error( - 'Calling conn.end() to release a pooled connection is ' + - 'deprecated. In next version calling conn.end() will be ' + - 'restored to default conn.end() behavior. Use ' + - 'conn.release() instead.', - ); - this.emit('warn', err); - console.warn(err.message); - this.release(); + end(callback) { + this._removeFromPool(); + super.end(callback); } destroy() { @@ -58,6 +51,3 @@ class BasePoolConnection extends BaseConnection { BasePoolConnection.statementKey = BaseConnection.statementKey; module.exports = BasePoolConnection; - -// TODO: Remove this when we are removing PoolConnection#end -BasePoolConnection.prototype._realEnd = BaseConnection.prototype.end; From 9201470af198c76fea3d9e1f1dcf9771ce57e0a7 Mon Sep 17 00:00:00 2001 From: dimov Date: Tue, 5 Nov 2024 23:48:57 +0200 Subject: [PATCH 3/3] test: add test for sidorares/node-mysql2/issues/3148 --- ...l-releases-connections-gracefully.test.cjs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/test-pool-releases-connections-gracefully.test.cjs diff --git a/test/test-pool-releases-connections-gracefully.test.cjs b/test/test-pool-releases-connections-gracefully.test.cjs new file mode 100644 index 0000000000..2d5845d747 --- /dev/null +++ b/test/test-pool-releases-connections-gracefully.test.cjs @@ -0,0 +1,41 @@ +'use strict'; +const createPool = require('../common.test.cjs').createPool; +const { assert } = require('poku'); + +/** + * This test case tests that the pool releases connections gracefully after the idle timeout has passed. + * + * @see https://github.com/sidorares/node-mysql2/issues/3148 + */ + +const pool = new createPool({ + connectionLimit: 3, + maxIdle: 2, + idleTimeout: 1000, +}); + +let connection1Ended = false; +let connection2Ended = false; +let connection3Ended = false; + +pool.getConnection((_err1, connection1) => { + pool.getConnection((_err2, connection2) => { + pool.getConnection((_err3, connection3) => { + connection1.stream.on('end', () => (connection1Ended = true)); + connection2.stream.on('end', () => (connection2Ended = true)); + connection3.stream.on('end', () => (connection3Ended = true)); + + connection1.release(); + connection2.release(); + connection3.release(); + + setTimeout(() => { + assert(connection1Ended, 'connection1 should have ended'); + assert(connection2Ended, 'connection2 should have ended'); + assert(connection3Ended, 'connection3 should have ended'); + + pool.end(); + }, 2000); + }); + }); +});