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

feat(pool): Add Minimum connections, and auto close extra connections #736

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aikar
Copy link
Contributor

@aikar aikar commented Feb 28, 2018

This adds a new configuration for desired minimum connections, to
complement the connection limit.

If the autoOpenConnections is enabled (default true), the pool will
immediately open the desired minimum number of connections.

Any connection opened above the minimum, who sits unused for a time
specified in idleTimeout, will be automatically closed.

This allows you to set up a minimum expectation of standard load for
connections to your database, but also allow you to configure a
higher maximum to handle unexpected burst traffic, and be able to
close them automatically when the burst subsides.

This adds a new configuration for desired minimum connections, to
complement the connection limit.

If the autoOpenConnections is enabled (default true), the pool will
immediately open the desired minimum number of connections.

Any connection opened above the minimum, who sits unused for a time
specified in idleTimeout, will be automatically closed.

This allows you to set up a minimum expectation of standard load for
connections to your database, but also allow you to configure a
higher maximum to handle unexpected burst traffic, and be able to
close them automatically when the burst subsides.
@aikar
Copy link
Contributor Author

aikar commented Feb 28, 2018

Github not letting me reopen the previous branch struck yet again (no github you will not change my workflow!)

Given that the memory leak bug is not an issue with node-mysql2, we should be good here now.

Since the last PR, I did discover (in relation to #732) that errors triggered during the initial connection such as setting the timezone, did not bubble back up correctly.

The logic of handling errors upon connection in the auto opened connections has been resolved, by bubbling the error to a pending connection.

I'm not sure what else we could do if theres an error with no pending connections, given that there is no error logging system officially.

I think my next PR will be to configure a general logger interface like I have here: aikar@b7d624c

But let there be debug/info/error levels.

@aikar
Copy link
Contributor Author

aikar commented Mar 6, 2018

@sidorares confirming that the memory leak fear is completely gone and was related to my own use of URL.

I've been running this for many days now with stable memory.

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

Successfully merging this pull request may close these issues.

None yet

1 participant