diff --git a/go/vt/mysqlctl/backup.go b/go/vt/mysqlctl/backup.go index 62004772ba..324f61c344 100644 --- a/go/vt/mysqlctl/backup.go +++ b/go/vt/mysqlctl/backup.go @@ -100,8 +100,6 @@ func Backup(ctx context.Context, dir, name string, params BackupParams) error { if err != nil { return vterrors.Wrap(err, "StartBackup failed") } - // Set params.BackupHandle to the selected backup - params.BackupHandle = bh be, err := GetBackupEngine() if err != nil { @@ -109,7 +107,7 @@ func Backup(ctx context.Context, dir, name string, params BackupParams) error { } // Take the backup, and either AbortBackup or EndBackup. - usable, err := be.ExecuteBackup(ctx, params) + usable, err := be.ExecuteBackup(ctx, params, bh) logger := params.Logger var finishErr error if usable { @@ -291,15 +289,13 @@ func Restore(ctx context.Context, params RestoreParams) (mysql.Position, error) if err != nil { return rval, err } - // Set params.BackupHandle to the selected backup - params.BackupHandle = bh re, err := GetRestoreEngine(ctx, bh) if err != nil { return mysql.Position{}, vterrors.Wrap(err, "Failed to find restore engine") } - if rval, err = re.ExecuteRestore(ctx, params); err != nil { + if rval, err = re.ExecuteRestore(ctx, params, bh); err != nil { return rval, err } diff --git a/go/vt/mysqlctl/backupengine.go b/go/vt/mysqlctl/backupengine.go index 8e48aa4189..e65ecd97c5 100644 --- a/go/vt/mysqlctl/backupengine.go +++ b/go/vt/mysqlctl/backupengine.go @@ -40,40 +40,51 @@ var ( // BackupEngine is the interface to take a backup with a given engine. type BackupEngine interface { - ExecuteBackup(ctx context.Context, params BackupParams) (bool, error) + ExecuteBackup(ctx context.Context, params BackupParams, bh backupstorage.BackupHandle) (bool, error) ShouldDrainForBackup() bool } // BackupParams is the struct that holds all params passed to ExecuteBackup type BackupParams struct { - Cnf *Mycnf - Mysqld MysqlDaemon - Logger logutil.Logger - BackupHandle backupstorage.BackupHandle - Concurrency int + Cnf *Mycnf + Mysqld MysqlDaemon + Logger logutil.Logger + // Concurrency is the value of -concurrency flag given to Backup command + // It determines how many files are processed in parallel + Concurrency int + // Extra env variables for pre-backup and post-backup transform hooks HookExtraEnv map[string]string - TopoServer *topo.Server - Keyspace string - Shard string + // TopoServer, Keyspace and Shard are used to discover master tablet + TopoServer *topo.Server + Keyspace string + Shard string } // RestoreParams is the struct that holds all params passed to ExecuteRestore type RestoreParams struct { - Cnf *Mycnf - Mysqld MysqlDaemon - Logger logutil.Logger - BackupHandle backupstorage.BackupHandle - Concurrency int - HookExtraEnv map[string]string - LocalMetadata map[string]string + Cnf *Mycnf + Mysqld MysqlDaemon + Logger logutil.Logger + // Concurrency is the value of -restore_concurrency flag (init restore parameter) + // It determines how many files are processed in parallel + Concurrency int + // Extra env variables for pre-restore and post-restore transform hooks + HookExtraEnv map[string]string + // Metadata to write into database after restore. See PopulateMetadataTables + LocalMetadata map[string]string + // DeleteBeforeRestore tells us whether existing data should be deleted before + // restoring. This is always set to false when starting a tablet with -restore_from_backup, + // but is set to true when executing a RestoreFromBackup command on an already running vttablet DeleteBeforeRestore bool - DbName string - Dir string + // Name of the managed database / schema + DbName string + // Directory location to search for a usable backup + Dir string } // RestoreEngine is the interface to restore a backup with a given engine. type RestoreEngine interface { - ExecuteRestore(ctx context.Context, params RestoreParams) (mysql.Position, error) + ExecuteRestore(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle) (mysql.Position, error) } // BackupRestoreEngine is a combination of BackupEngine and RestoreEngine. diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 7fa0a93dcf..e975c04bdf 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -239,23 +239,18 @@ func findFilesToBackup(cnf *Mycnf) ([]FileEntry, error) { // ExecuteBackup returns a boolean that indicates if the backup is usable, // and an overall error. -func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupParams) (bool, error) { +func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupParams, bh backupstorage.BackupHandle) (bool, error) { // extract all params from BackupParams cnf := params.Cnf mysqld := params.Mysqld logger := params.Logger - bh := params.BackupHandle backupConcurrency := params.Concurrency hookExtraEnv := params.HookExtraEnv topoServer := params.TopoServer keyspace := params.Keyspace shard := params.Shard - if bh == nil { - return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "ExecuteBackup must be called with a valid BackupHandle") - } - logger.Infof("Hook: %v, Compress: %v", *backupStorageHook, *backupStorageCompress) // Save initial state so we can restore. @@ -353,11 +348,12 @@ func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupP return usable, vterrors.Wrap(err, "slave is not restarting") } - // wait for reliable seconds behind master - // we have replicationPosition where we stopped - // if MasterPosition is the same, that means no writes - // have happened to master, so we are up-to-date - // otherwise, wait for replica's Position to change from + // Wait for a reliable value for SecondsBehindMaster from SlaveStatus() + + // We know that we stopped at replicationPosition. + // If MasterPosition is the same, that means no writes + // have happened to master, so we are up-to-date. + // Otherwise, we wait for replica's Position to change from // the saved replicationPosition before proceeding tmc := tmclient.NewTabletManagerClient() defer tmc.Close() @@ -365,7 +361,7 @@ func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupP defer remoteCancel() masterPos, err := getMasterPosition(remoteCtx, tmc, topoServer, keyspace, shard) - // if we are unable to get master position, return error + // If we are unable to get master position, return error. if err != nil { return usable, err } @@ -560,22 +556,15 @@ func (be *BuiltinBackupEngine) backupFile(ctx context.Context, cnf *Mycnf, mysql // ExecuteRestore restores from a backup. If the restore is successful // we return the position from which replication should start // otherwise an error is returned -func (be *BuiltinBackupEngine) ExecuteRestore( - ctx context.Context, - params RestoreParams) (mysql.Position, error) { +func (be *BuiltinBackupEngine) ExecuteRestore(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle) (mysql.Position, error) { cnf := params.Cnf mysqld := params.Mysqld logger := params.Logger - bh := params.BackupHandle restoreConcurrency := params.Concurrency hookExtraEnv := params.HookExtraEnv zeroPosition := mysql.Position{} - if bh == nil { - return zeroPosition, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "ExecuteRestore must be called with a valid BackupHandle") - } - var bm builtinBackupManifest if err := getBackupManifestInto(ctx, bh, &bm); err != nil { diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 2b7acb277e..a2a035b1be 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -121,16 +121,11 @@ func closeFile(wc io.WriteCloser, fileName string, logger logutil.Logger, finalE // ExecuteBackup returns a boolean that indicates if the backup is usable, // and an overall error. -func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupParams) (complete bool, finalErr error) { +func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupParams, bh backupstorage.BackupHandle) (complete bool, finalErr error) { // extract all params from BackupParams cnf := params.Cnf mysqld := params.Mysqld logger := params.Logger - bh := params.BackupHandle - - if bh == nil { - return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "ExecuteBackup must be called with a valid BackupHandle") - } if *xtrabackupUser == "" { return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "xtrabackupUser must be specified.") @@ -372,20 +367,13 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, cnf *Mycnf, logger } // ExecuteRestore restores from a backup. Any error is returned. -func (be *XtrabackupEngine) ExecuteRestore( - ctx context.Context, - params RestoreParams) (mysql.Position, error) { +func (be *XtrabackupEngine) ExecuteRestore(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle) (mysql.Position, error) { cnf := params.Cnf mysqld := params.Mysqld logger := params.Logger - bh := params.BackupHandle zeroPosition := mysql.Position{} - if bh == nil { - return zeroPosition, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "ExecuteRestore must be called with a valid BackupHandle") - } - var bm xtraBackupManifest if err := getBackupManifestInto(ctx, bh, &bm); err != nil {