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

xtrabackup implementation with testcases #4695

Merged
merged 15 commits into from
Apr 11, 2019

Conversation

deepthi
Copy link
Member

@deepthi deepthi commented Mar 2, 2019

Feature Description

This PR adds support for xtrabackup backups through Vitess.
Backups and restores can be executed just as they are today, the vtctl commands don't change.

Pre-requisites, Supported Versions

A compatible version of xtrabackup (and xbstream if using) must be present in PATH. For MySQL 5.7 and MariaDB 10, the xtrabackup versions that are allowed are 2.4.x and lower. For MySQL 8.0, it is 8.0.x.
MariaDB 10.3 is not compatible with xtrabackup.

How to Use

The change is to vttablet startup. vttablet needs to be told to use xtrabackup instead of the current method (which we are calling builtin), and provided the necessary flags. The vttablet flag is
-backup_engine_implementation=xtrabackup

Features

  • Only streaming mode backups are supported right now.
  • All 4 currently supported backup storage types will be supported by xtrabackup streaming backups also.
  • A MANIFEST file is created along with the backup archive, which can be either tar or xbstream.
  • gzip compression and uncompression is supported for xtrabackup just like for the current backups.

Options

Additional vttablet flags:

  • -xtrabackup_user: Required. database user to be used for backup
  • -xtrabackup_backup_flags: Optional. flags to add to the xtrabackup backup command
  • -xtrabackup_stream_mode: Optional. Valid values are tar and xbstream. Defaults to tar.
  • -xbstream_flags: Optional. Passed to xbstream command during restore.
    If additional flags are passed to xtrabackup and xbstream, care must be taken that they match (compress/uncompress, encrypt/decrypt etc.)

Tasks

Before we are ready to merge this, we need to
move the backup_engine_implementation choice from vttablet to vtctl
See #3957 (comment)

  • use vterrors to wrap errors
  • implement a backup user dbConfig
  • add -xtrabackup_user command line parameter
  • add xtrabackup to docker containers
  • get tests passing in docker
  • functional testing

Fixes #3957
Signed-off-by: deepthi deepthi@planetscale.com

config/mycnf/default-fast.cnf Show resolved Hide resolved
go/vt/dbconfigs/dbconfigs.go Outdated Show resolved Hide resolved
go/vt/vttablet/tabletmanager/rpc_backup.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
@rafael
Copy link
Member

rafael commented Apr 4, 2019

@deepthi - This is looking great. Using the xtrabackup manifest simplified the logic in the engine.

I added couple nit comments on some of the flags descriptions.

go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
docker/bootstrap/Dockerfile.mysql56 Outdated Show resolved Hide resolved
docker/bootstrap/Dockerfile.mariadb Outdated Show resolved Hide resolved
docker/bootstrap/Dockerfile.mariadb Outdated Show resolved Hide resolved
docker/bootstrap/Dockerfile.mysql80 Show resolved Hide resolved
go/vt/mysqlctl/backupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Outdated Show resolved Hide resolved
go/vt/mysqlctl/xtrabackupengine.go Show resolved Hide resolved
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
…k warnings

Signed-off-by: deepthi <deepthi@planetscale.com>
…storing xtrabackup

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
… using xtrabackup, loop through existing backups to find the most recent good backup

Signed-off-by: deepthi <deepthi@planetscale.com>
…ckup and restore

Signed-off-by: deepthi <deepthi@planetscale.com>
Signed-off-by: deepthi <deepthi@planetscale.com>
…r review comments

Signed-off-by: deepthi <deepthi@planetscale.com>
if use_xtrabackup:
xtra_args.extend(xtrabackup_args)

hook_args = ['-backup_storage_hook',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hook_args = ['-backup_storage_hook',
hook_args = ['-backup_storage_hook=test_backup_transform',
'-backup_storage_compress=false']

Copy link
Member

@rafael rafael left a comment

Choose a reason for hiding this comment

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

LGTM +1

One thing that caught my attention from the last commit is this function: findReplicationPosition. I think we should consider testing that in more depth. It seems to me that this part is still fragile. I wouldn't block for this right now.

@deepthi
Copy link
Member Author

deepthi commented Apr 11, 2019

LGTM +1

One thing that caught my attention from the last commit is this function: findReplicationPosition. I think we should consider testing that in more depth. It seems to me that this part is still fragile. I wouldn't block for this right now.

Good point. We have an integration test written specifically for this part, but we should also test some failure cases.

@deepthi deepthi merged commit f3bfe8b into vitessio:master Apr 11, 2019
setassociative added a commit to tinyspeck/vitess that referenced this pull request Apr 26, 2019
It's been busy and exciting in vitess upstream. Some changes that felt worth calling out as they could impact assumptions about behavior:

- vitessio#4832 — this adds a timeout that is 3x the `healthCheckInterval` which at which point the tablet will stop reporting as healthy
- vitessio#4814 — this changes when command line args are used by vttablet
- vitessio#4811 — a planned reparent now sets `super_read_only`
- vitessio#4805 — behavior coming back from a failed backup is different

**Slack changes**  
        @demmer vitessio#4827 Correct suppress logging for begin...commit in autocommit
        @rafael vitessio#4824 adds timeouts for all statements
        @demmer vitessio#4826 adds logging of stack traces as opt-in
        @demmer vitessio#4819 remove begin/commit logs from autocommit txns
        @demmer vitessio#4796 improves support for vtexplain for begin/dml/dml/commit txns

**Non slack changes**  
        vitessio#4839 Improve behavior with reference table routing & vreplication
        vitessio#4833 Support query routing given there could be multiple targets for a table
        vitessio#4832 tablets get new health check behavior (health checks time out)
        vitessio#4837 refresh bug around row streamer handling creds
        vitessio#4830 apply a default value to the db_name attribute in local_metadata
        vitessio#4785 SHOW SCHEMAS aliased to SHOW DATABASES
        vitessio#4829 add experimental support for split clone & vertical split clone via vreplication
        vitessio#4822 do not normalize in order by
        vitessio#4791 Adds vreplication row streaming support
        vitessio#4814 command line flags now used regardless of management mode
        vitessio#4811 vttablet sets super_read_only during planned reparent
        vitessio#4803 xtrabackup testing
        vitessio#4727 support multiple vttablets running against one mysql instance
        vitessio#4746 TopoCat can now produce JSON output
        vitessio#4805 mysqld is now restarted after a failed backup
        vitessio#4685 macos bootstrap bug
        vitessio#4874 ZK opts and java land
        vitessio#4695 Adds support for xtrabackup
        vitessio#4794 fixes build failures
        vitessio#4725 changes mysql8.0 start args
        vitessio#4736 introduce new states to support vreplication
        vitessio#4788 expands orc error

**Docs etc**  
        vitessio#4831 lint
        vitessio#4827 docs
        vitessio#4816 const declaration cleanup
        vitessio#4820 const declaration cleanup
        vitessio#4825 docs
        vitessio#4818 docs
        vitessio#4809 docs
        vitessio#4812 moves consts around
        vitessio#4813 docs
        vitessio#4808 docs
        vitessio#4800 docs
        vitessio#4795 docs
@harshit-gangal harshit-gangal deleted the ds-xtrabackup branch July 17, 2020 16:39
systay pushed a commit that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants