Skip to content

Conversation

@mattlord
Copy link
Member

@mattlord mattlord commented Nov 6, 2025

Description

This PR adds a new feature to Vitess Backups that is similar to these MySQL features

It allows users to specify a set of flags — via automated backups using vtbackup or via the vtctldclient Backup and BackupShard commands — which then support the ability to run a set of SQL commands on the mysqld instance before we back it up:

  1. --init-backup-sql-queries - Queries to execute before initializing the backup (flag can be specified multiple times or you can provide a CSV string)
  2. --init-backup-tablet-types - Tablet types used for the backup where the init SQL queries (--init-backup-sql-queries) will be executed before initializing the backup (flag can be specified multiple times or you can provide a CSV string)
  3. --init-backup-sql-timeout - At what point should we time out the init SQL query (--init-backup-sql-queries) work and either fail the backup job (--init-backup-sql-fail-on-error) or continue on with the backup
  4. --init-backup-sql-fail-on-error - Whether or not to fail the backup if the init SQL queries (--init-backup-sql-queries) fail, which includes if they fail to complete before the specified timeout (--init-backup-sql-timeout) (defaults to FALSE)

If any queries are specified then non-default values for the tablet-types and timeout flags must also be provided.

This supports any number of uses cases for users, including the one mentioned in the issue.

Note

Once the PR gets approval I will create a website PR to update the docs.

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported to release branches
  • If this change is to be back-ported to previous releases, a justification is included in the PR description
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on CI?
  • Documentation: docs: Document backup init SQL queries feature website#2039

Deployment Notes

AI Disclosure

My man Claude did the bulk of the unit test work.

@vitess-bot
Copy link
Contributor

vitess-bot bot commented Nov 6, 2025

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

  • Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

  • Apply the release notes (needs details) label if users need to know about this change.
  • New features should be documented.
  • There should be some code comments as to why things are implemented the way they are.
  • There should be a comment at the top of each new or modified test to explain what the test does.

New flags

  • Is this flag really necessary?
  • Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
  • vtctl command output order should be stable and awk-able.

@vitess-bot vitess-bot bot added NeedsBackportReason If backport labels have been applied to a PR, a justification is required NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsWebsiteDocsUpdate What it says labels Nov 6, 2025
@github-actions github-actions bot added this to the v24.0.0 milestone Nov 6, 2025
@mattlord mattlord added Type: Feature Component: Backup and Restore and removed NeedsBackportReason If backport labels have been applied to a PR, a justification is required labels Nov 6, 2025
@mattlord mattlord force-pushed the backup_optimize_table branch 2 times, most recently from 84b0f2f to 46c4392 Compare November 6, 2025 16:05
Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord force-pushed the backup_optimize_table branch from 46c4392 to b18a94c Compare November 6, 2025 16:34
Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord force-pushed the backup_optimize_table branch from 8279753 to 893d848 Compare November 6, 2025 17:17
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 47.42268% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.82%. Comparing base (1f49de4) to head (fd9fe8e).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
go/vt/vttablet/tabletmanager/rpc_backup.go 0.00% 21 Missing ⚠️
go/cmd/vtctldclient/command/backups.go 46.66% 16 Missing ⚠️
go/cmd/vtbackup/cli/vtbackup.go 0.00% 13 Missing ⚠️
go/vt/mysqlctl/backup.go 96.55% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #18883      +/-   ##
==========================================
+ Coverage   69.77%   69.82%   +0.05%     
==========================================
  Files        1608     1610       +2     
  Lines      214908   215299     +391     
==========================================
+ Hits       149953   150339     +386     
- Misses      64955    64960       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord force-pushed the backup_optimize_table branch from 0d295ee to 4a1d0d2 Compare November 6, 2025 22:13
Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord changed the title Backups: Add support for backup init SQL queries Backups: Add Support for Backup Init SQL Queries Nov 9, 2025
@mattlord mattlord removed NeedsIssue A linked issue is missing for this Pull Request NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work labels Nov 9, 2025
Signed-off-by: Matt Lord <[email protected]>
@mattlord mattlord requested a review from nickvanw December 4, 2025 14:01
Copy link
Member

@arthurschreiber arthurschreiber left a comment

Choose a reason for hiding this comment

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

Changes look good to me. I'm only a bit confused about the usefulness of the --init-backup-tablet-types flag. How do you intend for it to be used? 🙇‍♂️

@mattlord
Copy link
Member Author

mattlord commented Dec 4, 2025

Changes look good to me. I'm only a bit confused about the usefulness of the --init-backup-tablet-types flag. How do you intend for it to be used? 🙇‍♂️

Thanks! ❤️

You may only want to backup RDONLY tablets -- at least when available. Same for REPLICA tablets. And there are times where you might be forced to backup a PRIMARY tablet. The init statements may only be applicable/desirable/safe on a subset of those tablet types. That's the thinking behind it. Does that make sense?

Copy link
Contributor

@timvaillancourt timvaillancourt left a comment

Choose a reason for hiding this comment

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

Added one question that should not change my review. LGTM 👍

@mattlord
Copy link
Member Author

mattlord commented Dec 4, 2025

Docs PR: vitessio/website#2039

@mattlord mattlord removed the NeedsWebsiteDocsUpdate What it says label Dec 4, 2025
@mattlord mattlord merged commit 7eb81bb into main Dec 4, 2025
106 of 107 checks passed
@mattlord mattlord deleted the backup_optimize_table branch December 4, 2025 18:36
nickvanw pushed a commit that referenced this pull request Dec 7, 2025
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Nick Van Wiggeren <[email protected]>
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.

Feature Request: Add support for executing SQL queries prior to taking a backup

4 participants