Skip to content

Commit

Permalink
backupccl: allow user to run SHOW BACKUP on locality aware backups
Browse files Browse the repository at this point in the history
Previously, the user could only run  SHOW BACKUP on the default locality of a
backup. This patch allows users to pass in all backup localities, allowing
future changes to SHOW BACKUP (e.g. cockroachdb#77694) to analyze data from backup files
in non default locaties.

Users can not yet run SHOW BACKUP for locality aware backups created using the
incremental_location parameter.

Informs cockroachdb#78632

Release note (sql change): users can pass locality aware backup URIs to SHOW
BACKUP. This change only affects SHOW BACKUP with the new syntax: e.g.:

SHOW BACKUP FROM LATEST IN (<collectionURI>, <localityURI1>, <localityURI2>)

Users can not yet run SHOW BACKUP for locality aware backups created using the
incremental_location parameter. I'm hoping to slip this into the 22.1 release!
  • Loading branch information
msbutler committed Apr 6, 2022
1 parent bb2c29c commit 6292014
Show file tree
Hide file tree
Showing 8 changed files with 283 additions and 190 deletions.
14 changes: 7 additions & 7 deletions docs/generated/sql/bnf/show_backup.bnf
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
show_backup_stmt ::=
'SHOW' 'BACKUPS' 'IN' location
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder
| 'SHOW' 'BACKUP' subdirectory 'IN' location 'WITH' kv_option_list
| 'SHOW' 'BACKUP' subdirectory 'IN' location 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' subdirectory 'IN' location
'SHOW' 'BACKUPS' 'IN' location_opt_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list 'WITH' kv_option_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list 'WITH' kv_option_list
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list
| 'SHOW' 'BACKUP' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' string_or_placeholder
Expand Down
6 changes: 3 additions & 3 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,9 @@ use_stmt ::=
'USE' var_value

show_backup_stmt ::=
'SHOW' 'BACKUPS' 'IN' string_or_placeholder
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' string_or_placeholder 'IN' string_or_placeholder opt_with_options
'SHOW' 'BACKUPS' 'IN' string_or_placeholder_opt_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_options
| 'SHOW' 'BACKUP' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_options
| 'SHOW' 'BACKUP' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'SCHEMAS' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder opt_with_options
Expand Down
41 changes: 41 additions & 0 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,12 +648,19 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
}

sqlDB.Exec(t, "BACKUP TO ($1, $2, $3)", backups...)
sqlDB.Exec(t, "BACKUP TO ($1,$2,$3) INCREMENTAL FROM $4", append(incrementals, backups[0])...)
sqlDB.Exec(t, "BACKUP INTO ($1, $2, $3)", collections...)
sqlDB.Exec(t, "BACKUP INTO LATEST IN ($1, $2, $3)", collections...)
sqlDB.Exec(t, "BACKUP INTO $4 IN ($1, $2, $3)", append(collections, "subdir")...)
sqlDB.Exec(t, "BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)",
append(incrementals, collections[0])...)

{
// Ensure old style show backup runs properly with locality aware uri
sqlDB.Exec(t, "SHOW BACKUP $1", backups[0])
sqlDB.Exec(t, "SHOW BACKUP $1", incrementals[0])
}

// Find the subdirectory created by the full BACKUP INTO statement.
matches, err := filepath.Glob(path.Join(tmpDir, "full/*/*/*/"+backupManifestName))
require.NoError(t, err)
Expand All @@ -667,6 +674,8 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
[][]string{
{fmt.Sprintf("BACKUP TO ('%s', '%s', '%s')", backups[0].(string), backups[1].(string),
backups[2].(string))},
{fmt.Sprintf("BACKUP TO ('%s', '%s', '%s') INCREMENTAL FROM '%s'", incrementals[0],
incrementals[1], incrementals[2], backups[0])},
{fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s')", full1, collections[0],
collections[1], collections[2])},
{fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s')", full1,
Expand Down Expand Up @@ -9470,6 +9479,7 @@ func TestBackupRestoreOldIncrementalDefault(t *testing.T) {

sib := fmt.Sprintf("BACKUP DATABASE fkdb INTO LATEST IN %s WITH incremental_location = %s", dest, inc)
sqlDB.Exec(t, sib)

sir := fmt.Sprintf("RESTORE DATABASE fkdb FROM LATEST IN %s WITH new_db_name = 'inc_fkdb'", dest)
sqlDB.Exec(t, sir)

Expand Down Expand Up @@ -9579,6 +9589,37 @@ func TestBackupRestoreSeparateExplicitIsDefault(t *testing.T) {

sib := fmt.Sprintf("BACKUP DATABASE fkdb INTO LATEST IN %s WITH incremental_location = %s", dest, inc)
sqlDB.Exec(t, sib)
{
// Locality Aware Show Backup validation
// TODO (msbutler): move to data driven test after 22.1 backport

// assert the localities field populates correctly (not null if backup is locality aware)
localities := sqlDB.QueryStr(t,
fmt.Sprintf("SELECT locality FROM [SHOW BACKUP FILES FROM LATEST IN %s]", dest))
for _, locality := range localities {
if len(br.dest) > 1 {
require.NotEqual(t, "NULL", locality[0])
} else {
require.Equal(t, "NULL", locality[0])
}
}
// assert show backup still works
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUPS IN %s", dest))
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s", dest))

if len(br.dest) > 1 {
// locality aware show backups will eventually fail if not all localities are provided,
// but for now, they're ok
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s", br.dest[1]))

errorMsg := "SHOW BACKUP on locality aware backups using incremental_location is not" +
" supported yet"
sqlDB.ExpectErr(t, errorMsg, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s WITH incremental_location= %s", dest, br.inc[0]))
} else {
// non locality aware show backup with incremental_location should work!
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s WITH incremental_location= %s", dest, inc))
}
}
sir := fmt.Sprintf("RESTORE DATABASE fkdb FROM LATEST IN %s WITH new_db_name = 'inc_fkdb', incremental_location = %s", dest, inc)
sqlDB.Exec(t, sir)

Expand Down
4 changes: 3 additions & 1 deletion pkg/ccl/backupccl/manifest_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,8 @@ func loadBackupManifests(
return backupManifests, memSize, nil
}

var errLocalityDescriptor = errors.New(`Locality Descriptor not found`)

// getLocalityInfo takes a list of stores and their URIs, along with the main
// backup manifest searches each for the locality pieces listed in the the
// main manifest, returning the mapping.
Expand Down Expand Up @@ -709,7 +711,7 @@ func getLocalityInfo(
}
}
if !found {
return info, errors.Errorf("expected manifest %s not found in backup locations", filename)
return info, errors.Mark(errors.Newf("expected manifest %s not found in backup locations", filename), errLocalityDescriptor)
}
}
info.URIsByOriginalLocalityKV = urisByOrigLocality
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1486,13 +1486,13 @@ func doRestorePlan(
defaultURIs, mainBackupManifests, localityInfo, memReserved, err = resolveBackupManifestsExplicitIncrementals(
ctx, &mem, mkStore, from, endTime, encryption, p.User())
}
defer func() {
mem.Shrink(ctx, memReserved)
}()

if err != nil {
return err
}
defer func() {
mem.Shrink(ctx, memReserved)
}()

currentVersion := p.ExecCfg().Settings.Version.ActiveVersion(ctx)
for i := range mainBackupManifests {
Expand Down
Loading

0 comments on commit 6292014

Please sign in to comment.