-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reject TwoPC calls if semi-sync is not enabled #16608
Reject TwoPC calls if semi-sync is not enabled #16608
Conversation
…hout semi-sync Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16608 +/- ##
=======================================
Coverage 68.97% 68.98%
=======================================
Files 1562 1562
Lines 200673 200706 +33
=======================================
+ Hits 138424 138449 +25
- Misses 62249 62257 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not disallow primary promotion if they are not using 2PC.
At somepoint we will make 2pc flag as default or maybe remove it completely as well.
Ideally today also, we do not need this flag.
What we should fail is the Prepare call, we can do this by marking an internal flag as twopc disabled and whenever a call for Prepare comes, we check against the internal flag first.
…ut-semi-sync Signed-off-by: Manan Gupta <[email protected]>
… instead Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
…ut-semi-sync Signed-off-by: Manan Gupta <[email protected]>
I've made the required changes @harshit-gangal |
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Description
This PR disallows two-pc if semi-sync is not enabled. This check is performed whenever a new primary is promoted. So we do this both in
InitPrimary
andPromoteReplica
. We store whether we want to allow two-pc or not in the tx engine and use this to fail the prepare calls.Technically, it is still possible for users to shoot themselves in the foot, by promoting a new primary with a semi-sync durability policy but changing the policy to none after that.
The only way to check for correctness, in this case, would be to check for whether all the current primaries are not running two-pc when the durability policy suggests that the primary shouldn't be running semi-sync. This however, would be overkill, and would require a RPC call from the vtctld to all the vttablets.
Related Issue(s)
Checklist
Deployment Notes