MDEV-39008 FederatedX: set time_zone on connection change#4791
MDEV-39008 FederatedX: set time_zone on connection change#4791PeterKoletzki wants to merge 1 commit intoMariaDB:10.11from
Conversation
When the underlying connection to the remote server changes (e.g. load balancer switching nodes or driver reconnect), the session variable time_zone was not set again. TIMESTAMP values could then be wrong. Track the connection (mysql.net.vio) and re-send SET time_zone='+00:00' when the vio changes so TIMESTAMP remains consistent. Made-with: Cursor
|
|
gkodinov
added
the
External Contribution
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
I'd love if you would consider adding a regression test for this. It can be as simple as:
- open a federatedX conneciton
- restart the remote server
- try a query that requires a timezone
- try another query that requires a timezone
- cleanup
A slightly more advanced version would be to add a DBUG_EXECUTE_IF() with a unique key name, that, when set, will disconnect the federatedX session before the next query. And then do as the above, but instead of restarting the remote just set the flag on the federatedX server.
| time_zone_set_vio= mysql.net.vio; | ||
| mysql.reconnect= 1; | ||
| } | ||
| else if (mysql.net.vio != time_zone_set_vio) |
There was a problem hiding this comment.
I'd prefer that you develop a single boolean flag: e.g. needs_to_set_timezone and set it where it needs to be set. And then add a single check for it right before mysql_real_query() and set the timezone.
Note that this still has a potential for error: if you haven't detected the disconnect yet the first query you execute will be executed sans timezone set. The ones that follow will execute timezone set. But the first one will be off.
This is because automatic reconnect is done inside the client library. And it will automatically retry the same query right after re-connecting on its own.
So, MYSQL.reconnect should not be used for anything that can depend on session state. This is why the mariadb CLI is implementing its own, client app level reconnect.
If you really want to fix this in all cases, please consider removing the use of MYSQL.reconnect and do as mysql.cc does.
I will take the minor refactoring suggested in the first paragraph too. But I would like you to consider doing the full fix first.
4 participants
When the underlying connection to the remote server changes (e.g. load balancer switching nodes or driver reconnect), the session variable time_zone was not set again. TIMESTAMP values could then be wrong.
Track the connection (mysql.net.vio) and re-send SET time_zone='+00:00' when the vio changes so TIMESTAMP remains consistent.