From 04f52d255f4d83f01e18203d66a7d85d3e8e9291 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 20 Feb 2020 15:26:19 +0000 Subject: [PATCH] backupccl: allow picking a backup with AS OF SYSTEM time This change extends the support for picking, via AS OF SYSTEM TIME, any backup in the chain provided to RESTORE as the target to RESTORE to. For revision-history backups, this already works: one can specify any point in time covered anywhere in the chain, as long as the backup covering that time has revision history. For non-revision-history backups however, the only way to restore to a particular backup, say 'c' from the chain ('a', 'b', 'c', 'd'), was to explicitly say `RESTORE FROM a, b, c` and elide the rest of the chain from the RESTORE command. In practice, when dealing with large numbers of incremental backups, is is often easier to simply maintain the full list of backups and paste it in its entirety than try to manage it by hand and specify a prefix. Furthermore, with "appeandable" backups that automatically create and discover the list of incremental layers implicitly rather than specifying it by hand, there's not an option of specifying a prefix. This change thus allows specifying the timestamp of a backup in the chain provided to RESTORE by using AS OF SYSTEM TIME even if that backup was *not* created with revision history to effectively truncate the chain to that point, e.g. making it possible to say `RESTORE FROM a, b, c, d AS OF SYSTEM TIME `. Release note (enterprise change): RESTORE allows using AS OF SYSTEM TIME to pick a target backup from a larger list of incremental backups. --- pkg/ccl/backupccl/backup_test.go | 47 +++++++++++++++++------ pkg/ccl/backupccl/manifest_handling.go | 52 ++++++++++++++++++++++++++ pkg/ccl/backupccl/restore_planning.go | 33 +--------------- 3 files changed, 88 insertions(+), 44 deletions(-) diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index f1e1ec820d34..30fd17d16c8e 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -2378,27 +2378,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