Handle being terminated while reading query result #811
Conversation
The problem is that if execution is terminated while reading the result set, the next attempt to execute a query on the connection fails with "This connection is still waiting for a result, try again once you have the result". This error persists even if reconnect is enabled. The issue stems from reading the result set using rb_thread_call_without_gvl. If the calling thread is terminated inside rb_thread_call_without_gvl, the function does not return and the caller does not get a chance to reset the active thread associated with the connection - which normally happens after the result set has been read. The solution is to disconnect and mark the connection as inactive if the thread is terminated while reading a result set.
| # expect the connection to not be broken | ||
| expect { @client.query('SELECT 1') }.to_not raise_error | ||
| end | ||
|
|
There was a problem hiding this comment.
It's already tested elsewhere and the assumption that the connection won't be broken is wrong (unless reconnect is enabled).
| close(wrapper->client->net.fd); | ||
| } | ||
| /* Skip mysql client check performed before command execution. */ | ||
| wrapper->client->status = MYSQL_STATUS_READY; |
There was a problem hiding this comment.
This looks like another reach into the MySQL structures, could you explain more about what this causes the libmysqlclient code to do?
There was a problem hiding this comment.
Yes, another reach unfortunately. But basically libmysql has a top-level check before executing new commands which later prevents the internal reconnect logic from kicking in if the connection was interrupted while reading results.
More specifically, once the socket is closed by mysql2, we need the next execution to proceed further than the aforementioned check in order for sending a query to fail and have the internal Vio closed, which in turn is required for a subsequent attempt (to send the query) to actually do the reconnect.
In summary, this is basically to avoid the aforementioned check and allow reconnect to kick-in if the connection was interrupted while reading the result set. For what it's worth, even libmysql itself resets the flag in a few places to make things work.
The current state is:
- mysql_query() is interrupted while reading result, socket is closed
- mysql_query() again, returns error because "commands out of sync"
With the change:
- mysql_query() is interrupted while reading result, socket is closed, status reset
- mysql_query() attempts to write to socket and fails, reconnect is attempted if enabled.
Source: I used to work on libmysql.
|
Sorry for the long delay before I reviewed this! Just left some comments and questions. |
|
Any update on this PR? IT would be great to get this into the next version of the gem if possible |
|
Please try the new mysql2 0.4.6 release and confirm is the issue is resolved as expected. |
The problem is that if execution is terminated while reading a result set, the next attempt to execute a query on the connection fails with "This connection is still waiting for a result, try again once you have the result". This error persists even if reconnect is enabled.
The issue stems from reading the result set using
rb_thread_call_without_gvl. If the calling thread is terminated insiderb_thread_call_without_gvl, the function does not return and the caller doesn't get a chance to reset the active thread associated with the connection — which normally happens after the result set has been read.The solution is to disconnect and mark the connection as inactive if the thread is terminated while reading a result set.