Merge pull request #69700 from adityamaru/restore-system-table-orderi…

…ng-20.2

release-20.2: backupccl: add ordering to system table restore

pkg/ccl/backupccl/full_cluster_backup_restore_test.go

@@ -885,3 +885,37 @@ func TestReintroduceOfflineSpans(t *testing.T) {
 		destDB.ExpectErr(t, `relation "restoredb.bank" does not exist`, `SELECT count(*) FROM restoredb.bank`)
 	})
 }
+
+func TestClusterRestoreSystemTableOrdering(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	const numAccounts = 10
+	_, _, sqlDB, tempDir, cleanupFn := BackupRestoreTestSetup(t, singleNode, numAccounts, InitNone)
+	_, tcRestore, sqlDBRestore, cleanupEmptyCluster := backupRestoreTestSetupEmpty(t, singleNode,
+		tempDir, InitNone, base.TestClusterArgs{})
+	defer cleanupFn()
+	defer cleanupEmptyCluster()
+
+	restoredSystemTables := make([]string, 0)
+	for _, server := range tcRestore.Servers {
+		registry := server.JobRegistry().(*jobs.Registry)
+		registry.TestingResumerCreationKnobs = map[jobspb.Type]func(raw jobs.Resumer) jobs.Resumer{
+			jobspb.TypeRestore: func(raw jobs.Resumer) jobs.Resumer {
+				r := raw.(*restoreResumer)
+				r.testingKnobs.duringSystemTableRestoration = func(systemTableName string) error {
+					restoredSystemTables = append(restoredSystemTables, systemTableName)
+					return nil
+				}
+				return r
+			},
+		}
+	}
+
+	sqlDB.Exec(t, `BACKUP TO $1`, LocalFoo)
+	sqlDBRestore.Exec(t, `RESTORE FROM $1`, LocalFoo)
+	// Check that the settings table is the last of the system tables to be
+	// restored.
+	require.Equal(t, restoredSystemTables[len(restoredSystemTables)-1],
+		systemschema.SettingsTable.GetName())
+}

pkg/ccl/backupccl/restore_job.go

@@ -2064,13 +2064,29 @@ func (r *restoreResumer) restoreSystemTables(
 	executor := r.execCfg.InternalExecutor
 	var err error
 
-	// Iterate through all the tables that we're restoring, and if it was restored
-	// to the temporary system DB then copy it's data over to the real system
-	// table.
+	// Aggregate the system tables that are to be restored, and ensure that the
+	// settings table is restored last. This is important so that other system
+	// tables respect the cluster settings of the destination cluster during
+	// restore, rather than the settings being restored.
+	var settingsTableDesc catalog.TableDescriptor
+	systemTables := make([]catalog.TableDescriptor, 0)
 	for _, table := range tables {
 		if table.GetParentID() != tempSystemDBID {
 			continue
 		}
+		if table.GetName() == systemschema.SettingsTable.Name {
+			settingsTableDesc = table
+			continue
+		}
+		systemTables = append(systemTables, table)
+	}
+
+	// Append the settings table to the end of the system tables to be restored.
+	systemTables = append(systemTables, settingsTableDesc)
+
+	// Iterate through all the system tables that we're restoring, and copy their
+	// data over to the real system table.
+	for _, table := range systemTables {
 		systemTableName := table.GetName()
 		if details.SystemTablesRestored[systemTableName] {
 			// We've already restored this table.