diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index 3bf2db5f7c37..580697ce63ce 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -2391,27 +2391,50 @@ func TestRestoreAsOfSystemTime(t *testing.T) { // fullBackup covers up to ts[2], inc to ts[5], inc2 to > ts[8]. sqlDB.ExpectErr( - t, "incompatible RESTORE timestamp", + t, "invalid RESTORE timestamp", fmt.Sprintf(`RESTORE data.* FROM $1 AS OF SYSTEM TIME %s WITH into_db='err'`, ts[3]), fullBackup, ) for _, i := range ts { - sqlDB.ExpectErr( - t, "incompatible RESTORE timestamp", - fmt.Sprintf(`RESTORE data.* FROM $1 AS OF SYSTEM TIME %s WITH into_db='err'`, i), - latestBackup, - ) - sqlDB.ExpectErr( - t, "incompatible RESTORE timestamp", - fmt.Sprintf(`RESTORE data.* FROM $1, $2, $3 AS OF SYSTEM TIME %s WITH into_db='err'`, i), - latestBackup, incLatestBackup, inc2LatestBackup, - ) + if i == ts[2] { + // latestBackup is _at_ ts2 so that is the time, and the only time, at + // which restoring it is allowed. + sqlDB.Exec( + t, fmt.Sprintf(`RESTORE data.* FROM $1 AS OF SYSTEM TIME %s WITH into_db='err'`, i), + latestBackup, + ) + sqlDB.Exec(t, `DROP DATABASE err; CREATE DATABASE err`) + } else { + sqlDB.ExpectErr( + t, "invalid RESTORE timestamp", + fmt.Sprintf(`RESTORE data.* FROM $1 AS OF SYSTEM TIME %s WITH into_db='err'`, i), + latestBackup, + ) + } + + if i == ts[2] || i == ts[5] { + // latestBackup is _at_ ts2 and incLatestBackup is at ts5, so either of + // those are valid for the chain (latest,incLatest,inc2Latest). In fact + // there's a third time -- that of inc2Latest, that is valid as well but + // it isn't fixed when created above so we know it / test for it. + sqlDB.Exec( + t, fmt.Sprintf(`RESTORE data.* FROM $1, $2, $3 AS OF SYSTEM TIME %s WITH into_db='err'`, i), + latestBackup, incLatestBackup, inc2LatestBackup, + ) + sqlDB.Exec(t, `DROP DATABASE err; CREATE DATABASE err`) + } else { + sqlDB.ExpectErr( + t, "invalid RESTORE timestamp", + fmt.Sprintf(`RESTORE data.* FROM $1, $2, $3 AS OF SYSTEM TIME %s WITH into_db='err'`, i), + latestBackup, incLatestBackup, inc2LatestBackup, + ) + } } sqlDB.ExpectErr( - t, "incompatible RESTORE timestamp", + t, "invalid RESTORE timestamp", fmt.Sprintf(`RESTORE data.* FROM $1 AS OF SYSTEM TIME %s WITH into_db='err'`, after), latestBackup, ) diff --git a/pkg/ccl/backupccl/manifest_handling.go b/pkg/ccl/backupccl/manifest_handling.go index 81a26d8d0ede..c5c2e4d22878 100644 --- a/pkg/ccl/backupccl/manifest_handling.go +++ b/pkg/ccl/backupccl/manifest_handling.go @@ -325,6 +325,7 @@ func resolveBackupManifests( baseStores []cloud.ExternalStorage, mkStore cloud.ExternalStorageFromURIFactory, from [][]string, + endTime hlc.Timestamp, encryption *roachpb.FileEncryptionOptions, ) ( defaultURIs []string, @@ -437,6 +438,57 @@ func resolveBackupManifests( } } } + + // Check that the requested target time, if specified, is valid for the list + // of incremental backups resolved, truncating the results to the backup that + // contains the target time. + if !endTime.IsEmpty() { + ok := false + for i, b := range mainBackupManifests { + // Find the backup that covers the requested time. + if b.StartTime.Less(endTime) && endTime.LessEq(b.EndTime) { + ok = true + + mainBackupManifests = mainBackupManifests[:i+1] + defaultURIs = defaultURIs[:i+1] + localityInfo = localityInfo[:i+1] + + // Ensure that the backup actually has revision history. + if !endTime.Equal(b.EndTime) { + if b.MVCCFilter != MVCCFilter_All { + const errPrefix = "invalid RESTORE timestamp: restoring to arbitrary time requires that BACKUP for requested time be created with '%s' option." + if i == 0 { + return nil, nil, nil, errors.Errorf( + errPrefix+" nearest backup time is %s", backupOptRevisionHistory, b.EndTime, + ) + } + return nil, nil, nil, errors.Errorf( + errPrefix+" nearest BACKUP times are %s or %s", + backupOptRevisionHistory, mainBackupManifests[i-1].EndTime, b.EndTime, + ) + } + // Ensure that the revision history actually covers the requested time - + // while the BACKUP's start and end might contain the requested time for + // example if start time is 0 (full backup), the revision history was + // only captured since the GC window. Note that the RevisionStartTime is + // the latest for ranges backed up. + if endTime.LessEq(b.RevisionStartTime) { + return nil, nil, nil, errors.Errorf( + "invalid RESTORE timestamp: BACKUP for requested time only has revision history from %v", b.RevisionStartTime, + ) + } + } + break + } + } + + if !ok { + return nil, nil, nil, errors.Errorf( + "invalid RESTORE timestamp: supplied backups do not cover requested time", + ) + } + } + return defaultURIs, mainBackupManifests, localityInfo, nil } diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index 97e132b2c148..b89f62368aef 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -697,7 +697,7 @@ func doRestorePlan( } defaultURIs, mainBackupManifests, localityInfo, err := resolveBackupManifests( - ctx, baseStores, p.ExecCfg().DistSQLSrv.ExternalStorageFromURI, from, encryption, + ctx, baseStores, p.ExecCfg().DistSQLSrv.ExternalStorageFromURI, from, endTime, encryption, ) if err != nil { return err @@ -728,37 +728,6 @@ func doRestorePlan( return err } - if !endTime.IsEmpty() { - ok := false - for _, b := range mainBackupManifests { - // Find the backup that covers the requested time. - if b.StartTime.Less(endTime) && endTime.LessEq(b.EndTime) { - ok = true - // Ensure that the backup actually has revision history. - if b.MVCCFilter != MVCCFilter_All { - return errors.Errorf( - "incompatible RESTORE timestamp: BACKUP for requested time needs option '%s'", backupOptRevisionHistory, - ) - } - // Ensure that the revision history actually covers the requested time - - // while the BACKUP's start and end might contain the requested time for - // example if start time is 0 (full backup), the revision history was - // only captured since the GC window. Note that the RevisionStartTime is - // the latest for ranges backed up. - if endTime.LessEq(b.RevisionStartTime) { - return errors.Errorf( - "incompatible RESTORE timestamp: BACKUP for requested time only has revision history from %v", b.RevisionStartTime, - ) - } - } - } - if !ok { - return errors.Errorf( - "incompatible RESTORE timestamp: supplied backups do not cover requested time", - ) - } - } - sqlDescs, restoreDBs, err := selectTargets(ctx, p, mainBackupManifests, restoreStmt.Targets, restoreStmt.DescriptorCoverage, endTime) if err != nil { return err