-
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
Making Reshard work smoothly with Atomic Transactions #16844
Making Reshard work smoothly with Atomic Transactions #16844
Conversation
…vice 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
|
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]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16844 +/- ##
==========================================
- Coverage 69.43% 69.43% -0.01%
==========================================
Files 1571 1571
Lines 203086 203239 +153
==========================================
+ Hits 141013 141113 +100
- Misses 62073 62126 +53 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Manan Gupta <[email protected]>
…sactions Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
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.
Elegant solution!
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.
LGTM. I only had some minor nits, requests, suggestions. Nice work on this, @GuptaManan100 !
Signed-off-by: Manan Gupta <[email protected]>
Thank you for the reviews everyone! 🚀 I've addressed the review comments and am merging the PR now. |
Description
This PR makes the changes required to make Reshard work with Atomic transactions. Before these changes, Resharding won't wait for prepared atomic transactions to go through, and we could be in a situation where a prepared transaction is unable to commit on the source shards because we have stopped query service on it. At the same time, that prepared transaction won't exist on the target shards, leading to us being unable to commit a prepared transaction altogether.
This PR fixes this situation by changing the
RefreshState
RPC that Resharding is using. Now, when vttablet is executing theRefreshState
RPC, if it decides that query service needs to be stopped, it first turns off the two pc engine from accepting new prepared transactions and then waits for the current prepared transactions until they're resolved. This ensures Resharding doesn't proceed until all the prepared transactions have concluded.This PR adds fuzzer and stress tests to verify that the changes work as intended. The changes in this PR are different from the ones proposed in the RFC #16245 (comment), because we found that this solution is more elegant and easier to implement than the one proposed there.
Related Issue(s)
Checklist
Deployment Notes