backup: Allow for upgrade safe backups

go/cmd/vtbackup/vtbackup.go

@@ -107,6 +107,7 @@ var (
 	initialBackup       bool
 	allowFirstBackup    bool
 	restartBeforeBackup bool
+	upgradeSafe         bool
 	// vttablet-like flags
 	initDbNameOverride string
 	initKeyspace       string
@@ -135,6 +136,7 @@ func registerFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&initialBackup, "initial_backup", initialBackup, "Instead of restoring from backup, initialize an empty database with the provided init_db_sql_file and upload a backup of that for the shard, if the shard has no backups yet. This can be used to seed a brand new shard with an initial, empty backup. If any backups already exist for the shard, this will be considered a successful no-op. This can only be done before the shard exists in topology (i.e. before any tablets are deployed).")
 	fs.BoolVar(&allowFirstBackup, "allow_first_backup", allowFirstBackup, "Allow this job to take the first backup of an existing shard.")
 	fs.BoolVar(&restartBeforeBackup, "restart_before_backup", restartBeforeBackup, "Perform a mysqld clean/full restart after applying binlogs, but before taking the backup. Only makes sense to work around xtrabackup bugs.")
+	fs.BoolVar(&upgradeSafe, "upgrade_safe", upgradeSafe, "Whether to use innodb_fast_shutdown=0 for the backup so it is safe to use for MySQL upgrades.")
 	// vttablet-like flags
 	fs.StringVar(&initDbNameOverride, "init_db_name_override", initDbNameOverride, "(init parameter) override the name of the db used by vttablet")
 	fs.StringVar(&initKeyspace, "init_keyspace", initKeyspace, "(init parameter) keyspace to use for this tablet")
