From 561026609daeaa959851a604508df282382267d0 Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Sat, 16 Mar 2024 18:24:02 -0600 Subject: [PATCH] backupccl: allow for empty database and schema only online restore 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 --- .../backupccl/backuprand/backup_rand_test.go | 2 +- pkg/ccl/backupccl/restore_job.go | 6 +++-- pkg/ccl/backupccl/restore_online.go | 7 ++++-- pkg/ccl/backupccl/restore_online_test.go | 6 +++++ pkg/ccl/backupccl/restore_planning.go | 4 +++ .../online-restore-empty-database | 25 +++++++++++++++++++ pkg/jobs/jobspb/jobs.proto | 4 ++- 7 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 pkg/ccl/backupccl/testdata/backup-restore/online-restore-empty-database diff --git a/pkg/ccl/backupccl/backuprand/backup_rand_test.go b/pkg/ccl/backupccl/backuprand/backup_rand_test.go index 2f910cee255b..8bcf074d23eb 100644 --- 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 index f5267ff393d1..b66c56d4a272 100644 --- 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 index 7cde6166e0b9..cc4bec16ca7d 100644 --- 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 index c344e3173dfa..f57986d8e08d 100644 --- 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 index 609a368ade8b..f79944cf8ae2 100644 --- 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 new file mode 100644 index 000000000000..4ad85ac43415 --- /dev/null +++ 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 \ No newline at end of file diff --git a/pkg/jobs/jobspb/jobs.proto b/pkg/jobs/jobspb/jobs.proto index dee8d43147a5..7c34b91e1a89 100644 --- a/pkg/jobs/jobspb/jobs.proto +++ b/pkg/jobs/jobspb/jobs.proto @@ -511,7 +511,9 @@ message RestoreDetails { // end of the download job. map post_download_table_auto_stats_settings = 35; - // NEXT ID: 36. + bool download_job = 36; + + // NEXT ID: 37. }