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

Add -restart_before_backup parameter for vtbackup #8608

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

aquarapid
Copy link
Contributor

@aquarapid aquarapid commented Aug 9, 2021

If this flag is set, we perform a clean MySQL shutdown & startup cycle to work around xtrabackup
DDL bugs (see https://jira.percona.com/browse/PXB-2205). xtrabackup flag
-lock-ddl (which is the default) is not sufficient to fix this, and we
have run into this in real-world scenarios. See #8610

Signed-off-by: Jacques Grove [email protected]

a clean MySQL shutdown & startup cycle to work around xtrabackup bugs.

Signed-off-by: Jacques Grove <[email protected]>
@aquarapid aquarapid requested a review from deepthi August 9, 2021 17:40
@aquarapid aquarapid marked this pull request as ready for review August 9, 2021 18:01
@deepthi
Copy link
Member

deepthi commented Aug 9, 2021

vtbackup can be run with either builtin or xtrabackup backup engine. A better place for this flag and functionality is in the xtrabackup backup engine code. That gives us two benefits:

  • This functionality will become available to all uses of xtrabackup, not just via vtbackup.
  • Someone using builtin backup method won't be able to make the mistake of stopping and restarting mysql twice using this flag.

@aquarapid
Copy link
Contributor Author

Not sure I agree with this; limiting this to vtbackup only avoids someone doing the wrong thing when running xtrabackup against a live instance, and killing MySQL

@deepthi
Copy link
Member

deepthi commented Aug 10, 2021

Not sure I agree with this; limiting this to vtbackup only avoids someone doing the wrong thing when running xtrabackup against a live instance, and killing MySQL

They would have to start vttablets with the flag for that to happen. We would still default it to false.

@deepthi deepthi added Component: Cluster management Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Aug 10, 2021
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

Based on an offline discussion, I'm ok with adding this flag just to vtbackup and not vttablet.
The reasoning is that for anyone using vitess-operator, there's currently no way to pass differing xtrabackup flags through vtbackup than the standard vttablet ones; so you wouldn't be able to vary this flag depending on whether you take an offline (vtbackup) backup or an online one (vttablet). We can revisit a vttablet flag when someone is able to make that change in concert with a vitess-operator change.

@aquarapid Can you rename the flag to reflect that we are doing a stop/start and not actually executing any FLUSH statements?

Signed-off-by: Jacques Grove <[email protected]>
Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM

@deepthi deepthi merged commit ce0cd9b into vitessio:main Aug 20, 2021
@deepthi deepthi deleted the jg_vtbwork branch August 20, 2021 21:44
@deepthi deepthi changed the title Add -full_flush parameter for vtbackup, which performs Add -restart_before_backup parameter for vtbackup Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Cluster management Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants