Skip to content

Commit

Permalink
Merge remote-tracking branch 'rvrangel/mysqlshell-backup' into floren…
Browse files Browse the repository at this point in the history
…t-mysqlshell-backup-custom-docker-build

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
  • Loading branch information
frouioui committed Sep 18, 2024
2 parents cf277f1 + 9243f22 commit 69fd4b6
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 11 deletions.
24 changes: 13 additions & 11 deletions go/vt/mysqlctl/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,17 +447,19 @@ func Restore(ctx context.Context, params RestoreParams) (*BackupManifest, error)
return nil, err
}

// mysqld needs to be running in order for mysql_upgrade to work.
// If we've just restored from a backup from previous MySQL version then mysqld
// may fail to start due to a different structure of mysql.* tables. The flag
// --skip-grant-tables ensures that these tables are not read until mysql_upgrade
// is executed. And since with --skip-grant-tables anyone can connect to MySQL
// without password, we are passing --skip-networking to greatly reduce the set
// of those who can connect.
params.Logger.Infof("Restore: starting mysqld for mysql_upgrade")
// Note Start will use dba user for waiting, this is fine, it will be allowed.
if err := params.Mysqld.Start(context.Background(), params.Cnf, "--skip-grant-tables", "--skip-networking"); err != nil {
return nil, err
if re.ShouldStartMySQLAfterRestore() { // all engines except mysqlshell since MySQL is always running there
// mysqld needs to be running in order for mysql_upgrade to work.
// If we've just restored from a backup from previous MySQL version then mysqld
// may fail to start due to a different structure of mysql.* tables. The flag
// --skip-grant-tables ensures that these tables are not read until mysql_upgrade
// is executed. And since with --skip-grant-tables anyone can connect to MySQL
// without password, we are passing --skip-networking to greatly reduce the set
// of those who can connect.
params.Logger.Infof("Restore: starting mysqld for mysql_upgrade")
// Note Start will use dba user for waiting, this is fine, it will be allowed.
if err := params.Mysqld.Start(context.Background(), params.Cnf, "--skip-grant-tables", "--skip-networking"); err != nil {
return nil, err
}
}

params.Logger.Infof("Restore: running mysql_upgrade")
Expand Down
1 change: 1 addition & 0 deletions go/vt/mysqlctl/backupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ func (p *RestoreParams) IsIncrementalRecovery() bool {
// Returns the manifest of a backup if successful, otherwise returns an error
type RestoreEngine interface {
ExecuteRestore(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle) (*BackupManifest, error)
ShouldStartMySQLAfterRestore() bool
}

// BackupRestoreEngine is a combination of BackupEngine and RestoreEngine.
Expand Down
5 changes: 5 additions & 0 deletions go/vt/mysqlctl/builtinbackupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,11 @@ func (be *BuiltinBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.Bac
return true
}

// ShouldStartMySQLAfterRestore signifies if this backup engine needs to restart MySQL once the restore is completed.
func (be *BuiltinBackupEngine) ShouldStartMySQLAfterRestore() bool {
return true
}

func getPrimaryPosition(ctx context.Context, tmc tmclient.TabletManagerClient, ts *topo.Server, keyspace, shard string) (replication.Position, error) {
si, err := ts.GetShard(ctx, keyspace, shard)
if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions go/vt/mysqlctl/fakebackupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,7 @@ func (be *FakeBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.Backup
be.ShouldDrainForBackupCalls = be.ShouldDrainForBackupCalls + 1
return be.ShouldDrainForBackupReturn
}

func (be *FakeBackupEngine) ShouldStartMySQLAfterRestore() bool {
return true
}
6 changes: 6 additions & 0 deletions go/vt/mysqlctl/mysqlshellbackupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,12 @@ func (be *MySQLShellBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.
return mysqlShellBackupShouldDrain
}

// ShouldStartMySQLAfterRestore signifies if this backup engine needs to restart MySQL once the restore is completed.
// Since MySQL Shell operates on a live MySQL instance, there is no need to start it once the restore is completed
func (be *MySQLShellBackupEngine) ShouldStartMySQLAfterRestore() bool {
return false
}

func (be *MySQLShellBackupEngine) backupPreCheck(location string) error {
if mysqlShellBackupLocation == "" {
return fmt.Errorf("%w: no backup location set via --mysql-shell-backup-location", MySQLShellPreCheckError)
Expand Down
5 changes: 5 additions & 0 deletions go/vt/mysqlctl/xtrabackupengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,11 @@ func (be *XtrabackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.Backup
return false
}

// ShouldStartMySQLAfterRestore signifies if this backup engine needs to restart MySQL once the restore is completed.
func (be *XtrabackupEngine) ShouldStartMySQLAfterRestore() bool {
return true
}

func init() {
BackupRestoreEngineMap[xtrabackupEngineName] = &XtrabackupEngine{}
}

0 comments on commit 69fd4b6

Please sign in to comment.