restore: do not change tablet type to RESTORE if not actually perform…

…ing a restore (#6679)

* restore: do not change tablet type to RESTORE if not actually performing a restore

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

* restore: cleanup usage of ErrExistingDB, improve comments, do not restore if mysql doesn't come up

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

* restore: remove ErrExistingDB which is no longer needed

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

go/cmd/vtbackup/vtbackup.go

@@ -306,8 +306,6 @@ func takeBackup(ctx context.Context, topoServer *topo.Server, backupStorage back
  return fmt.Errorf("no backup found; not starting up empty since -initial_backup flag was not enabled")
  }
  restorePos = mysql.Position{}
- case mysqlctl.ErrExistingDB:
- return fmt.Errorf("can't run vtbackup because data directory is not empty")
  default:
  return fmt.Errorf("can't restore from backup: %v", err)
  }

go/test/endtoend/backup/vtbackup/backup_only_test.go

@@ -147,6 +147,8 @@ func firstBackupTest(t *testing.T, tabletType string) {
  // check that the restored replica has the right local_metadata
  result, err := replica2.VttabletProcess.QueryTabletWithDB("select * from local_metadata", "_vt")
  require.Nil(t, err)
+ require.NotNil(t, result)
+ require.NotEmpty(t, result.Rows)
  assert.Equal(t, replica2.Alias, result.Rows[0][1].ToString(), "Alias")
  assert.Equal(t, "ks.0", result.Rows[1][1].ToString(), "ClusterAlias")
  assert.Equal(t, cell, result.Rows[2][1].ToString(), "DataCenter")

go/vt/mysqlctl/backup.go

@@ -64,9 +64,6 @@ var (
  // but none of them are complete.
  ErrNoCompleteBackup = errors.New("backup(s) found but none are complete")
 
- // ErrExistingDB is returned when there's already an active DB.
- ErrExistingDB = errors.New("skipping restore due to existing database")
-
  // backupStorageHook contains the hook name to use to process
  // backup files. If not set, we will not process the files. It is
  // only used at backup time. Then it is put in the manifest,
@@ -218,34 +215,25 @@ func removeExistingFiles(cnf *Mycnf) error {
  return nil
 }
 
+// ShouldRestore checks whether a database with tables already exists
+// and returns whether a restore action should be performed
+func ShouldRestore(ctx context.Context, params RestoreParams) (bool, error) {
+ if params.DeleteBeforeRestore || RestoreWasInterrupted(params.Cnf) {
+ return true, nil
+ }
+ params.Logger.Infof("Restore: No %v file found, checking no existing data is present", RestoreState)
+ // Wait for mysqld to be ready, in case it was launched in parallel with us.
+ // If this doesn't succeed, we should not attempt a restore
+ if err := params.Mysqld.Wait(ctx, params.Cnf); err != nil {
+ return false, err
+ }
+ return checkNoDB(ctx, params.Mysqld, params.DbName)
+}
+
 // Restore is the main entry point for backup restore. If there is no
 // appropriate backup on the BackupStorage, Restore logs an error
 // and returns ErrNoBackup. Any other error is returned.
 func Restore(ctx context.Context, params RestoreParams) (*BackupManifest, error) {
-
- if !params.DeleteBeforeRestore {
- params.Logger.Infof("Restore: Checking if a restore is in progress")
- if !RestoreWasInterrupted(params.Cnf) {
- params.Logger.Infof("Restore: No %v file found, checking no existing data is present", RestoreState)
- // Wait for mysqld to be ready, in case it was launched in parallel with us.
- if err := params.Mysqld.Wait(ctx, params.Cnf); err != nil {
- return nil, err
- }
-
- ok, err := checkNoDB(ctx, params.Mysqld, params.DbName)
- if err != nil {
- return nil, err
- }
- if !ok {
- params.Logger.Infof("Auto-restore is enabled, but mysqld already contains data. Assuming vttablet was just restarted.")
- if err = PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName); err == nil {
- err = ErrExistingDB
- }
- return nil, err
- }
- }
- }
-
  // find the right backup handle: most recent one, with a MANIFEST
  params.Logger.Infof("Restore: looking for a suitable backup to restore")
  bs, err := backupstorage.GetBackupStorage()

go/vt/vttablet/tabletmanager/restore.go

@@ -76,12 +76,9 @@ func (tm *TabletManager) RestoreData(ctx context.Context, logger logutil.Logger,
 }
 
 func (tm *TabletManager) restoreDataLocked(ctx context.Context, logger logutil.Logger, waitForBackupInterval time.Duration, deleteBeforeRestore bool) error {
+
  tablet := tm.Tablet()
  originalType := tablet.Type
- if err := tm.tmState.ChangeTabletType(ctx, topodatapb.TabletType_RESTORE); err != nil {
- return err
- }
-
  // Try to restore. Depending on the reason for failure, we may be ok.
  // If we're not ok, return an error and the tm will log.Fatalf,
  // causing the process to be restarted and the restore retried.
@@ -117,6 +114,24 @@ func (tm *TabletManager) restoreDataLocked(ctx context.Context, logger logutil.L
  StartTime: logutil.ProtoToTime(keyspaceInfo.SnapshotTime),
  }
 
+ // Check whether we're going to restore before changing to RESTORE type,
+ // so we keep our MasterTermStartTime (if any) if we aren't actually restoring.
+ ok, err := mysqlctl.ShouldRestore(ctx, params)
+ if err != nil {
+ return err
+ }
+ if !ok {
+ params.Logger.Infof("Attempting to restore, but mysqld already contains data. Assuming vttablet was just restarted.")
+ return mysqlctl.PopulateMetadataTables(params.Mysqld, params.LocalMetadata, params.DbName)
+ }
+ // We should not become master after restore, because that would incorrectly
+ // start a new master term, and it's likely our data dir will be out of date.
+ if originalType == topodatapb.TabletType_MASTER {
+ originalType = tm.baseTabletType
+ }
+ if err := tm.tmState.ChangeTabletType(ctx, topodatapb.TabletType_RESTORE); err != nil {
+ return err
+ }
  // Loop until a backup exists, unless we were told to give up immediately.
  var backupManifest *mysqlctl.BackupManifest
  for {
@@ -161,11 +176,6 @@ func (tm *TabletManager) restoreDataLocked(ctx context.Context, logger logutil.L
  }
  case mysqlctl.ErrNoBackup:
  // No-op, starting with empty database.
- case mysqlctl.ErrExistingDB:
- // No-op, assuming we've just restarted. Note the
- // replication reporter may restart replication at the
- // next health check if it thinks it should. We do not
- // alter replication here.
  default:
  // If anything failed, we should reset the original tablet type
  if err := tm.tmState.ChangeTabletType(ctx, originalType); err != nil {

go/vt/wrangler/testlib/backup_test.go

@@ -23,6 +23,10 @@ import (
  "testing"
  "time"
 
+ "github.com/stretchr/testify/require"
+
+ "github.com/stretchr/testify/assert"
+
  "vitess.io/vitess/go/vt/discovery"
 
  "golang.org/x/net/context"
@@ -74,9 +78,7 @@ func TestBackupRestore(t *testing.T) {
 
  // Initialize our temp dirs
  root, err := ioutil.TempDir("", "backuptest")
- if err != nil {
- t.Fatalf("os.TempDir failed: %v", err)
- }
+ require.NoError(t, err)
  defer os.RemoveAll(root)
 
  // Initialize BackupStorage
@@ -90,19 +92,11 @@ func TestBackupRestore(t *testing.T) {
  sourceDataDir := path.Join(root, "source_data")
  sourceDataDbDir := path.Join(sourceDataDir, "vt_db")
  for _, s := range []string{sourceInnodbDataDir, sourceInnodbLogDir, sourceDataDbDir} {
- if err := os.MkdirAll(s, os.ModePerm); err != nil {
- t.Fatalf("failed to create directory %v: %v", s, err)
- }
- }
- if err := ioutil.WriteFile(path.Join(sourceInnodbDataDir, "innodb_data_1"), []byte("innodb data 1 contents"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file innodb_data_1: %v", err)
- }
- if err := ioutil.WriteFile(path.Join(sourceInnodbLogDir, "innodb_log_1"), []byte("innodb log 1 contents"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file innodb_log_1: %v", err)
- }
- if err := ioutil.WriteFile(path.Join(sourceDataDbDir, "db.opt"), []byte("db opt file"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file db.opt: %v", err)
+ require.NoError(t, os.MkdirAll(s, os.ModePerm))
  }
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceInnodbDataDir, "innodb_data_1"), []byte("innodb data 1 contents"), os.ModePerm))
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceInnodbLogDir, "innodb_log_1"), []byte("innodb log 1 contents"), os.ModePerm))
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceDataDbDir, "db.opt"), []byte("db opt file"), os.ModePerm))
 
  // create a master tablet, set its master position
  master := NewFakeTablet(t, wr, "cell1", 0, topodatapb.TabletType_MASTER, db)
@@ -150,20 +144,12 @@ func TestBackupRestore(t *testing.T) {
  }
 
  // run the backup
- if err := vp.Run([]string{"Backup", topoproto.TabletAliasString(sourceTablet.Tablet.Alias)}); err != nil {
- t.Fatalf("Backup failed: %v", err)
- }
+ require.NoError(t, vp.Run([]string{"Backup", topoproto.TabletAliasString(sourceTablet.Tablet.Alias)}))
 
  // verify the full status
- if err := sourceTablet.FakeMysqlDaemon.CheckSuperQueryList(); err != nil {
- t.Errorf("sourceTablet.FakeMysqlDaemon.CheckSuperQueryList failed: %v", err)
- }
- if !sourceTablet.FakeMysqlDaemon.Replicating {
- t.Errorf("sourceTablet.FakeMysqlDaemon.Replicating not set")
- }
- if !sourceTablet.FakeMysqlDaemon.Running {
- t.Errorf("sourceTablet.FakeMysqlDaemon.Running not set")
- }
+ require.NoError(t, sourceTablet.FakeMysqlDaemon.CheckSuperQueryList())
+ assert.True(t, sourceTablet.FakeMysqlDaemon.Replicating)
+ assert.True(t, sourceTablet.FakeMysqlDaemon.Running)
 
  // create a destination tablet, set it up so we can do restores
  destTablet := NewFakeTablet(t, wr, "cell1", 2, topodatapb.TabletType_REPLICA, db)
@@ -204,21 +190,59 @@ func TestBackupRestore(t *testing.T) {
  RelayLogInfoPath: path.Join(root, "relay-log.info"),
  }
 
- if err := destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */); err != nil {
- t.Fatalf("RestoreData failed: %v", err)
+ require.NoError(t, destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */))
+ // verify the full status
+ require.NoError(t, destTablet.FakeMysqlDaemon.CheckSuperQueryList(), "destTablet.FakeMysqlDaemon.CheckSuperQueryList failed")
+ assert.True(t, destTablet.FakeMysqlDaemon.Replicating)
+ assert.True(t, destTablet.FakeMysqlDaemon.Running)
+
+ // Initialize mycnf, required for restore
+ masterInnodbDataDir := path.Join(root, "master_innodb_data")
+ masterInnodbLogDir := path.Join(root, "master_innodb_log")
+ masterDataDir := path.Join(root, "master_data")
+ master.TM.Cnf = &mysqlctl.Mycnf{
+ DataDir: masterDataDir,
+ InnodbDataHomeDir: masterInnodbDataDir,
+ InnodbLogGroupHomeDir: masterInnodbLogDir,
+ BinLogPath: path.Join(root, "bin-logs/filename_prefix"),
+ RelayLogPath: path.Join(root, "relay-logs/filename_prefix"),
+ RelayLogIndexPath: path.Join(root, "relay-log.index"),
+ RelayLogInfoPath: path.Join(root, "relay-log.info"),
  }
 
- // verify the full status
- if err := destTablet.FakeMysqlDaemon.CheckSuperQueryList(); err != nil {
- t.Errorf("destTablet.FakeMysqlDaemon.CheckSuperQueryList failed: %v", err)
+ master.FakeMysqlDaemon.FetchSuperQueryMap = map[string]*sqltypes.Result{
+ "SHOW DATABASES": {},
  }
- if !destTablet.FakeMysqlDaemon.Replicating {
- t.Errorf("destTablet.FakeMysqlDaemon.Replicating not set")
+ master.FakeMysqlDaemon.ExpectedExecuteSuperQueryList = []string{
+ "STOP SLAVE",
+ "RESET SLAVE ALL",
+ "FAKE SET SLAVE POSITION",
+ "FAKE SET MASTER",
+ "START SLAVE",
  }
- if !destTablet.FakeMysqlDaemon.Running {
- t.Errorf("destTablet.FakeMysqlDaemon.Running not set")
+
+ master.FakeMysqlDaemon.SetReplicationPositionPos = master.FakeMysqlDaemon.CurrentMasterPosition
+
+ // restore master from backup
+ require.NoError(t, master.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */), "RestoreData failed")
+ // tablet was created as MASTER, so it's baseTabletType is MASTER
+ assert.Equal(t, topodatapb.TabletType_MASTER, master.Tablet.Type)
+ assert.False(t, master.FakeMysqlDaemon.Replicating)
+ assert.True(t, master.FakeMysqlDaemon.Running)
+
+ // restore master when database already exists
+ // checkNoDb should return false
+ // so fake the necessary queries
+ master.FakeMysqlDaemon.FetchSuperQueryMap = map[string]*sqltypes.Result{
+ "SHOW DATABASES": {Rows: [][]sqltypes.Value{{sqltypes.NewVarBinary("vt_test_keyspace")}}},
+ "SHOW TABLES FROM `vt_test_keyspace`": {Rows: [][]sqltypes.Value{{sqltypes.NewVarBinary("a")}}},
  }
 
+ require.NoError(t, master.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */), "RestoreData failed")
+ // Tablet type should not change
+ assert.Equal(t, topodatapb.TabletType_MASTER, master.Tablet.Type)
+ assert.False(t, master.FakeMysqlDaemon.Replicating)
+ assert.True(t, master.FakeMysqlDaemon.Running)
 }
 
 func TestRestoreUnreachableMaster(t *testing.T) {
@@ -252,9 +276,7 @@ func TestRestoreUnreachableMaster(t *testing.T) {
 
  // Initialize our temp dirs
  root, err := ioutil.TempDir("", "backuptest")
- if err != nil {
- t.Fatalf("os.TempDir failed: %v", err)
- }
+ require.NoError(t, err)
  defer os.RemoveAll(root)
 
  // Initialize BackupStorage
@@ -268,19 +290,11 @@ func TestRestoreUnreachableMaster(t *testing.T) {
  sourceDataDir := path.Join(root, "source_data")
  sourceDataDbDir := path.Join(sourceDataDir, "vt_db")
  for _, s := range []string{sourceInnodbDataDir, sourceInnodbLogDir, sourceDataDbDir} {
- if err := os.MkdirAll(s, os.ModePerm); err != nil {
- t.Fatalf("failed to create directory %v: %v", s, err)
- }
- }
- if err := ioutil.WriteFile(path.Join(sourceInnodbDataDir, "innodb_data_1"), []byte("innodb data 1 contents"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file innodb_data_1: %v", err)
- }
- if err := ioutil.WriteFile(path.Join(sourceInnodbLogDir, "innodb_log_1"), []byte("innodb log 1 contents"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file innodb_log_1: %v", err)
- }
- if err := ioutil.WriteFile(path.Join(sourceDataDbDir, "db.opt"), []byte("db opt file"), os.ModePerm); err != nil {
- t.Fatalf("failed to write file db.opt: %v", err)
+ require.NoError(t, os.MkdirAll(s, os.ModePerm))
  }
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceInnodbDataDir, "innodb_data_1"), []byte("innodb data 1 contents"), os.ModePerm))
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceInnodbLogDir, "innodb_log_1"), []byte("innodb log 1 contents"), os.ModePerm))
+ require.NoError(t, ioutil.WriteFile(path.Join(sourceDataDbDir, "db.opt"), []byte("db opt file"), os.ModePerm))
 
  // create a master tablet, set its master position
  master := NewFakeTablet(t, wr, "cell1", 0, topodatapb.TabletType_MASTER, db)