@@ -301,6 +303,7 @@ func takeBackup(ctx context.Context, topoServer *topo.Server, backupStorage back
 		Shard:              initShard,
 		TabletAlias:        topoproto.TabletAliasString(tabletAlias),
 		Stats:              backupstats.BackupStats(),
+		UpgradeSafe:        upgradeSafe,
 	}
 	// In initial_backup mode, just take a backup of this empty database.
 	if initialBackup {

go/flags/endtoend/vtbackup.txt

@@ -181,6 +181,7 @@ Usage of vtbackup:
       --topo_zk_tls_ca string                                       the server ca to use to validate servers when connecting to the zk topo server
       --topo_zk_tls_cert string                                     the cert to use to connect to the zk topo server, requires topo_zk_tls_key, enables TLS
       --topo_zk_tls_key string                                      the key to use to connect to the zk topo server, enables TLS
+      --upgrade_safe                                                Whether to use innodb_fast_shutdown=0 for the backup so it is safe to use for MySQL upgrades.
       --v Level                                                     log level for V logs
   -v, --version                                                     print binary version
       --vmodule moduleSpec                                          comma-separated list of pattern=N settings for file-filtered logging

go/test/endtoend/backup/vtctlbackup/backup_utils.go

@@ -810,6 +810,7 @@ func terminatedRestore(t *testing.T) {
 }
 
 func checkTabletType(t *testing.T, alias string, tabletType topodata.TabletType) {
+	t.Helper()
 	// for loop for 15 seconds to check if tablet type is correct
 	for i := 0; i < 15; i++ {
 		output, err := localCluster.VtctldClientProcess.ExecuteCommandWithOutput("GetTablet", alias)
@@ -1060,8 +1061,8 @@ func terminateRestore(t *testing.T) {
 				assert.Fail(t, "restore in progress file missing")
 			}
 			tmpProcess.Process.Signal(syscall.SIGTERM)
-			found = true //nolint
-			return
+			found = true
+			break
 		}
 	}
 	assert.True(t, found, "Restore message not found")

go/vt/mysqlctl/backup_test.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 	"os"
 	"path"
@@ -419,6 +420,134 @@ func TestRestoreTriesToParameterizeBackupStorage(t *testing.T) {
 	require.NotNil(t, scopedStats)
 }
 
+// TestRestoreManifestMySQLVersionValidation tests that Restore tries to validate
+// the MySQL version and safe upgrade attribute.
+func TestRestoreManifestMySQLVersionValidation(t *testing.T) {
+	testCases := []struct {
+		fromVersion, toVersion string
+		upgradeSafe            bool
+		wantErr                bool
+	}{
+		{
+			fromVersion: "mysqld  Ver 5.6.42",
+			toVersion:   "mysqld  Ver 5.7.40",
+			upgradeSafe: false,
+			wantErr:     true,
+		},
+		{
+			fromVersion: "mysqld  Ver 5.6.42",
+			toVersion:   "mysqld  Ver 5.7.40",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "mysqld  Ver 5.7.42",
+			toVersion:   "mysqld  Ver 8.0.32",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "mysqld  Ver 5.7.42",
+			toVersion:   "mysqld  Ver 8.0.32",
+			upgradeSafe: false,
+			wantErr:     true,
+		},
+		{
+			fromVersion: "mysqld  Ver 5.7.42",
+			toVersion:   "mysqld  Ver 8.0.32",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.32",
+			upgradeSafe: false,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.32",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.31",
+			upgradeSafe: false,
+			wantErr:     true,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.31",
+			upgradeSafe: true,
+			wantErr:     true,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.33",
+			upgradeSafe: false,
+			wantErr:     true,
+		},
+		{
+			fromVersion: "mysqld  Ver 8.0.32",
+			toVersion:   "mysqld  Ver 8.0.33",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "",
+			toVersion:   "mysqld  Ver 8.0.33",
+			upgradeSafe: false,
+			wantErr:     false,
+		},
+		{
+			fromVersion: "",
+			toVersion:   "mysqld  Ver 8.0.33",
+			upgradeSafe: true,
+			wantErr:     false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%s->%s upgradeSafe=%t", tc.fromVersion, tc.toVersion, tc.upgradeSafe), func(t *testing.T) {
+			env, closer := createFakeBackupRestoreEnv(t)
+			defer closer()
+			env.mysqld.Version = tc.toVersion
+
+			manifest := BackupManifest{
+				BackupTime:   time.Now().Add(-1 * time.Hour).Format(time.RFC3339),
+				BackupMethod: "fake",
+				Keyspace:     "test",
+				Shard:        "-",
+				MySQLVersion: tc.fromVersion,
+				UpgradeSafe:  tc.upgradeSafe,
+			}
+
+			manifestBytes, err := json.Marshal(manifest)
+			require.Nil(t, err)
+
+			env.backupEngine.ExecuteRestoreReturn = FakeBackupEngineExecuteRestoreReturn{&manifest, nil}
+			env.backupStorage.ListBackupsReturn = FakeBackupStorageListBackupsReturn{
+				BackupHandles: []backupstorage.BackupHandle{
+					&FakeBackupHandle{
+						ReadFileReturnF: func(context.Context, string) (io.ReadCloser, error) {
+							return io.NopCloser(bytes.NewBuffer(manifestBytes)), nil
+						},
+					},
+				},
+			}
+
+			_, err = Restore(env.ctx, env.restoreParams)
+			if tc.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+
+}
+
 type forTest []FileEntry
 
 func (f forTest) Len() int           { return len(f) }
@@ -490,6 +619,7 @@ func createFakeBackupRestoreEnv(t *testing.T) (*fakeBackupRestoreEnv, func()) {
 		BackupMethod: "fake",
 		Keyspace:     "test",
 		Shard:        "-",
+		MySQLVersion: "8.0.32",
 	}
 
 	manifestBytes, err := json.Marshal(manifest)

go/vt/mysqlctl/backupengine.go

@@ -73,22 +73,25 @@ type BackupParams struct {
 	IncrementalFromPos string
 	// Stats let's backup engines report detailed backup timings.
 	Stats backupstats.Stats
+	// UpgradeSafe indicates whether the backup is safe for upgrade and created with innodb_fast_shutdown=0
+	UpgradeSafe bool
 }
 
-func (b BackupParams) Copy() BackupParams {
+func (b *BackupParams) Copy() BackupParams {
 	return BackupParams{
-		b.Cnf,
-		b.Mysqld,
-		b.Logger,
-		b.Concurrency,
-		b.HookExtraEnv,
-		b.TopoServer,
-		b.Keyspace,
-		b.Shard,
-		b.TabletAlias,
-		b.BackupTime,
-		b.IncrementalFromPos,
-		b.Stats,
+		Cnf:                b.Cnf,
+		Mysqld:             b.Mysqld,
+		Logger:             b.Logger,
+		Concurrency:        b.Concurrency,
+		HookExtraEnv:       b.HookExtraEnv,
+		TopoServer:         b.TopoServer,
+		Keyspace:           b.Keyspace,
+		Shard:              b.Shard,
+		TabletAlias:        b.TabletAlias,
+		BackupTime:         b.BackupTime,
+		IncrementalFromPos: b.IncrementalFromPos,
+		Stats:              b.Stats,
+		UpgradeSafe:        b.UpgradeSafe,
 	}
 }
 
@@ -123,21 +126,21 @@ type RestoreParams struct {
 	Stats backupstats.Stats
 }
 
-func (p RestoreParams) Copy() RestoreParams {
+func (p *RestoreParams) Copy() RestoreParams {
 	return RestoreParams{
-		p.Cnf,
-		p.Mysqld,
-		p.Logger,
-		p.Concurrency,
-		p.HookExtraEnv,
-		p.DeleteBeforeRestore,
-		p.DbName,
-		p.Keyspace,
-		p.Shard,
-		p.StartTime,
-		p.RestoreToPos,
-		p.DryRun,
-		p.Stats,
+		Cnf:                 p.Cnf,
+		Mysqld:              p.Mysqld,
+		Logger:              p.Logger,
+		Concurrency:         p.Concurrency,
+		HookExtraEnv:        p.HookExtraEnv,
+		DeleteBeforeRestore: p.DeleteBeforeRestore,
+		DbName:              p.DbName,
+		Keyspace:            p.Keyspace,
+		Shard:               p.Shard,
+		StartTime:           p.StartTime,
+		RestoreToPos:        p.RestoreToPos,
+		DryRun:              p.DryRun,
+		Stats:               p.Stats,
 	}
 }
 
@@ -273,6 +276,12 @@ type BackupManifest struct {
 	Keyspace string
 
 	Shard string
+
+	// MySQLversion is the version of MySQL when the backup was taken.
+	MySQLVersion string
+
+	// UpgradeSafe indicates whether the backup is safe to use for an upgrade to a newer MySQL version
+	UpgradeSafe bool
 }
 
 func (m *BackupManifest) HashKey() string {
@@ -392,6 +401,11 @@ func FindBackupToRestore(ctx context.Context, params RestoreParams, bhs []backup
 	manifests := make([]*BackupManifest, len(bhs))
 	manifestHandleMap := NewManifestHandleMap()
 
+	mysqlVersion, err := params.Mysqld.GetVersionString(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	fullBackupIndex := func() int {
 		for index := len(bhs) - 1; index >= 0; index-- {
 			bh := bhs[index]
@@ -409,6 +423,14 @@ func FindBackupToRestore(ctx context.Context, params RestoreParams, bhs []backup
 				continue
 			}
 
+			// check if the backup can be used with this MySQL version.
+			if bm.MySQLVersion != "" {
+				if err := isMySQLVersionUpgradeCompatible(mysqlVersion, bm.MySQLVersion, bm.UpgradeSafe); err != nil {
+					params.Logger.Warningf("Skipping backup %v/%v with incompatible MySQL version %v (upgrade safe: %v): %v", backupDir, bh.Name(), bm.MySQLVersion, bm.UpgradeSafe, err)
+					continue
+				}
+			}
+
 			var backupTime time.Time
 			if checkBackupTime {
 				backupTime, err = time.Parse(time.RFC3339, bm.BackupTime)
@@ -461,14 +483,48 @@ func FindBackupToRestore(ctx context.Context, params RestoreParams, bhs []backup
 	// restore to a position (using incremental backups):
 	// we calculate a possible restore path based on the manifests. The resulting manifests are
 	// a sorted subsequence, with the full backup first, and zero or more incremental backups to follow.
-	manifests, err := FindPITRPath(params.RestoreToPos.GTIDSet, manifests)
+	restorePath.manifests, err = FindPITRPath(params.RestoreToPos.GTIDSet, manifests)
 	if err != nil {
 		return nil, err
 	}
-	restorePath.manifests = manifests
 	return restorePath, nil
 }
 
+func isMySQLVersionUpgradeCompatible(to string, from string, upgradeSafe bool) error {
+	// It's always safe to use the same version.
+	if to == from {
+		return nil
+	}
+
+	flavorTo, parsedTo, err := ParseVersionString(to)
+	if err != nil {
+		return err
+	}
+
+	flavorFrom, parsedFrom, err := ParseVersionString(from)
+	if err != nil {
+		return err
+	}
+
+	if flavorTo != flavorFrom {
+		return fmt.Errorf("cannot use backup between different flavors: %q vs. %q", from, to)
+	}
+
+	if parsedTo == parsedFrom {
+		return nil
+	}
+
+	if !parsedTo.atLeast(parsedFrom) {
+		return fmt.Errorf("running MySQL version %q is older than backup MySQL version %q", to, from)
+	}
+
+	if upgradeSafe {
+		return nil
+	}
+
+	return fmt.Errorf("running MySQL version %q is newer than backup MySQL version %q which is not safe to upgrade", to, from)
+}
+
 func prepareToRestore(ctx context.Context, cnf *Mycnf, mysqld MysqlDaemon, logger logutil.Logger) error {
 	// shutdown mysqld if it is running
 	logger.Infof("Restore: shutdown mysqld")