sequelize / sequelize-pool

Resource pool implementation. It can be used to throttle expensive resources.
Other
39 stars 17 forks source link

Custom keepalive on idle connections protected due to minimum pool size #55

Open brandonw opened 4 months ago

brandonw commented 4 months ago

Greetings!

While diagnosing some db issues, we realized there may be a feature that could help mitigate or remove aborted connections.

The current state of our setup is:

On the surface, one would think there should never be any aborted clients because the pool idle timeout is lower than the db connection idle timeout. However, this is not the case because the pool (rightly) does not reap idle connections once you have reaped down to the pool minimum size.

This means that in periods of low activity, the db will end up aborting the pool idle connections because they are not being used but the pool is protecting them from reaping.

With a goal of reducing the number of db aborted clients to zero, I wonder if it is possible to allow some configuration in [1]. The goal being something like:

  1. removeIdle triggers
  2. all idle connections down to min pool size are reaped as normal
  3. all idle connections below the min pool size allow a config option to be called with that resource to basically tell the resource "do what you need to do in order to keep yourself healthy"

In my specific case using mysql2, this would probably be something like pooledObject.resource.ping().

The end result would be that even in periods of low traffic, the pooled connections below the min threshold would keep themselves healthy by continually resetting the idle timeout on the server so that they are not closed due to lack of usage.

Would a feature like that be useful or accepted? If so, I may be able to work on a PR.

[1] https://github.com/sequelize/sequelize-pool/blob/da269d7d8ca2763e87fc324beddfc6523d71167c/src/Pool.ts#L263-L277

brandonw commented 4 months ago

An added difficulty here would be that the operation of telling the server that you are healthy likely means you have to effectively "acquire" the resource so that you can do whatever the resource needs to do to remain healthy. Otherwise, the ping() could be mid-execution while another client tries to acquire that resource while it is technically not usable.

sushantdhiman commented 4 months ago

Another alternative for keeping the min pooled resources healthy is to use time based maxUses, as discussed in #51. Will such an implementation suit your needs?

brandonw commented 4 months ago

In my specific case, I don't think that would help, since in this context there are no uses at all, so the resource would still be maintained while idle and not recycled.

I am trying out a min pool size of zero and seeing if there is not any adverse effect on request latency. Realistically, the spikes we get are not very short where having a min pool size would be most useful. They are relatively longer lived (at least into the minute or more than a minute) where there will be enough time to ramp up the connection pool and persist all the resources as they will be in constant use.

sushantdhiman commented 4 months ago

To perform a health check on all available resources, we will need a separate queue here.

https://github.com/sequelize/sequelize-pool/blob/da269d7d8ca2763e87fc324beddfc6523d71167c/src/Pool.ts#L115-L119

Then, at regular interval,

I don't think I want this in the main code-base. It will introduce unnecessary complexity.

For your use case :- At the application level, you can keep acquiring connections one by one, and perform a health check on them.