go-sql-driver
Context cancelling cause close connection without sending COMM_QUIT for authenticated connection
Issue description
startWatcher() watches the context provided by the Go client. When context cancellation occurs, this method only cleanup() the connection without sending COMM_QUIT, regardless of whether the connection has already been authenticated or not.
Without COMM_QUIT, MySQL Server will abort authenticated clients, and increase Aborted_clients metric.
Related code
-
Current handling when context cancel:
- https://github.com/go-sql-driver/mysql/blob/c9f41c074062d5ab9aeb5e44adeac3a7d85fbc4e/connection.go#L439-L441
-
But cleanup() is only called before auth or on auth failure
- https://github.com/go-sql-driver/mysql/blob/c9f41c074062d5ab9aeb5e44adeac3a7d85fbc4e/connection.go#L134-L138
-
Close() should be called if the connection has been authenticated successfully with sending COMM_QUIT to MySQL server
- https://github.com/go-sql-driver/mysql/blob/master/connection.go#L119
Error log
If you have an error log, please paste it here.
Configuration
Driver version (or git SHA): 1.8.1
Go version: run go version in your console
Server version: E.g. MySQL 5.6, MariaDB 10.0.20
Server OS: E.g. Debian 8.1 (Jessie), Windows 10
@methane Would you be able to confirm that this is the intended behavior? Is it a bug?
It is intended behavior, not a bug. I do not recommend using context cancel in regular case because it can not stop query. I recommend max_execution_time hint instead. https://dev.mysql.com/doc/refman/8.4/en/optimizer-hints.html
On the other hand, I do not against sending COM_QUIT on cancel. I will consider about it is really safe or not.
@methane thanks for reply
It is intended behavior, not a bug.
I think to properly close an authenticated connection, we need to send a COMM_QUIT packet to the MySQL server. This will prevent the MySQL server from treating the connection as aborted, which can negatively impact performance monitoring. (I spend a lot of time investigating frequent MySQL connection aborts.)
On the other hand, I do not against sending COM_QUIT on cancel. I will consider about it is really safe or not.
I agree that the SQL driver should send COMM_QUIT on cancel in a safe and reliable manner.
I do not recommend using context cancel in regular case because it can not stop query. I recommend max_execution_time hint instead. https://dev.mysql.com/doc/refman/8.4/en/optimizer-hints.html
Did you mean that the database query will run despite context cancel occurred?
What are the best practices for using context in queries, Would you be able to share any example, the case we should context or not.
Did you mean that the database query will run despite context cancel occurred?
Yes. MySQL protocol doesn't provide safe&robust way to cancel running query.
What are the best practices for using context in queries, Would you be able to share any example, the case we should context or not.
- Ensure context cancel doesn't happen in regular cases.
- set max_execution_time on connection variable or optimizer hint to prevent query running unintentionally long time.
I will add some option for context cancellation next year (e.g. ignore context cancel at all, safe for MariaDB, and not-safe & use it your own risk version). But I cannot promise it. My company doesn't need it at the moment so it is a my spare time project.