Skip to content

Commit

Permalink
backupccl: error message for restoring databases with AOST before GC TTL
Browse files Browse the repository at this point in the history
Previously, if a target database was not found for a RESTORE, whether it's due to AOST being before GC TTL or attempting to restore a database that has never existed, the same error message gets printed.

Since the errors do not occur for the same reason, having different error messages would be more informative for users to understand the cause of error.

Resolves: cockroachdb#47105

Release note (bug fix): more accurate error message for restoring AOST before GC TTL
  • Loading branch information
azhu-crl committed Jun 7, 2021
1 parent 7adbf53 commit 8988b1a
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 6 deletions.
24 changes: 24 additions & 0 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4120,6 +4120,30 @@ func TestRestoreAsOfSystemTimeGCBounds(t *testing.T) {
sqlDB.Exec(
t, fmt.Sprintf(`RESTORE data.bank FROM $1 AS OF SYSTEM TIME %s`, postGC), lateFullTableBackup,
)

t.Run("restore-pre-gc-aost", func(t *testing.T) {
backupPath := dir + "/tbl-before-gc"
_, _, sqlDB, _, cleanupFn := BackupRestoreTestSetup(t, singleNode, 0, InitManualReplication)
defer cleanupFn()

sqlDB.Exec(t, "CREATE DATABASE db")
sqlDB.Exec(t, "CREATE TABLE db.a (k int, v string)")
sqlDB.Exec(t, `BACKUP DATABASE db TO $1 WITH revision_history`, backupPath)

sqlDB.Exec(t, "DROP TABLE db.a")
sqlDB.ExpectErr(
t, `pq: failed to resolve targets in the BACKUP location specified by the RESTORE stmt, use SHOW BACKUP to find correct targets: table "db.a" does not exist, or invalid RESTORE timestamp: supplied backups do not cover requested time`,
fmt.Sprintf(`RESTORE TABLE db.a FROM $1 AS OF SYSTEM TIME %s`, preGC),
backupPath,
)

sqlDB.Exec(t, "DROP DATABASE db")
sqlDB.ExpectErr(
t, `pq: failed to resolve targets in the BACKUP location specified by the RESTORE stmt, use SHOW BACKUP to find correct targets: unknown database "db", or invalid RESTORE timestamp: supplied backups do not cover requested time`,
fmt.Sprintf(`RESTORE DATABASE db FROM $1 AS OF SYSTEM TIME %s`, preGC),
backupPath,
)
})
}

func TestAsOfSystemTimeOnRestoredData(t *testing.T) {
Expand Down
15 changes: 12 additions & 3 deletions pkg/ccl/backupccl/backupresolver/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ func DescriptorsMatchingTargets(
searchPath sessiondata.SearchPath,
descriptors []catalog.Descriptor,
targets tree.TargetList,
asOf hlc.Timestamp,
) (DescriptorsMatched, error) {
ret := DescriptorsMatched{}

Expand All @@ -258,11 +259,16 @@ func DescriptorsMatchingTargets(

alreadyRequestedDBs := make(map[descpb.ID]struct{})
alreadyExpandedDBs := make(map[descpb.ID]struct{})
invalidRestoreTsErr := errors.Errorf("supplied backups do not cover requested time")
// Process all the DATABASE requests.
for _, d := range targets.Databases {
dbID, ok := resolver.DbsByName[string(d)]
if !ok {
return ret, errors.Errorf("unknown database %q", d)
// given a non-empty aost, it's possible that we're unable to find the database due to aost being set before gc ttl
if asOf.IsEmpty() {
return ret, errors.Errorf("database %q does not exist", d)
}
return ret, errors.Wrapf(invalidRestoreTsErr, "database %q does not exist, or invalid RESTORE timestamp", d)
}
if _, ok := alreadyRequestedDBs[dbID]; !ok {
desc := resolver.DescByID[dbID]
Expand Down Expand Up @@ -355,7 +361,10 @@ func DescriptorsMatchingTargets(
p.ObjectNamePrefix = prefix
doesNotExistErr := errors.Errorf(`table %q does not exist`, tree.ErrString(p))
if !found {
return ret, doesNotExistErr
if asOf.IsEmpty() {
return ret, doesNotExistErr
}
return ret, errors.Wrapf(invalidRestoreTsErr, `table %q does not exist, or invalid RESTORE timestamp`, tree.ErrString(p))
}
tableDesc, isTable := descI.(catalog.TableDescriptor)
// If the type assertion didn't work, then we resolved a type instead, so
Expand Down Expand Up @@ -513,7 +522,7 @@ func ResolveTargetsToDescriptors(

var matched DescriptorsMatched
if matched, err = DescriptorsMatchingTargets(ctx,
p.CurrentDatabase(), p.CurrentSearchPath(), allDescs, *targets); err != nil {
p.CurrentDatabase(), p.CurrentSearchPath(), allDescs, *targets, endTime); err != nil {
return nil, nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backupresolver/targets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func TestDescriptorsMatchingTargets(t *testing.T) {
targets := stmt.AST.(*tree.Grant).Targets

matched, err := DescriptorsMatchingTargets(context.Background(),
test.sessionDatabase, searchPath, descriptors, targets)
test.sessionDatabase, searchPath, descriptors, targets, hlc.Timestamp{})
if test.err != "" {
if !testutils.IsError(err, test.err) {
t.Fatalf("expected error matching '%v', but got '%v'", test.err, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/targets.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func selectTargets(
}

matched, err := backupresolver.DescriptorsMatchingTargets(ctx,
p.CurrentDatabase(), p.CurrentSearchPath(), allDescs, targets)
p.CurrentDatabase(), p.CurrentSearchPath(), allDescs, targets, asOf)
if err != nil {
return nil, nil, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/testdata/backup-restore/temp-tables
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public
exec-sql
BACKUP TABLE temp_table TO 'nodelocal://0/temp_table_backup'
----
pq: failed to resolve targets specified in the BACKUP stmt: table "temp_table" does not exist
pq: failed to resolve targets specified in the BACKUP stmt: table "temp_table" does not exist, or invalid RESTORE timestamp: supplied backups do not cover requested time

exec-sql
BACKUP DATABASE d1 TO 'nodelocal://0/d1_backup/'
Expand Down

0 comments on commit 8988b1a

Please sign in to comment.