vtgate: separate transaction_mode default and limit into independent flags#19426
Open
Devanshusharma2005 wants to merge 4 commits intovitessio:mainfrom
Open
vtgate: separate transaction_mode default and limit into independent flags#19426Devanshusharma2005 wants to merge 4 commits intovitessio:mainfrom
transaction_mode default and limit into independent flags#19426Devanshusharma2005 wants to merge 4 commits intovitessio:mainfrom
Conversation
Signed-off-by: Devanshu Sharma <devanshusharma658@gmail.com>
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Contributor
|
📝 Documentation updates detected! New suggestion: Add changelog entry for VTGate Tip: Whenever you leave a comment tagged |
Contributor
|
📝 Documentation updates detected! New suggestion: Document vtgate transaction-mode-default and transaction-mode-limit flags Tip: Configure how Promptless handles changelogs in Agent Settings 📋 |
…and vtgombo configuration Signed-off-by: Devanshu Sharma <devanshusharma658@gmail.com>
Author
|
@mhamza15 lets have a look. |
…nd backward compatibility checks. Ensure that the absence of the --transaction-mode-limit flag allows unrestricted SET transaction_mode. Update tests to reflect these changes and clarify the behavior of transaction mode limits. Signed-off-by: Devanshu Sharma <devanshusharma658@gmail.com>
Author
|
@mhamza15 |
Signed-off-by: Devanshu Sharma <devanshusharma658@gmail.com>
This file contains hidden or 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
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.
Description
This PR splits the
--transaction-modeflag into three separate flags so operators can independently control the default transaction mode and the maximum mode a session is allowed toSET.Previously,
--transaction-modedid double duty, it set both the default for new sessions and (implicitly) acted as the ceiling.That made it impossible to say:
“Default to
SINGLEfor safety, but allow power users to opt intoMULTI.”Now that’s possible.
What changed ?
--transaction-mode(existing) Still works as before. If neither of the new flags are set, both default and limit fall back to this. Fully backward compatible at the flag level.--transaction-mode-default(new)Controls what new sessions start with. Falls back to
--transaction-modeif unset.--transaction-mode-limit(new)Defines the ceiling for
SET transaction_mode= .... Also falls back to--transaction-modeif unset. If a session tries to set a mode above the limit, we return a MySQL-compatible error.Modes have a natural ordering:
SINGLE < MULTI < TWOPCLimit enforcement is simply
mode > limitusing the enum ordering.UNSPECIFIEDis always allowed (it just resets to default).We also validate at startup that
default <= limit. If not, vtgate refuses to start.SetTransactionModenow returns anerrorinstead of nothing, which flows throughSessionActionsand theSETengine.Related Issue(s)
Fixes #5416
Checklist
Deployment Notes
AI Disclosure
I used Claude more like a pairing buddy to brainstorm and generate a wide range of edge cases for the E2E and unit tests, mainly to make sure the limit enforcement holds up against weird or unexpected inputs.
The actual design and implementation especially the Viper flag wiring and session state propagation were done manually. I traced the full
SETlifecycle through VTGate usinggrepand basically did a DFS style walk across the interfaces to make suretransaction_modeflows correctly fromSessionActions down to the engine.Very open to any “tough” edge cases or uncomfortable scenarios the maintainers think I should stress test further