Min Pool is not respected

[SergeyCherman]

Issue Description In the sequelize configuration, I've set

pool: { min: 10, max: 30 }

When the connection is acquired it does not check if additional connections need to be made and hence only 1 connection is made instead of 10.

What do you expect to happen? I expect the min setting to be respected and 10 connections to be made.

Issue is a copy of: https://github.com/sequelize/sequelize/issues/11440 however I believe the issue is in sequelize-pool lib and not in the main sequelize project.

[sushantdhiman]

Sequelize pool will keep on increasing available resources to given min config, and will keep them at that level after pool saturation.

If application does not issue enough acquire calls, then pool will not grow to given min config. This is technically a bug, but I don't think this affects performance in any way. This means application is able to satisfy traffic with subset of resources.

What is the real motivation behind this change @SergeyCherman? Are you facing any performance issues?

[SergeyCherman]

@sushantdhiman Thank you for the quick reply! There were a couple reasons for this PR attempt:

1. I noticed perf degradations for the initial batch of queries being done as creating the connection took a little time. Since the connections will also expire after some time, it caused random cases where perf was worse then expected. I could be wrong but I assume it was due to connections being re-created.

2. I ran into an issue where my service was unable to make additional connections due to postgres max_connections config, which then caused other issues, perhaps the pool isn't handling that gracefully as I encountered errors (however I haven't tried that scenario with seq 5.x yet, only 4.x)

3. In my environment there are many apps creating database connections and it was possible that another app ate more then expected. I wanted to ensure my app would not start up (if available connections were below min) and be in an invalid state (from 2 above.) Due to this I was trying to set min/max to the same number and expected my service to either start and have all connections required, or not start and hence the deployment error out.

You said that application should issue acquire do you mean the users of sequelize should call acquire up to the min or the Sequelize library itself (currently Sequelize doesn't.) Also what would happen if connections expire and then drop below the min?

[sushantdhiman]

One point to keep in mind, this pool is not used with [email protected]. It was introduced with [email protected]

[SergeyCherman]

Sorry for the miscommunication, I meant that in 4.x (pre pool) min was respected and Sequelize would make the min configured connections on app start, in 5.x it looked like a regression as min connections weren't made.

Is there a different area where the fix should be made? I attempted to do it via: https://github.com/sequelize/sequelize-pool/pull/23

[marcogrcr]

If this behavior is not going to be changed, I believe the documentation should be updated:

https://github.com/sequelize/sequelize-pool/blob/066339cb92fbdd4a2775410733b25cc02ecbab7b/src/Pool.ts#L51-L57

Particularly,

When the pool is created, or a resource destroyed, this minimum will be checked.

[SergeyCherman]

@marcogrcr I'm sure this was a regression. I know @sushantdhiman had to depart the project, not sure how to get the discussion going with the new maintainers.

[bploetz]

Any update on this? Who's maintaining this repo now?

[sushantdhiman]

I have thought about this and #23, I think a new method to fill pool like fill | enrich etc can be introduced without breaking compatibility with current behavior. This method may be called when application starts, thus solving issue #23 is trying to solve.

I'll accept such PR if this idea sounds ok

[SergeyCherman]

@sushantdhiman yes I think that makes sense, I will add a new public method for fill to the pool class. Once I do that I will add an issue and a PR for updating the docs of the main sequelize branch.

I'll try and get it done this weekend.

[azai91]

Any update on this issue?

[SergeyCherman]

Sorry, I got tied up and forgot. I'll try and get a PR out this week.

[Shahor]

Any news? Or help we can provide?

[WikiRik]

Hi @SergeyCherman Not sure if you ever got to making a PR, but feel free to let us know if you can't anymore of require assistance with it 🙂

[ShaunB-SQ]

Was this ever done, as I feel I am experiencing a similar issue in sequelize 6.19.2.
As connections error out or disconnect new ones are not established, and I finally get a message

SequelizeConnectionError: Failed to connect to xxxx.xxxx.com:1433 in 15000ms
    at ConnectionManager.connect (/usr/src/app/server/node_modules/sequelize/lib/dialects/mssql/connection-manager.js:116:17)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async ConnectionManager._connect (/usr/src/app/server/node_modules/sequelize/lib/dialects/abstract/connection-manager.js:216:24)

even though in mssql and in ssms I am able to connect to the db just fine.

[vshubhansh]

Did we go around fixing this? I see that PR by @SergeyCherman was closed. I'm on v6.20 and getting same issue. This is impacting the performance of the first batch of queries that the application receives, which i'm hypothesising is due to new connections being opened. Is there any workaround to open a minimum number of connections when the application first comes up, rather than waiting for the load and then acquiring new connections for the pool?

[WikiRik]

I have thought about this and #23, I think a new method to fill pool like fill | enrich etc can be introduced without breaking compatibility with current behavior. This method may be called when application starts, thus solving issue #23 is trying to solve.

I'll accept such PR if this idea sounds ok

I believe that this message is still what the current sequelize maintainers can review.