Support TRILOGY_CAPABILITIES_MULTI_RESULTS
#57
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The TRILOGY_CAPABILITIES_MULTI_RESULTS client flag should be set along with TRILOGY_CAPABILITIES_MULTI_STATEMENTS when the Trilogy client is initialized with `multi_statement: true`. The TRILOGY_CAPABILITIES_MULTI_RESULTS flag can also be set independently via `multi_result: true` to enable multi-result processing without allowing the client to send multi-statement queries. This is useful for stored procedures, dynamic SQL in init files, etc. To match behaviour with libmysqlclient, TRILOGY_CAPABILITIES_MULTI_RESULTS is enabled by default.
eileencodes
approved these changes
Mar 15, 2023
composerinteralia
added a commit
that referenced
this pull request
Apr 26, 2023
#57 set multi-results as a default. To safely upgrade to the latest version of Trilogy, we want to turn that off for now, and then turn it on one cluster at a time. This commit removes the default from the C library and into the Ruby C extension so we can disable it as needed. (We had an option to enable multi results, but removed it in #68 because the option was always a no-op. This PR adds the option back, with some modification).
luanzeba
added a commit
that referenced
this pull request
May 17, 2023
#57 set multi-results capability as a default. To safely upgrade to the latest version of Trilogy, we want to turn that option off and then turn it on one cluster at a time. This commit removes the multi-results capability default from TRILOGY_CAPABILITIES_CLIENT but will still set it unless the client is initialized with `multi_results: false`. We had an option to enable multi results, but removed it in #68 because the option was always a no-op. This PR adds the option back, with some modification. Co-authored-by: Daniel Colson <composerinteralia@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes
The MySQL client library enables
CLIENT_MULTI_RESULTSeither by passing the CLIENT_MULTI_RESULTS flag itself, or implicitly by passingCLIENT_MULTI_STATEMENTS(which also enablesCLIENT_MULTI_RESULTS).CLIENT_MULTI_RESULTSis enabled by default.Consequently, we should set the
TRILOGY_CAPABILITIES_MULTI_RESULTSclient flag along withTRILOGY_CAPABILITIES_MULTI_STATEMENTSwhen the Trilogy client is initialized withmulti_statement: true.Since the client flag can be set explicitly as well, this PR allows
TRILOGY_CAPABILITIES_MULTI_RESULTSto be set independently viamulti_result: true. This enables multi-result processing without allowing the client to send multi-statement queries, which is useful for stored procedures, dynamic SQL in init files, etc.To match
Mysql2's behaviour, we enableTRILOGY_CAPABILITIES_MULTI_RESULTSby default. There shouldn't be any downsides to enabling this flag; it essentially specifies that the client is capable of multi-result set processing, which it is thanks to Paarth's changes in #35.Motivation for Change
At Shopify, this change is important for us as we attempt to migrate from Mysql2 to Trilogy, because ProxySQL treats the two drivers differently if they have different client flags set. This results in a lot of
COM_CHANGE_USERcommands if the two clients are running simultaneously.This is where ProxySQL compares the client flags and this is where ProxySQL determines whether it can use the cached connections, or whether it needs to perform a
COM_CHANGE_USERcommand. The issue for us was that running both of the adapters at 50% resulted in a ton ofCOM_CHANGE_USERcommands as the connection pool was handed off between these two users, and we overloaded MySQL.We could, of course, just configure the flag ourselves, but it makes sense to me to match
Mysql2's behaviour.cc @byroot @paarthmadan @eileencodes