Skip to content

Commit

Permalink
backupccl: ensure user passes locality aware uris with incremental_lo…
Browse files Browse the repository at this point in the history
…cation

Release note (sql change): This patch ensures the user passes the same number
of locality aware URIs for the full backup destination as the
incremental_location parameter. I.e.

Good:
`BACKUP INTO LATEST IN ($1, $2, $3) WITH incremental_location = ($4, $5, $6)`

Bad:
`BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)`

Note that the non locality uris for the full backup don't really affect
incremental backup planning -- the patch just adds guardrails to the UX. Further
work will ensure users cannot create incremental backup chains with mixed
localities (cockroachdb#79135)
  • Loading branch information
msbutler committed Apr 6, 2022
1 parent f981d44 commit b99c29e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
4 changes: 4 additions & 0 deletions pkg/ccl/backupccl/backup_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,10 @@ func backupPlanHook(
if !backupStmt.Nested && len(incrementalStorage) > 0 {
return errors.New("incremental_location option not supported with `BACKUP TO` syntax")
}
if len(incrementalStorage) > 0 && (len(incrementalStorage) != len(to)) {
return errors.New("the incremental_location option must contain the same number of locality" +
" aware URIs as the full backup destination")
}

endTime := p.ExecCfg().Clock.Now()
if backupStmt.AsOf.Expr != nil {
Expand Down
10 changes: 7 additions & 3 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,11 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
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)",
sqlDB.Exec(t, "BACKUP INTO LATEST IN ($1, $2, $3) WITH incremental_location = ($4, $5, $6)",
append(collections, incrementals...)...)

sqlDB.ExpectErr(t, "the incremental_location option must contain the same number of locality",
"BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)",
append(incrementals, collections[0])...)

// Find the subdirectory created by the full BACKUP INTO statement.
Expand All @@ -673,8 +677,8 @@ func TestBackupAndRestoreJobDescription(t *testing.T) {
collections[0], collections[1], collections[2])},
{fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s')", "/subdir",
collections[0], collections[1], collections[2])},
{fmt.Sprintf("BACKUP INTO '%s' IN '%s' WITH incremental_location = ('%s', '%s', '%s')",
"/subdir", collections[0], incrementals[0],
{fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s') WITH incremental_location = ('%s', '%s', '%s')",
"/subdir", collections[0], collections[1], collections[2], incrementals[0],
incrementals[1], incrementals[2])},
},
)
Expand Down

0 comments on commit b99c29e

Please sign in to comment.