Merge #45368

45368:  backupccl: allow picking a backup with AS OF SYSTEM time r=dt a=dt

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 <ts of c>`.

Release note (enterprise change): RESTORE allows using AS OF SYSTEM TIME to pick a target backup from a larger list of incremental backups.

Co-authored-by: David Taylor <[email protected]>

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,
 		)

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
 }
 

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