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

Feature request: be able to change the Pool options "max" and "min" at runtime #55

Open
capaj opened this issue Sep 14, 2020 · 10 comments

Comments

@capaj
Copy link
Contributor

capaj commented Sep 14, 2020

I would like to use this lib with a Postgre on AWS aurora serverless. Problem is that aurora serverless scales up at runtime so I need to tweak the max connections value to be able to utilize serverless as it scales up. Is this possible with tarn.js?

Looking at the code it seems like theoretically it could work like this:

const p = new Pool({max: 90}) // one ACU
// when scaling occurs
p.max = 180 // two ACUs
p.check()

is this assumption correct?

@elhigu
Copy link
Collaborator

elhigu commented Sep 15, 2020

There is no planned / tested support for changing options after initialization.

Though that feature doesn't sound like really hard to implement especially if the downscaling is done not by forcing it, but just downscaling when connections are returned back to the pool.

If something like this is implemented, we would need to check out all options whose dynamic controls we would like to allow to be changed to have more complete feature if possible.

@springmeyer
Copy link

A potential workaround would be to have the max start at 180, immediately acquire 90 and then only release them back to the pool when scaling occurs.

@elhigu
Copy link
Collaborator

elhigu commented Sep 16, 2020

A potential workaround would be to have the max start at 180, immediately acquire 90 and then only release them back to the pool when scaling occurs.

Umm.... wouldn't that mean that then you have just acquired all the 90 connections and they are idling outside of pool and you would not have any extra connections from DB side to use?

Or is that 90 just self made limit that you would like to use limit concurrent queries? If there is a hard limit from DB or proxy side, then that shouldn't work.

Though one could actually kill those connections and not return them to pool and that could work as a placeholder that doesn't use DBs real connection resources.

@capaj
Copy link
Contributor Author

capaj commented Sep 16, 2020

Ok, seeing as it is not possible at the moment, let me rephrase this issue

@capaj capaj changed the title Is it possible to change the Pool options at runtime? Feature request: be able to change the Pool options "max" and "min" at runtime Sep 16, 2020
@elhigu
Copy link
Collaborator

elhigu commented Sep 16, 2020

I kind of see this being really useful feature.

I see there few options how this could be implemented:

  1. When downscaling is done, tarn will throw an error if there are more than (in this case) 90 resourcesin use.

This would be probably easiest and most robust way to implement this. From application side one would has to make sure before downscaling that it is a good moment to do it. For example temporarily to stop making queries and nothing unexpected should happen during the scaling.

  1. When downscaling stop giving new resources from pool until the lower limit has been reached.

This could cause user application to halt for example if there are many dependent transactions going on and there are no more resources to finish them. Though that kind of code would fail anyways with single pool at some point since the same problem would occur when pool is full. So probably it would just make app to stop creating new connections for a while.

  1. If more than max connection are leased call abort / destroy / anything to them and let the app to handled possible errors.

I really wouldn't like this option. I think it is a lot better to let user gracefully free the resources and handle the downscaling properly. User should have all the power to do it properly and not just force tarn to be a jerk.

I like the option 1 since it is easiest and most robust to implement on tarn.js side. There is no need to start thinking all the cases in current logic which might break because of this and good testing is also easiest to write for it. Maybe after that if it is not enough option 2 could be considered to be implemented afterwards.

@springmeyer
Copy link

Umm.... wouldn't that mean that then you have just acquired all the 90 connections and they are idling outside of pool and you would not have any extra connections from DB side to use?

Yes, the idea is that if you could predict the ultimate max you'd need but don't need that same max at startup, you could hold onto some of the connections until they are need, then release them back into the pool to allow all 180 to be used. But I agree that this is not ideal: just a poor workaround that I offered in case it might help others. I've been using it for one use-case where I want to have a high max, but can't at program startup since I need to throttle concurrency until the application warms up. But it would be more ideal to be able to change the max dynamically.

@newhouse
Copy link

I kind of see this being really useful feature.

I really think this could be a great feature, too. For my use case, it's not the DB connection capacity that's variable, but rather the number of instances of my application (and therefore clients with connection pools) that is variable.

For example, if my DB can only allow 100 connections but I've got a distributed and auto-scaling web application, then I'd like each instance to dynamically determine what its pool min/max should be at startup, and re-compute it (and adjust if necessary) on a regular interval. I'm not expecting this library to do all that, but to implement it smoothly I'd need the ability for an existing pool instance to have its max (and perhaps min) adjusted on-the-fly.

I don't think that I want to tear down an existing pool while starting up a new pool as that seems like it can lead to issues (like a period of time where the application is doing no work that requires a DB connection), and being able to adjust an existing pool just "feels" like where the solution should lay.

I like the option 1 since it is easiest and most robust to implement on tarn.js side. There is no need to start thinking all the cases in current logic which might break because of this and good testing is also easiest to write for it. Maybe after that if it is not enough option 2 could be considered to be implemented afterwards.

My gut feeling was that option 2 would be the preferred implementation method:

  1. If you go with option 1, and it's up to the application to know when it's a "good time" to decrease the pool max, it seems like it'd be possible for the application to never find a good time without halting requests or access to the connection pool. If the instance is under a decent amount of utilization, then it's possible that the number of in-use connections may stay greater than the new max forever if the pool (or the application) is not aware of the fact that you're trying to pare things down.
  2. If I'm not mistaken about needing to halt the application from giving new connections while the application waits for the numUsed() <= newMax condition, is that same behavior not already (mostly) implemented in the acquire() method?
  3. As you said: with option 2, it increases the chance that that the application experiences a TimeoutError (which in knex translates to a KnexTimeoutError) that it may otherwise not have, but that's just a possibility that the developer/application will have to be OK with if they're going to use this feature.

@newhouse
Copy link

@elhigu
It does not look to me like (as a workaround) that simply calling pool.max = newMax; pool.check() would do the trick, would you agree?

However - and not to trivialize the complexity of this library - it does seem like it's a small amount of changes to places like check(), _hasFreeResources(), _canAcquire(), _shouldCreateMoreResources, etc, to incorporate the this.max value in their computations that would "do the trick".

Perhaps it's an easy change for someone like you that's got the feel for it, and exposing the ability through a method (like update({ min: <integer>?, max: <integer>? })) and providing a warning/caveat in the docs would be an easy win?

@alvinlys
Copy link

@capaj Hi, im facing the same issue with you and may i know whats the current method you using?

@capaj
Copy link
Contributor Author

capaj commented Jun 16, 2023

@alvinlys I actually stopped using aurora. I use regular postgres with pgbouncer and that way I can basically open as many connections as I need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants