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

pool acquireTimeout - doesn't error when waiting #2105

Closed
thirteen-eleven opened this issue Sep 20, 2018 · 3 comments
Closed

pool acquireTimeout - doesn't error when waiting #2105

thirteen-eleven opened this issue Sep 20, 2018 · 3 comments
Assignees
Labels

Comments

@thirteen-eleven
Copy link

Perhaps, I don't understand the intent of acquireTimeout - or it has a bug.

I can set my pool to a small number (e.g. 2), and invoke (bad) code that does not properly release a connection. Once, I exhaust the connectionLimit, connections start queuing - but will wait indefinitely regardless of how low I specify the acquireTimeout (e.g. 1000ms).

I can force an error with lowering the queueLimit, or by setting waitForConnections: false, but what I really wanted was to only generate an error when it takes a long time to get a connection from the pool.

(I did see a feature request referring to a maxWait or queueWaitTimeout - but a comment said it was the same as acquireTimeout.)

Thanks for any assistance.

@dougwilson
Copy link
Member

Ah, yea, I think the docs can be improved there -- the acquireTimeout option does not have anything to do with waiting for a checked out connection to be released back to the pool -- there is no timeout option around that case currently.

The acquireTimeout comes with the "acquire" routine internally which basically is the process or either (a) opening a new connection to the server or (b) validating a free connection is still alive.

It sounds like two things from your report:
(a) improve docs around acquireTimeout to better describe what exactly the timeout is around (what I said is just an incomplete summary -- someone needs to validate that is accurate to land in docs)
(b) add a timeout option around requests that are in the queue to wait for a connection to be returned to the pool.

@thirteen-eleven
Copy link
Author

Thanks for the quick reply, @dougwilson .

Just confirmed your (a) above in Pool.js lines 42-48:

   if (this.config.connectionLimit === 0 || this._allConnections.length < this.config.connectionLimit) {
    connection = new PoolConnection(this, { config: this.config.newConnectionConfig() });

    this._acquiringConnections.push(connection);
    this._allConnections.push(connection);

    connection.connect({timeout: this.config.acquireTimeout}, function onConnect(err) {

... as the acquireTimeout only applies when the pool has at least one free connection.

(b) would be nice . 👍

@devMYC
Copy link

devMYC commented Apr 16, 2019

(b) add a timeout option around requests that are in the queue to wait for a connection to be returned to the pool.

+1 for this

@dougwilson dougwilson self-assigned this Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants