Closed sougou closed 7 years ago
@sjmudd has a need for SET max_execution_time=120000
. I told him this would probably be easy to add. Thoughts?
Additionally, some SET commands may be nice to just optionally no-op.
I guess coming from a setup where we knew we had a persistent connection then it makes sense to use some of these settings. That said by setting them one connection may behave differently to another and that would not be what's expected. So sticky sessions would be quite nice to avoid this problem and possibly "reset state" when the app disconnects.
The other option might be (depending on the variables concerned) to accept the statement and ignore it, or give a warning. This allows for testing without breaking the app, but does not have the intended affect so might not be ideal but aprovides a stepping stone to play with vitess while the app is being adjusted.
But using something like the query hints in 5.7 seems better. https://dev.mysql.com/doc/refman/5.7/en/optimizer-hints.html#optimizer-hints-execution-time as this works at the query level. Potentially vitess could also understand the hints and even apply them even if the underlying mysql (mariadb ?) doesn't understand them. Maybe too much work though ?
There is functionality overlap between this variable and the vttablet query killer. The difference is that the query killer works for all queries including DMLs. So, this gives us a few options:
max_execution_time
as applicable to all queries. If so, the implementation will just set the timeout on the query killer.SELECT
query.SELECT
query, send the current timeout as max_execution_time
to MySQL.Option 1 would be the most elegant, but it's not 100% backward compatible.
2 & 3 differ from the fact that vttablet issues a KILL
, which also kills the connection; It results in a reconnect later. But it's reliable and proven to work well in production.
To consider option 4, we have to see if max_execution_time
is better than a KILL
:
The initial use cases that this bug needed have been addressed. So, I'll close this. Going forward, we can create specific ones as needed.
2724 mentioned that there's a need to support for arbitrary SET statements. Looking at the list here, many of those variables are pretty intrusive and likely to break Vitess. Just reserving a connection where you're allowed to set anything will not be sufficient.
So, I think we should instead support variables on a case-by-case basis. For some of them, special code may have to be written, like we had to do for autocommit.
Let's see if we can get the list built and addressed.