-
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
VReplication Atomic Copy Workflows: fix bugs around concurrent inserts #17772
VReplication Atomic Copy Workflows: fix bugs around concurrent inserts #17772
Conversation
…e after all copy is done. Updated test to run concurrent inserts Signed-off-by: Rohit Nayak <[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 #17772 +/- ##
==========================================
- Coverage 67.95% 67.95% -0.01%
==========================================
Files 1585 1586 +1
Lines 255162 255207 +45
==========================================
+ Hits 173408 173432 +24
- Misses 81754 81775 +21 ☔ 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.
Great catch! Minor code beauty suggestion, at your discretion.
…d regular on TestFKWorkflow Signed-off-by: Rohit Nayak <[email protected]>
#17772) Signed-off-by: Rohit Nayak <[email protected]>
…ncurrent inserts (#17772) (#17793) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Description
Setting
--vreplication-parallel-insert-workers
greater than 1, had not been tested earlier for atomic copies. When a user attempted to do it, it caused a panic invttablet
. There were two fixes needed here:In addition the
TestFKWorkflow
e2e test has been changed to test with concurrent inserts.Related Issue(s)
Fixes #17716
Checklist
Deployment Notes