Skip to content
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

Fixes the SwitchTraffic bug that wasn't respecting --dry_run for readonly and replica tablets during a resharding event #91

Conversation

austenLacy
Copy link

@austenLacy austenLacy commented Apr 26, 2023

ref: https://github.com/Shopify/vitess-project/issues/431

Fixes the SwitchTraffic bug that wasn't respecting --dry_run for readonly and replica tablets during a resharding event. More details specifically in this comment.

@austenLacy austenLacy changed the base branch from main to v15.0.3-shopify-3 April 26, 2023 15:39
@austenLacy austenLacy changed the title austenlacy/vitess project 431/respect dry run flag on switchtraffic Fixes the SwitchTraffic bug that wasn't respect --dry_run for readonly and replica tablets during a resharding event Apr 26, 2023
Copy link

@brendar brendar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Submit it upstream?

@austenLacy austenLacy changed the title Fixes the SwitchTraffic bug that wasn't respect --dry_run for readonly and replica tablets during a resharding event Fixes the SwitchTraffic bug that wasn't respecting --dry_run for readonly and replica tablets during a resharding event Apr 26, 2023
@austenLacy
Copy link
Author

Looks good to me. Submit it upstream?

Yep, might as well. Will be curious to see if they'll accept it or reopen the original PR. Either way should be fine for us.

Copy link

@hkdsun hkdsun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch 👍 I'm sure it will get merged

@hkdsun
Copy link

hkdsun commented Apr 27, 2023

reopen the original PR

Missing context? 🤔

@austenLacy
Copy link
Author

@hkdsun the linked issue has a PR that was closed (not sure why) with the same fix - vitessio#6497

@austenLacy
Copy link
Author

Upstream PR was merged - vitessio#12992

It had some e2e tests that have some conflicts with the v15 branch. The one-liner change represented in this PR is the same though so I think it's safe to merge this into our v15 branch. The upstream PR did have labels to backport the change to 14,15, and 16 though.

@hkdsun @brendar thoughts on merging this as is?

@brendar
Copy link

brendar commented Apr 28, 2023

I'm fine with merging as is

@austenLacy austenLacy merged commit 46f1def into v15.0.3-shopify-3 May 9, 2023
@austenLacy austenLacy deleted the austenlacy/vitess-project-431/respect-dry-run-flag-on-switchtraffic branch May 9, 2023 20:18
shanth96 pushed a commit that referenced this pull request Mar 21, 2024
Signed-off-by: Austen Lacy <[email protected]>
Co-authored-by: Austen Lacy <[email protected]>
(cherry picked from commit 46f1def)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: DELETE FROM <sequence table> fails with does not have a primary vindex
4 participants