Merge #120600

120600: backupccl: allow for empty database and schema only online restore r=dt a=msbutler This patch ensures online restore works on empty databases and for schema only restore. This patch also disallows verify_backup_table_data restores, which uses the traditional restore data processor. Release note: none Epic: none Co-authored-by: Michael Butler <[email protected]>
cockroachdb · Mar 26, 2024 · 215ece4 · 215ece4
2 parents 6df7436 + 5610266
commit 215ece4
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 6 deletions.
diff --git a/pkg/ccl/backupccl/backuprand/backup_rand_test.go b/pkg/ccl/backupccl/backuprand/backup_rand_test.go
@@ -165,7 +165,7 @@ database_name = 'rand' AND schema_name = 'public'`)
 
 	withOnlineRestore := func() string {
 		onlineRestoreExtension := ""
-		if rng.Intn(2) != 0 && runSchemaOnlyExtension == "" {
+		if rng.Intn(2) != 0 {
 			onlineRestoreExtension = " , experimental deferred copy"
 		}
 		return onlineRestoreExtension

diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go
@@ -1683,8 +1683,7 @@ func (r *restoreResumer) doResume(ctx context.Context, execCtx interface{}) erro
 	if err := maybeRelocateJobExecution(ctx, r.job.ID(), p, details.ExecutionLocality, "RESTORE"); err != nil {
 		return err
 	}
-
-	if len(details.DownloadSpans) > 0 {
+	if details.DownloadJob {
 		if err := p.ExecCfg().JobRegistry.CheckPausepoint("restore.before_do_download_files"); err != nil {
 			return err
 		}
@@ -1787,6 +1786,9 @@ func (r *restoreResumer) doResume(ctx context.Context, execCtx interface{}) erro
 				return err
 			}
 		}
+		if err := r.maybeWriteDownloadJob(ctx, p.ExecCfg(), preData, mainData); err != nil {
+			return err
+		}
 		emitRestoreJobEvent(ctx, p, jobs.StatusSucceeded, r.job)
 		return nil
 	}

diff --git a/pkg/ccl/backupccl/restore_online.go b/pkg/ccl/backupccl/restore_online.go
@@ -527,8 +527,11 @@ func (r *restoreResumer) maybeWriteDownloadJob(
 	downloadJobRecord := jobs.Record{
 		Description: fmt.Sprintf("Background Data Download for %s", r.job.Payload().Description),
 		Username:    r.job.Payload().UsernameProto.Decode(),
-		Details:     jobspb.RestoreDetails{DownloadSpans: downloadSpans, PostDownloadTableAutoStatsSettings: details.PostDownloadTableAutoStatsSettings},
-		Progress:    jobspb.RestoreProgress{},
+		Details: jobspb.RestoreDetails{
+			DownloadJob:                        true,
+			DownloadSpans:                      downloadSpans,
+			PostDownloadTableAutoStatsSettings: details.PostDownloadTableAutoStatsSettings},
+		Progress: jobspb.RestoreProgress{},
 	}
 
 	return execConfig.InternalDB.DescsTxn(ctx, func(

diff --git a/pkg/ccl/backupccl/restore_online_test.go b/pkg/ccl/backupccl/restore_online_test.go
@@ -215,6 +215,7 @@ func TestOnlineRestoreErrors(t *testing.T) {
 	defer cleanupFnRestored()
 	rSQLDB.Exec(t, "CREATE DATABASE data")
 	var (
+		fullBackup                = "nodelocal://1/full-backup"
 		fullBackupWithRevs        = "nodelocal://1/full-backup-with-revs"
 		incrementalBackup         = "nodelocal://1/incremental-backup"
 		incrementalBackupWithRevs = "nodelocal://1/incremental-backup-with-revs"
@@ -245,6 +246,11 @@ func TestOnlineRestoreErrors(t *testing.T) {
 		rSQLDB.ExpectErr(t, "scheme userfile is not accessible during node startup",
 			"RESTORE DATABASE bank FROM LATEST IN 'userfile:///my_backups' WITH EXPERIMENTAL DEFERRED COPY")
 	})
+	t.Run("verify_backup_table_data not supported", func(t *testing.T) {
+		sqlDB.Exec(t, fmt.Sprintf("BACKUP INTO '%s'", fullBackup))
+		sqlDB.ExpectErr(t, "cannot run online restore with verify_backup_table_data",
+			fmt.Sprintf("RESTORE data FROM LATEST IN '%s' WITH EXPERIMENTAL DEFERRED COPY, schema_only, verify_backup_table_data", fullBackup))
+	})
 }
 
 func bankOnlineRestore(

diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go
@@ -1346,6 +1346,10 @@ func restorePlanHook(
 		}
 	}
 
+	if restoreStmt.Options.ExperimentalOnline && restoreStmt.Options.VerifyData {
+		return nil, nil, nil, false, errors.New("cannot run online restore with verify_backup_table_data")
+	}
+
 	var newTenantID *roachpb.TenantID
 	var newTenantName *roachpb.TenantName
 	if restoreStmt.Options.AsTenant != nil || restoreStmt.Options.ForceTenantID != nil {

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/online-restore-empty-database b/pkg/ccl/backupccl/testdata/backup-restore/online-restore-empty-database
@@ -0,0 +1,25 @@
+# This test ensures that online restore works on an empty database.
+
+reset test-nodelocal
+----
+
+new-cluster name=s1 disable-tenant
+----
+
+
+exec-sql
+CREATE DATABASE d;
+----
+
+exec-sql
+BACKUP INTO 'nodelocal://1/cluster/';
+----
+
+exec-sql
+RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/cluster/' with EXPERIMENTAL DEFERRED COPY, new_db_name='d2';
+----
+
+query-sql retry
+SELECT count(*) FROM [SHOW JOBS] WHERE job_type='RESTORE' and status='succeeded';
+----
+2
diff --git a/pkg/jobs/jobspb/jobs.proto b/pkg/jobs/jobspb/jobs.proto
@@ -511,7 +511,9 @@ message RestoreDetails {
   // end of the download job. 
   map<uint32,cockroach.sql.catalog.catpb.AutoStatsSettings> post_download_table_auto_stats_settings = 35;
 
-  // NEXT ID: 36.
+  bool download_job = 36;
+
+  // NEXT ID: 37.
 }