diff --git a/go/vt/mysqlctl/backupengine.go b/go/vt/mysqlctl/backupengine.go index 3bf1560f8b3..5a79edbdde0 100644 --- a/go/vt/mysqlctl/backupengine.go +++ b/go/vt/mysqlctl/backupengine.go @@ -28,12 +28,12 @@ import ( "github.com/spf13/pflag" - "vitess.io/vitess/go/mysql/replication" - "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/mysqlctl/backupstats" "vitess.io/vitess/go/vt/mysqlctl/backupstorage" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/topo" @@ -48,7 +48,7 @@ var ( // BackupEngine is the interface to take a backup with a given engine. type BackupEngine interface { ExecuteBackup(ctx context.Context, params BackupParams, bh backupstorage.BackupHandle) (bool, error) - ShouldDrainForBackup() bool + ShouldDrainForBackup(req *tabletmanagerdatapb.BackupRequest) bool } // BackupParams is the struct that holds all params passed to ExecuteBackup diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 8442c855c0a..e8b6d54e152 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -36,10 +36,9 @@ import ( "github.com/spf13/pflag" "golang.org/x/sync/semaphore" - "vitess.io/vitess/go/mysql/replication" - "vitess.io/vitess/go/ioutil" "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/vt/concurrency" "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" @@ -47,6 +46,7 @@ import ( "vitess.io/vitess/go/vt/mysqlctl/backupstorage" "vitess.io/vitess/go/vt/proto/mysqlctl" mysqlctlpb "vitess.io/vitess/go/vt/proto/mysqlctl" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/topo" @@ -1149,7 +1149,11 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa // ShouldDrainForBackup satisfies the BackupEngine interface // backup requires query service to be stopped, hence true -func (be *BuiltinBackupEngine) ShouldDrainForBackup() bool { +func (be *BuiltinBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.BackupRequest) bool { + if req != nil && req.IncrementalFromPos != "" { + // Incremental backup: we do not drain the tablet. + return false + } return true } diff --git a/go/vt/mysqlctl/builtinbackupengine_test.go b/go/vt/mysqlctl/builtinbackupengine_test.go index 29252392c7f..39e4aa7ae1c 100644 --- a/go/vt/mysqlctl/builtinbackupengine_test.go +++ b/go/vt/mysqlctl/builtinbackupengine_test.go @@ -21,6 +21,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" ) func TestGetIncrementalFromPosGTIDSet(t *testing.T) { @@ -67,3 +69,12 @@ func TestGetIncrementalFromPosGTIDSet(t *testing.T) { }) } } + +func TestShouldDrainForBackupBuiltIn(t *testing.T) { + be := &BuiltinBackupEngine{} + + assert.True(t, be.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{})) + assert.False(t, be.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{IncrementalFromPos: "auto"})) + assert.False(t, be.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{IncrementalFromPos: "99ca8ed4-399c-11ee-861b-0a43f95f28a3:1-197"})) + assert.False(t, be.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{IncrementalFromPos: "MySQL56/99ca8ed4-399c-11ee-861b-0a43f95f28a3:1-197"})) +} diff --git a/go/vt/mysqlctl/fakebackupengine.go b/go/vt/mysqlctl/fakebackupengine.go index c0fce435d35..2b8c3208ac5 100644 --- a/go/vt/mysqlctl/fakebackupengine.go +++ b/go/vt/mysqlctl/fakebackupengine.go @@ -21,6 +21,7 @@ import ( "time" "vitess.io/vitess/go/vt/mysqlctl/backupstorage" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" ) type FakeBackupEngine struct { @@ -86,7 +87,7 @@ func (be *FakeBackupEngine) ExecuteRestore( return be.ExecuteRestoreReturn.Manifest, be.ExecuteRestoreReturn.Err } -func (be *FakeBackupEngine) ShouldDrainForBackup() bool { +func (be *FakeBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.BackupRequest) bool { be.ShouldDrainForBackupCalls = be.ShouldDrainForBackupCalls + 1 return be.ShouldDrainForBackupReturn } diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index fb9fa4c9667..d11699167d9 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -36,6 +36,7 @@ import ( "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/mysqlctl/backupstorage" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/vterrors" @@ -950,7 +951,7 @@ func stripeReader(readers []io.Reader, blockSize int64) io.Reader { // ShouldDrainForBackup satisfies the BackupEngine interface // xtrabackup can run while tablet is serving, hence false -func (be *XtrabackupEngine) ShouldDrainForBackup() bool { +func (be *XtrabackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.BackupRequest) bool { return false } diff --git a/go/vt/mysqlctl/xtrabackupengine_test.go b/go/vt/mysqlctl/xtrabackupengine_test.go index 26e53c6c949..7a829ce4ba0 100644 --- a/go/vt/mysqlctl/xtrabackupengine_test.go +++ b/go/vt/mysqlctl/xtrabackupengine_test.go @@ -22,7 +22,10 @@ import ( "math/rand" "testing" + "github.com/stretchr/testify/assert" + "vitess.io/vitess/go/vt/logutil" + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" ) func TestFindReplicationPosition(t *testing.T) { @@ -115,3 +118,10 @@ func TestStripeRoundTrip(t *testing.T) { // Test block size and stripe count that don't evenly divide data size. test(6000, 7) } + +func TestShouldDrainForBackupXtrabackup(t *testing.T) { + be := &XtrabackupEngine{} + + assert.False(t, be.ShouldDrainForBackup(nil)) + assert.False(t, be.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{})) +} diff --git a/go/vt/vttablet/tabletmanager/rpc_backup.go b/go/vt/vttablet/tabletmanager/rpc_backup.go index 0364af11027..b3d2e2794f6 100644 --- a/go/vt/vttablet/tabletmanager/rpc_backup.go +++ b/go/vt/vttablet/tabletmanager/rpc_backup.go @@ -68,7 +68,7 @@ func (tm *TabletManager) Backup(ctx context.Context, logger logutil.Logger, req // Prevent concurrent backups, and record stats backupMode := backupModeOnline - if engine.ShouldDrainForBackup() { + if engine.ShouldDrainForBackup(req) { backupMode = backupModeOffline } if err := tm.beginBackup(backupMode); err != nil { @@ -80,7 +80,7 @@ func (tm *TabletManager) Backup(ctx context.Context, logger logutil.Logger, req l := logutil.NewTeeLogger(logutil.NewConsoleLogger(), logger) var originalType topodatapb.TabletType - if engine.ShouldDrainForBackup() { + if engine.ShouldDrainForBackup(req) { if err := tm.lock(ctx); err != nil { return err }