diff --git a/go/vt/mysqlctl/backupengine.go b/go/vt/mysqlctl/backupengine.go index f811a08af58..1401195548a 100644 --- a/go/vt/mysqlctl/backupengine.go +++ b/go/vt/mysqlctl/backupengine.go @@ -32,6 +32,7 @@ import ( "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" @@ -46,7 +47,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 ca70e3eb908..cf8c6a1a564 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -42,6 +42,7 @@ import ( "vitess.io/vitess/go/vt/logutil" stats "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" @@ -1080,7 +1081,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/builtinbackupengine2_test.go b/go/vt/mysqlctl/builtinbackupengine2_test.go new file mode 100644 index 00000000000..ace019f93e2 --- /dev/null +++ b/go/vt/mysqlctl/builtinbackupengine2_test.go @@ -0,0 +1,35 @@ +/* +Copyright 2023 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package mysqlctl_test is the blackbox tests for package mysqlctl. +package mysqlctl + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata" +) + +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 cfcf7344301..6b3d04c77ca 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -35,6 +35,7 @@ import ( "vitess.io/vitess/go/mysql" "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" @@ -944,7 +945,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 6cbdfd1cfeb..eb365843e65 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 }