@@ -327,9 +341,7 @@ func TestRestoreUnreachableMaster(t *testing.T) {
  }
 
  // run the backup
- if err := vp.Run([]string{"Backup", topoproto.TabletAliasString(sourceTablet.Tablet.Alias)}); err != nil {
- t.Fatalf("Backup failed: %v", err)
- }
+ require.NoError(t, vp.Run([]string{"Backup", topoproto.TabletAliasString(sourceTablet.Tablet.Alias)}))
 
  // create a destination tablet, set it up so we can do restores
  destTablet := NewFakeTablet(t, wr, "cell1", 2, topodatapb.TabletType_REPLICA, db)
@@ -376,19 +388,9 @@ func TestRestoreUnreachableMaster(t *testing.T) {
  // set a short timeout so that we don't have to wait 30 seconds
  *topo.RemoteOperationTimeout = 2 * time.Second
  // Restore should still succeed
- if err := destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */); err != nil {
- t.Fatalf("RestoreData failed: %v", err)
- }
-
+ require.NoError(t, destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */))
  // verify the full status
- if err := destTablet.FakeMysqlDaemon.CheckSuperQueryList(); err != nil {
- t.Errorf("destTablet.FakeMysqlDaemon.CheckSuperQueryList failed: %v", err)
- }
- if !destTablet.FakeMysqlDaemon.Replicating {
- t.Errorf("destTablet.FakeMysqlDaemon.Replicating not set")
- }
- if !destTablet.FakeMysqlDaemon.Running {
- t.Errorf("destTablet.FakeMysqlDaemon.Running not set")
- }
-
+ require.NoError(t, destTablet.FakeMysqlDaemon.CheckSuperQueryList(), "destTablet.FakeMysqlDaemon.CheckSuperQueryList failed")
+ assert.True(t, destTablet.FakeMysqlDaemon.Replicating)
+ assert.True(t, destTablet.FakeMysqlDaemon.Running)
 }