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

A connection in the pool that is closing due to ConnectionLifetime being exceeded should not result in a Warning message #586

Closed
koendhondt opened this issue Nov 21, 2018 · 11 comments

Comments

@koendhondt
Copy link

@koendhondt koendhondt commented Nov 21, 2018

I've noticed that if a connection is closed due to the Connection Lifetime being exceeded, it will result in a (cryptic) Warning message. IMO this should be an Information level message at best.

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Nov 21, 2018

Is this the message?

PoolX received invalid SessionN; destroying it

I agree that it doesn't describe the situation very well. And I suppose if the user has opted into ConnectionLifeTime != 0 they are deliberately choosing to reduce the benefits of pooling and don't need a warning.

I'm just curious: what's your reason for setting a ConnectionLifeTime?

@koendhondt
Copy link
Author

@koendhondt koendhondt commented Nov 22, 2018

Correct, that is the message.

The reason we are using ConnectionLifetime is that we have technical limitations on the number of connections the server can handle. The server uses a wait_timeout to kill idle connections (if we don't, we get 'too many connection' errors). The clients (the number of which is small but not fixed) are each configured with a small connection pool and work in bursts.

Note that we do not need to explicitly set a Connection Lifetime, we are trying to get rid of the "PoolX received invalid SessionN" warning message in our logs (which arose due to the server killing connections after wait_timeout expired), and we figured that by setting a Connection Lifetime, the connector would silently close connections for which the lifetime was exceeded.

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Nov 22, 2018

Have you tried using MinimumPoolSize=0 (this is the default, but set it back to 0 if you changed it) and setting ConnectionIdleTimeout to a lower value, maybe 60 (seconds)? This will cause the client to prune idle connections every minute, which should gracefully clean up old connections and not require wait_timeout on the server to kill them.

@koendhondt
Copy link
Author

@koendhondt koendhondt commented Nov 22, 2018

That is a good suggestion; I'll try that and let you know.

@koendhondt
Copy link
Author

@koendhondt koendhondt commented Nov 29, 2018

Update: the (many) invalid SessionN warning messages are now gone, but unfortunately we now see the following warning messages in the log:

"Pool{MySqlPool} is empty; recovering leaked sessions"

Would it make sense to use a minPoolSize of 1 to eliminate these? Or would this connection also be terminated by the IdleTimeout, followed by a subsequent reconnect?

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Nov 29, 2018

Increasing MinimumPoolSize won't help with this message.

There are two possible causes for this:

  1. The pool is empty because all available connections are in use. Increase MaximumPoolSize to the number of concurrent database connections you expect to be used in normal operations. Otherwise, your application will "block" waiting for a database connection to become available, decreasing throughput.

  2. The pool is empty because inactive connections are not being returned to it. Make sure every MySqlConnection object is Closed or Disposed so it's returned to the pool. In typical code, every MySqlConnection should be in a using block, i.e., using (var connection = new MySqlConnection(connectionString)) { ... }.

(I suspect no. 2 isn't the problem for you because you wouldn't have seen the message "PoolX received invalid SessionN; destroying it" if you weren't closing connections, and because you didn't report seeing any "RecoveredSessionCount=N" messages.)

That said, this message doesn't really need to be a warning, so its severity should be reduced to Info, I think.

@marcrocny
Copy link
Contributor

@marcrocny marcrocny commented Nov 30, 2018

Agreed. I use PoolSize as a backstop in situations where directly controlling concurrence is prohibitive. It works well, but those “warnings” can get annoying.

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Nov 30, 2018

Reduced severity of the "Pool is empty" message in b2d1c0e.

@koendhondt
Copy link
Author

@koendhondt koendhondt commented Dec 3, 2018

Thanks!

Administrative question: should I close this issue?

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Dec 3, 2018

No need; GitHub will automatically close it when the commit is pushed to master.

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Dec 8, 2018

Fixed in 0.48.0.

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

No branches or pull requests

3 participants