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

Connection clean-up does not reset session system variables #240

Closed
demurgos opened this issue Mar 21, 2023 · 4 comments
Closed

Connection clean-up does not reset session system variables #240

demurgos opened this issue Mar 21, 2023 · 4 comments

Comments

@demurgos
Copy link

demurgos commented Mar 21, 2023

When calling get_conn on a pool, there's a risk to get a connection with session system variables in an unexpected state.
(For example variables defined like SET session innodb_lock_wait_timeout = 0;)

Both MariaDB and MySQL have support to reset the connection state, and this library exposes it as Conn::reset. However, it's required to call it manually every single time to avoid leaking state from some recycled connection.

I believe that the connection state should be reset by default in Conn::cleanup_for_pool, with potentially an option to opt-out of this behavior. Having resets enables by default is a better choice in my opinion to get predictable results.

@cloneable
Copy link
Contributor

134cbf8 adds implicit non-configurable connection reset when it's returned to the pool. This kills the prepared statement cache, degrading performance, and also the conn init statements are not run after a reset, so default session settings are not kept.

@blackbeam
Copy link
Owner

@cloneable, please note, that 134cbf8 does not enforce reset for clean connections because of this condition. I would say it's broken.

I agree with @demurgos that we should reset the connection state by default, actually it might be dangerous not to do so (consider something like unique_checks).

The problem, I think, that there is no way to opt-out, so I have to add one (actually, this is the reason for a new boolean argument on GetConn::new).

and also the conn init statements are not run after a reset

This is a bug.

@cloneable
Copy link
Contributor

Ok, thanks! It's very unfortunate that COM_RESET_CONNECTION clears prepared statements. I wish they would offer a reset that keeps the statement cache.

@blackbeam
Copy link
Owner

Fixed in v0.32.1

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

3 participants