feat(pool): emit 'connectionQueueAcquired' event on connection dequeue fixes #3844

[shubhanshu2103]

This PR introduces a new event, 'connectionQueueAcquired', on the connection pool. This event fires whenever a waiting request is pulled from the internal queue (_connectionQueue) and successfully acquires a free connection.

This addresses the request for better observability of connection pool bottlenecks by providing real-time data on the queue state at the exact moment a waiting request is served.

🌟 Key Changes

New Event Emission in lib/base/pool.js:

The BasePool.releaseConnection() method is modified to emit 'connectionQueueAcquired' when this._connectionQueue.length is greater than 0, after shifting the waiting callback but before calling it.

Event Payload:

The event payload provides the essential information needed for diagnostics:

{
connection: connection, // The Connection object being handed to the request
queueDepth: this._connectionQueue.length // The actual size of the queue after the request was dequeued
}

New Integration Test:

A new integration test file, test/integration/test-pool-queue-event.js, was added to fully validate the feature.

The test saturates the pool (limit 2), queues two additional requests (depth 2), and then releases the active connections, asserting that the event fires exactly twice with the correct descending queueDepth values of 1 and 0.

Implementation Details

The implementation in releaseConnection is minimal and targeted to avoid side effects:

} else if (this._connectionQueue.length) {
cb = this._connectionQueue.shift();

// New Event: Fired when request acquires connection from queue
this.emit('connectionQueueAcquired', {
    connection: connection,
    queueDepth: this._connectionQueue.length 
});

process.nextTick(cb.bind(null, null, connection));

} else {
// ...

[shubhanshu2103]

@wellwelwel please let me know the improvements needed.

[wellwelwel]

Hey, @shubhanshu2103! I've suggested some ideas in the added test, but I'd suggest implementing the feature itself before moving on to the tests (or discussing the feature implementation in issue #3844) 🤝

[wellwelwel]

Are there a specific limitation or motivation for using Node.js assert in this test? For example:

const { assert, test } = require('poku');

[wellwelwel]

I recommend declaring functions/variables before they are used 🙋🏻‍♂️

[wellwelwel]

You can use the connection from const { createPool } = require('../common.test.cjs'), for example:

node-mysql2/test/integration/test-pool-end.test.cjs

Lines 3 to 6 in eba0c2b

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

[wellwelwel]

There is no done parameter in the tests, I think this is due to the callbacks. If you have any questions about how to wait the callbacks, please feel free to ask 🙋🏻‍♂️

[wellwelwel]

An interesting approach to testing is to use the test descriptions themselves to "comment" (describe) on what is happening. Not as a rule, but by reserving comments within the code for specific or complex behaviors.

You can, for example, combine describe and it to make it easier to describe the tests, as well as the messages in the assertions themselves 💡

[wellwelwel]

Note

I'm marking this PR as a draft until the feature is implemented 🚧