Skip to content

Commit

Permalink
[CONJS-200] Improve pool connection error messaging
Browse files Browse the repository at this point in the history
  • Loading branch information
rusher committed Jun 1, 2022
1 parent 6117b64 commit 268111d
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 25 deletions.
2 changes: 2 additions & 0 deletions documentation/callback-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ Specific options for pools are :
| **`acquireTimeout`** | Timeout to get a new connection from pool in ms. |*integer* | 10000 |
| **`connectionLimit`** | Maximum number of connection in pool. |*integer* | 10 |
| **`idleTimeout`** | Indicate idle time after which a pool connection is released. Value must be lower than [@@wait_timeout](https://mariadb.com/kb/en/library/server-system-variables/#wait_timeout). In seconds (0 means never release) |*integer* | 1800 |
| **`initializationTimeout`** | Pool will retry creating connection in loop, emitting 'error' event when reaching this timeout. In milliseconds |*integer* | 30000 |
| **`minimumIdle`** | Permit to set a minimum number of connection in pool. **Recommendation is to use fixed pool, so not setting this value**.|*integer* | *set to connectionLimit value* |
| **`minDelayValidation`** | When asking a connection to pool, the pool will validate the connection state. "minDelayValidation" permits disabling this validation if the connection has been borrowed recently avoiding useless verifications in case of frequent reuse of connections. 0 means validation is done each time the connection is asked. (in ms) |*integer*| 500|
| **`noControlAfterUse`** | After giving back connection to pool (connection.end) connector will reset or rollback connection to ensure a valid state. This option permit to disable those controls|*boolean*| false|
Expand All @@ -345,6 +346,7 @@ Specific options for pools are :
| **`connection`** | This event is emitted when a new connection is added to the pool. Has a connection object parameter |
| **`enqueue`** | This event is emitted when a command cannot be satisfied immediately by the pool and is queued. |
| **`release`** | This event is emitted when a connection is released back into the pool. Has a connection object parameter|
| **`error`** | When pool fails to create new connection after reaching `initializationTimeout` timeout |

**Example:**

Expand Down
2 changes: 2 additions & 0 deletions documentation/promise-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ Specific options for pools are :
| **`acquireTimeout`** | Timeout to get a new connection from pool in ms. In order to have connection error information, must be higher than connectTimeout |*integer* | 10000 |
| **`connectionLimit`** | Maximum number of connection in pool. |*integer* | 10 |
| **`idleTimeout`** | Indicate idle time after which a pool connection is released. Value must be lower than [@@wait_timeout](https://mariadb.com/kb/en/library/server-system-variables/#wait_timeout). In seconds (0 means never release) |*integer* | 1800 |
| **`initializationTimeout`** | Pool will retry creating connection in loop, emitting 'error' event when reaching this timeout. In milliseconds |*integer* | 30000 |
| **`minimumIdle`** | Permit to set a minimum number of connection in pool. **Recommendation is to use fixed pool, so not setting this value**.|*integer* | *set to connectionLimit value* |
| **`minDelayValidation`** | When asking a connection to pool, the pool will validate the connection state. "minDelayValidation" permits disabling this validation if the connection has been borrowed recently avoiding useless verifications in case of frequent reuse of connections. 0 means validation is done each time the connection is asked. (in ms) |*integer*| 500|
| **`noControlAfterUse`** | After giving back connection to pool (connection.end) connector will reset or rollback connection to ensure a valid state. This option permit to disable those controls|*boolean*| false|
Expand Down Expand Up @@ -1362,6 +1363,7 @@ This is an alias for [`connection.escapeId(value) → String`](#connectionescape
| **`connection`** | This event is emitted when a new connection is added to the pool. Has a connection object parameter |
| **`enqueue`** | This event is emitted when a command cannot be satisfied immediately by the pool and is queued. |
| **`release`** | This event is emitted when a connection is released back into the pool. Has a connection object parameter|
| **`error`** | When pool fails to create new connection after reaching `initializationTimeout` timeout |

**Example:**

Expand Down
1 change: 1 addition & 0 deletions lib/pool-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class PoolCallback extends EventEmitter {
this.#pool.on('connection', this.emit.bind(this, 'connection'));
this.#pool.on('enqueue', this.emit.bind(this, 'enqueue'));
this.#pool.on('release', this.emit.bind(this, 'release'));
this.#pool.on('error', this.emit.bind(this, 'error'));
}

#noop = () => {};
Expand Down
1 change: 1 addition & 0 deletions lib/pool-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class PoolPromise extends EventEmitter {
this.#pool.on('connection', this.emit.bind(this, 'connection'));
this.#pool.on('enqueue', this.emit.bind(this, 'enqueue'));
this.#pool.on('release', this.emit.bind(this, 'release'));
this.#pool.on('error', this.emit.bind(this, 'error'));
}

get closed() {
Expand Down
59 changes: 34 additions & 25 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ class Pool extends EventEmitter {
#opts;
#closed = false;
#connectionInCreation = false;
#errorCreatingConnection = null;
#idleConnections = new Queue();
#activeConnections = {};
#requests = new Queue();
#unusedConnectionRemoverId;
#requestTimeoutId;
#connErrorNumber = 0;
#initialized = false;
_sizeHandlerTimeout;

constructor(options) {
Expand Down Expand Up @@ -231,6 +233,8 @@ class Pool extends EventEmitter {
this._doCreateConnection(resolve, reject, timeoutEnd);
})
.then(() => {
this.#initialized = true;
this.#errorCreatingConnection = null;
this.#connErrorNumber = 0;
if (this._shouldCreateMoreConnections()) {
this.emit('validateSize');
Expand All @@ -239,25 +243,26 @@ class Pool extends EventEmitter {
})
.catch((err) => {
this.#connectionInCreation = false;
if (this.totalConnections() === 0) {
const task = this.#requests.shift();
if (task) {
this._rejectTask(task, err);
if (!this.#closed) {
if (!this.#initialized) {
err.message = 'Error during pool initialization: ' + err.message;
} else {
err.message = 'Pool fails to create connection: ' + err.message;
}
} else if (!this.#closed) {
console.error(`pool fail to create connection (${err.message})`);
this.#errorCreatingConnection = err;
this.emit('error', err);

//delay next try
this._sizeHandlerTimeout = setTimeout(
function () {
this._sizeHandlerTimeout = null;
if (!this.#requests.isEmpty()) {
this._sizeHandler();
}
}.bind(this),
Math.min(++this.#connErrorNumber * 500, 10000)
);
}

//delay next try
this._sizeHandlerTimeout = setTimeout(
function () {
this._sizeHandlerTimeout = null;
if (!this.#requests.isEmpty()) {
this._sizeHandler();
}
}.bind(this),
Math.min(++this.#connErrorNumber * 500, 10000)
);
});
}.bind(this)
);
Expand Down Expand Up @@ -330,14 +335,18 @@ class Pool extends EventEmitter {
while ((request = this.#requests.peekFront())) {
if (request.timeout <= currTime) {
this.#requests.shift();
request.reject(
Errors.createError(
`retrieve connection from pool timeout after ${Math.abs(
Date.now() - (request.timeout - this.#opts.acquireTimeout)
)}ms`,
Errors.ER_GET_CONNECTION_TIMEOUT
)
);
// if no connection and connections fails with error, throw connection error,
// else just a timeout error indicating that no connection was available until reaching timeout
let err =
this.activeConnections() == 0 && this.#errorCreatingConnection
? this.#errorCreatingConnection
: Errors.createError(
`retrieve connection from pool timeout after ${Math.abs(
Date.now() - (request.timeout - this.#opts.acquireTimeout)
)}ms`,
Errors.ER_GET_CONNECTION_TIMEOUT
);
request.reject(err);
} else {
this.#requestTimeoutId = setTimeout(this._requestTimeoutHandler.bind(this), request.timeout - currTime);
return;
Expand Down
50 changes: 50 additions & 0 deletions test/integration/test-pool-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('Pool callback', () => {

it('pool with wrong authentication', function (done) {
this.timeout(10000);
const initTime = Date.now();
const pool = base.createPoolCallback({
acquireTimeout: 4000,
initializationTimeout: 2000,
Expand All @@ -21,6 +22,7 @@ describe('Pool callback', () => {
if (!err) {
done(new Error('must have thrown error'));
} else {
assert(Date.now() - initTime >= 3980, 'expected > 4s, but was ' + (Date.now() - initTime));
pool.query('SELECT 3', (err) => {
if (!err) {
done(new Error('must have thrown error'));
Expand All @@ -44,6 +46,54 @@ describe('Pool callback', () => {
if (!err) {
done(new Error('must have thrown error'));
}
assert(Date.now() - initTime >= 3980, 'expected > 4s, but was ' + (Date.now() - initTime));
});
});

it('pool error event', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql-ha') this.skip(); //to avoid host being blocked
this.timeout(10000);
const pool = base.createPoolCallback({
acquireTimeout: 4000,
initializationTimeout: 2000,
user: 'wrongAuthentication'
});

await new Promise(function (resolver, rejecter) {
pool.on('error', (err) => {
assert.isTrue(err.message.includes('Error during pool initialization:'));
assert.isTrue(
err.errno === 1524 ||
err.errno === 1045 ||
err.errno === 1698 ||
err.errno === 45028 ||
err.errno === 45025 ||
err.errno === 45044,
err.message
);
pool.end();
resolver();
});
});
});

it('pool error fail connection', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql-ha') this.skip(); //to avoid host being blocked
this.timeout(10000);
const initTime = Date.now();
const pool = base.createPoolCallback({
acquireTimeout: 4000,
initializationTimeout: 2000,
host: 'wronghost'
});

await new Promise(function (resolver, rejecter) {
pool.on('error', (err) => {
assert(Date.now() - initTime >= 1980, 'expected > 2s, but was ' + (Date.now() - initTime));
assert.isTrue(err.message.includes('Error during pool initialization:'));
pool.end();
resolver();
});
});
});

Expand Down
55 changes: 55 additions & 0 deletions test/integration/test-pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,21 @@ describe('Pool', () => {
it('pool with wrong authentication', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql-ha') this.skip(); //to avoid host being blocked
this.timeout(10000);
const initTime = Date.now();
const pool = base.createPool({
acquireTimeout: 4000,
initializationTimeout: 2000,
user: 'wrongAuthentication'
});

setTimeout(async () => {
try {
await pool.query('SELECT 2');
pool.end();
throw new Error('must have thrown error');
} catch (err) {
assert(Date.now() - initTime >= 3980, 'expected > 4s, but was ' + (Date.now() - initTime));
assert.isTrue(err.message.includes('Error during pool initialization:'));
assert.isTrue(
err.errno === 1524 ||
err.errno === 1045 ||
Expand All @@ -214,6 +218,8 @@ describe('Pool', () => {
await pool.end();
throw new Error('must have thrown error');
} catch (err) {
assert(Date.now() - initTime >= 3980, 'expected > 4s, but was ' + (Date.now() - initTime));
assert.isTrue(err.message.includes('Error during pool initialization:'));
assert.isTrue(
err.errno === 1524 ||
err.errno === 1045 ||
Expand All @@ -227,6 +233,8 @@ describe('Pool', () => {
await pool.query('SELECT 3');
throw new Error('must have thrown error');
} catch (err) {
assert(Date.now() - initTime >= 3980, 'expected > 4s, but was ' + (Date.now() - initTime));
assert.isTrue(err.message.includes('Error during pool initialization:'));
assert.isTrue(
err.errno === 1524 ||
err.errno === 1045 ||
Expand All @@ -242,6 +250,53 @@ describe('Pool', () => {
}
});

it('pool error event', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql-ha') this.skip(); //to avoid host being blocked
this.timeout(10000);
const pool = base.createPool({
acquireTimeout: 4000,
initializationTimeout: 2000,
user: 'wrongAuthentication'
});

await new Promise(function (resolver, rejecter) {
pool.on('error', (err) => {
assert.isTrue(err.message.includes('Error during pool initialization:'));
assert.isTrue(
err.errno === 1524 ||
err.errno === 1045 ||
err.errno === 1698 ||
err.errno === 45028 ||
err.errno === 45025 ||
err.errno === 45044,
err.message
);
pool.end();
resolver();
});
});
});

it('pool error fail connection', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql-ha') this.skip(); //to avoid host being blocked
this.timeout(10000);
const initTime = Date.now();
const pool = base.createPool({
acquireTimeout: 4000,
initializationTimeout: 2000,
host: 'wronghost'
});

await new Promise(function (resolver, rejecter) {
pool.on('error', (err) => {
assert(Date.now() - initTime >= 1980, 'expected > 2s, but was ' + (Date.now() - initTime));
assert.isTrue(err.message.includes('Error during pool initialization:'));
pool.end();
resolver();
});
});
});

it('pool with wrong authentication connection', async function () {
if (process.env.srv === 'maxscale' || process.env.srv === 'skysql' || process.env.srv === 'skysql-ha') this.skip();
this.timeout(10000);
Expand Down

0 comments on commit 268111d

Please sign in to comment.