Skip to content

Commit 2165047

Browse files
craig[bot]kev-cao
andcommitted
Merge #152142
152142: backup: teach SHOW BACKUPS to use backup index r=msbutler a=kev-cao `SHOW BACKUPS` previously would find all full backups by delimiting on the `data/` prefix and listing all objects from the root store. This was not very performant as while the data SSTs in all the backups would not be listed, all of the other files would be. This commit teaches `SHOW BACKUPS` to use the new backup index introduced in #150384 when listing backups. This method relies on listing the top-level contents in the `index/` directory and delimiting on `/`, which means only the required objects are listed. This behavior is gated behind the `WITH INDEX` option that has been added to `SHOW BACKUPS`. This is because in 25.4, it is still possible to restore from backups made before the backup index was added. Since the new behavior relies entirely on the `index/`, we would miss any backups that do not have a corresponding index item created. To capture all backups in a collection, we'd need to list both from the `index/` and via the legacy path, which would be counterintuitive. Instead, we will instead rely on `WITH INDEX` until v26.1, at which point all restorable backups should have an index created for them. At that time, we will make `WITH INDEX` the default and use `WITH LEGACY LISTING` to list older backups. Epic: CRDB-47942 Fixes: #150328 Release note (general change): Added `WITH INDEX` option to `SHOW BACKUPS` for faster listing of 25.4+ backups. Co-authored-by: Kevin Cao <[email protected]>
2 parents 205e55a + e74c47c commit 2165047

File tree

12 files changed

+172
-31
lines changed

12 files changed

+172
-31
lines changed

docs/generated/sql/bnf/show_backup.bnf

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
show_backup_stmt ::=
2-
'SHOW' 'BACKUPS' 'IN' collectionURI
2+
'SHOW' 'BACKUPS' 'IN' collectionURI opt_with_show_backups_options
33
| 'SHOW' 'BACKUP' 'SCHEMAS' 'FROM' subdirectory 'IN' collectionURI 'WITH' show_backup_options ( ( ',' show_backup_options ) )*
44
| 'SHOW' 'BACKUP' 'SCHEMAS' 'FROM' subdirectory 'IN' collectionURI 'WITH' 'OPTIONS' '(' show_backup_options ( ( ',' show_backup_options ) )* ')'
55
| 'SHOW' 'BACKUP' 'SCHEMAS' 'FROM' subdirectory 'IN' collectionURI

docs/generated/sql/bnf/stmt_block.bnf

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ use_stmt ::=
840840
'USE' var_value
841841

842842
show_backup_stmt ::=
843-
'SHOW' 'BACKUPS' 'IN' string_or_placeholder_opt_list
843+
'SHOW' 'BACKUPS' 'IN' string_or_placeholder_opt_list opt_with_show_backups_options
844844
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_show_backup_options
845845
| 'SHOW' 'BACKUP' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_show_backup_options
846846

@@ -2114,6 +2114,11 @@ var_value ::=
21142114
a_expr
21152115
| extra_var_value
21162116

2117+
opt_with_show_backups_options ::=
2118+
'WITH' show_backups_options_list
2119+
| 'WITH' 'OPTIONS' '(' show_backups_options_list ')'
2120+
|
2121+
21172122
show_backup_details ::=
21182123
'SCHEMAS'
21192124

@@ -3003,6 +3008,9 @@ extra_var_value ::=
30033008
| 'NONE'
30043009
| cockroachdb_extra_reserved_keyword
30053010

3011+
show_backups_options_list ::=
3012+
( show_backups_options ) ( ( ',' show_backups_options ) )*
3013+
30063014
show_backup_options_list ::=
30073015
( show_backup_options ) ( ( ',' show_backup_options ) )*
30083016

@@ -3633,6 +3641,9 @@ for_locking_item ::=
36333641
var_list ::=
36343642
( var_value ) ( ( ',' var_value ) )*
36353643

3644+
show_backups_options ::=
3645+
'INDEX'
3646+
36363647
show_backup_options ::=
36373648
'AS_JSON'
36383649
| 'CHECK_FILES'

pkg/backup/backup_test.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1759,30 +1759,34 @@ func TestBackupRestoreResume(t *testing.T) {
17591759
if err != nil {
17601760
t.Fatal(err)
17611761
}
1762-
backupDir := dir + "/backup" + "-" + item.testName
1762+
collectionURI := fmt.Sprintf("nodelocal://1/%s", item.testName)
1763+
subdir := time.Now().Format(backupbase.DateBasedIntoFolderName)
17631764

1764-
if err := os.MkdirAll(backupDir+item.checkpointDirectory, 0755); err != nil {
1765+
checkpointDir := path.Join(dir, item.testName, subdir, item.checkpointDirectory)
1766+
if err := os.MkdirAll(checkpointDir, 0755); err != nil {
17651767
t.Fatal(err)
17661768
}
1767-
checkpointFile := backupDir + item.checkpointDirectory + "/" + backupinfo.BackupManifestCheckpointName
1769+
checkpointFile := path.Join(checkpointDir, backupinfo.BackupManifestCheckpointName)
17681770
if err := os.WriteFile(checkpointFile, mockManifest, 0644); err != nil {
17691771
t.Fatal(err)
17701772
}
1771-
uri := "nodelocal://1/backup" + "-" + item.testName
17721773
createAndWaitForJob(
17731774
t, sqlDB, []descpb.ID{backupTableDesc.GetID()},
17741775
jobspb.BackupDetails{
1776+
Destination: jobspb.BackupDetails_Destination{
1777+
Subdir: subdir,
1778+
},
17751779
EndTime: srv.Clock().Now(),
1776-
CollectionURI: uri,
1777-
URI: uri,
1780+
CollectionURI: collectionURI,
1781+
URI: fmt.Sprintf("%s%s", collectionURI, subdir),
17781782
},
17791783
jobspb.BackupProgress{},
17801784
roachpb.Version{},
17811785
)
17821786

17831787
// If the backup properly took the (incorrect) checkpoint into account, it
17841788
// won't have tried to re-export any keys within backupCompletedSpan.
1785-
backupManifestFile := backupDir + "/" + backupbase.BackupManifestName
1789+
backupManifestFile := path.Join(dir, item.testName, subdir, backupbase.BackupManifestName)
17861790
backupManifestBytes, err := os.ReadFile(backupManifestFile)
17871791
if err != nil {
17881792
t.Fatal(err)

pkg/backup/backupbase/constants.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ const (
6868
// to the directory containing the index files for the backup collection.
6969
BackupIndexDirectoryPath = "index/"
7070

71+
// BackupIndexFlattenedSubdir is the format used for the top-level
72+
// subdirectories within the index directory.
73+
BackupIndexFlattenedSubdir = "2006-01-02-150405.00"
74+
7175
// BackupIndexFilenameTimestampFormat is the format used for the human
7276
// readable start and end times in the index file names.
7377
BackupIndexFilenameTimestampFormat = "20060102-150405.00"

pkg/backup/backupdest/backup_destination.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,12 @@ func GetURIsByLocalityKV(
515515
// ListFullBackupsInCollection lists full backup paths in the collection
516516
// of an export store
517517
func ListFullBackupsInCollection(
518-
ctx context.Context, store cloud.ExternalStorage,
518+
ctx context.Context, store cloud.ExternalStorage, useIndex bool,
519519
) ([]string, error) {
520+
if useIndex {
521+
return ListSubdirsFromIndex(ctx, store)
522+
}
523+
520524
var backupPaths []string
521525
if err := store.List(ctx, "", backupbase.ListingDelimDataSlash, func(f string) error {
522526
if backupPathRE.MatchString(f) {

pkg/backup/backupdest/backup_index.go

Lines changed: 67 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,13 @@ func WriteBackupIndexMetadata(
120120
// words, we can remove these checks in v26.2+.
121121
func IndexExists(ctx context.Context, store cloud.ExternalStorage, subdir string) (bool, error) {
122122
var indexExists bool
123+
indexDir, err := indexSubdir(subdir)
124+
if err != nil {
125+
return false, err
126+
}
123127
if err := store.List(
124128
ctx,
125-
indexSubdir(subdir),
129+
indexDir,
126130
"/",
127131
func(file string) error {
128132
indexExists = true
@@ -150,9 +154,13 @@ func ListIndexes(
150154
ctx context.Context, store cloud.ExternalStorage, subdir string,
151155
) ([]string, error) {
152156
var indexBasenames []string
157+
indexDir, err := indexSubdir(subdir)
158+
if err != nil {
159+
return nil, err
160+
}
153161
if err := store.List(
154162
ctx,
155-
indexSubdir(subdir)+"/",
163+
indexDir+"/",
156164
"",
157165
func(file string) error {
158166
indexBasenames = append(indexBasenames, path.Base(file))
@@ -219,8 +227,12 @@ func GetBackupTreeIndexMetadata(
219227
g := ctxgroup.WithContext(ctx)
220228
for i, basename := range indexBasenames {
221229
g.GoCtx(func(ctx context.Context) error {
230+
indexDir, err := indexSubdir(subdir)
231+
if err != nil {
232+
return err
233+
}
222234
reader, size, err := store.ReadFile(
223-
ctx, path.Join(indexSubdir(subdir), basename), cloud.ReadOptions{},
235+
ctx, path.Join(indexDir, basename), cloud.ReadOptions{},
224236
)
225237
if err != nil {
226238
return errors.Wrapf(err, "reading index file %s", basename)
@@ -300,6 +312,30 @@ func parseIndexFilename(basename string) (start time.Time, end time.Time, err er
300312
return start, end, nil
301313
}
302314

315+
// ListSubdirsFromIndex lists the paths of all full backup subdirectories that
316+
// have an entry in the index. The store should be rooted at the default
317+
// collection URI. The subdirs are returned in chronological order.
318+
func ListSubdirsFromIndex(ctx context.Context, store cloud.ExternalStorage) ([]string, error) {
319+
var subdirs []string
320+
if err := store.List(
321+
ctx,
322+
backupbase.BackupIndexDirectoryPath,
323+
"/",
324+
func(indexSubdir string) error {
325+
indexSubdir = strings.TrimSuffix(indexSubdir, "/")
326+
subdir, err := unflattenIndexSubdir(indexSubdir)
327+
if err != nil {
328+
return err
329+
}
330+
subdirs = append(subdirs, subdir)
331+
return nil
332+
},
333+
); err != nil {
334+
return nil, errors.Wrapf(err, "listing index subdirs")
335+
}
336+
return subdirs, nil
337+
}
338+
303339
// shouldWriteIndex determines if a backup index file should be written for a
304340
// given backup. The rule is:
305341
// 1. An index should only be written on a v25.4+ cluster.
@@ -346,8 +382,12 @@ func getBackupIndexFilePath(subdir string, startTime, endTime hlc.Timestamp) (st
346382
if strings.EqualFold(subdir, backupbase.LatestFileName) {
347383
return "", errors.AssertionFailedf("expected subdir to be resolved and not be 'LATEST'")
348384
}
385+
indexDir, err := indexSubdir(subdir)
386+
if err != nil {
387+
return "", err
388+
}
349389
return backuputils.JoinURLPath(
350-
indexSubdir(subdir),
390+
indexDir,
351391
getBackupIndexFileName(startTime, endTime),
352392
), nil
353393
}
@@ -372,8 +412,12 @@ func getBackupIndexFileName(startTime, endTime hlc.Timestamp) string {
372412
// path for a given full backup subdir. The path is relative to the root of the
373413
// collection URI and does not contain a trailing slash. It assumes that subdir
374414
// has been resolved and is not `LATEST`.
375-
func indexSubdir(subdir string) string {
376-
return path.Join(backupbase.BackupIndexDirectoryPath, flattenSubdirForIndex(subdir))
415+
func indexSubdir(subdir string) (string, error) {
416+
flattened, err := flattenSubdirForIndex(subdir)
417+
if err != nil {
418+
return "", err
419+
}
420+
return path.Join(backupbase.BackupIndexDirectoryPath, flattened), nil
377421
}
378422

379423
// flattenSubdirForIndex flattens a full backup subdirectory to be used in the
@@ -393,11 +437,21 @@ func indexSubdir(subdir string) string {
393437
//
394438
// Listing on `index/` and delimiting on `/` will return the subdirectories
395439
// without listing the files in them.
396-
func flattenSubdirForIndex(subdir string) string {
397-
return strings.ReplaceAll(
398-
// Trimming any trailing and leading slashes guarantees a specific format when
399-
// returning the flattened subdir, so callers can expect a consistent result.
400-
strings.TrimSuffix(strings.TrimPrefix(subdir, "/"), "/"),
401-
"/", "-",
402-
)
440+
func flattenSubdirForIndex(subdir string) (string, error) {
441+
subdirTime, err := time.Parse(backupbase.DateBasedIntoFolderName, subdir)
442+
if err != nil {
443+
return "", errors.Wrapf(err, "parsing subdir %q for flattening", subdir)
444+
}
445+
return subdirTime.Format(backupbase.BackupIndexFlattenedSubdir), nil
446+
}
447+
448+
// unflattenIndexSubdir is the inverse of flattenSubdirForIndex. It converts a
449+
// flattened index subdir back to the original full backup subdir.
450+
func unflattenIndexSubdir(flattened string) (string, error) {
451+
subdirTime, err := time.Parse(backupbase.BackupIndexFlattenedSubdir, flattened)
452+
if err != nil {
453+
return "", errors.Wrapf(err, "parsing flattened index subdir %q for unflattening", flattened)
454+
}
455+
unflattened := subdirTime.Format(backupbase.DateBasedIntoFolderName)
456+
return unflattened, nil
403457
}

pkg/backup/backupdest/backup_index_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func TestDontWriteBackupIndexMetadata(t *testing.T) {
261261
return externalStorage, nil
262262
}
263263

264-
subdir := "2025/07/18-143826.00"
264+
subdir := "/2025/07/18-143826.00"
265265
details := jobspb.BackupDetails{
266266
Destination: jobspb.BackupDetails_Destination{
267267
To: []string{"nodelocal://1/backup"},
@@ -344,8 +344,8 @@ func TestIndexExists(t *testing.T) {
344344
ctx := context.Background()
345345

346346
const collectionURI = "nodelocal://1/backup"
347-
const subdir1 = "/2025/07/18-222500.00/"
348-
const subdir2 = "/2025/07/19-123456.00/"
347+
const subdir1 = "/2025/07/18-222500.00"
348+
const subdir2 = "/2025/07/19-123456.00"
349349
st := cluster.MakeTestingClusterSettingsWithVersions(
350350
clusterversion.Latest.Version(),
351351
clusterversion.Latest.Version(),

pkg/backup/show.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1396,7 +1396,7 @@ func showBackupsInCollectionPlanHook(
13961396
return errors.Wrapf(err, "connect to external storage")
13971397
}
13981398
defer store.Close()
1399-
res, err := backupdest.ListFullBackupsInCollection(ctx, store)
1399+
res, err := backupdest.ListFullBackupsInCollection(ctx, store, showStmt.Options.Index)
14001400
if err != nil {
14011401
return err
14021402
}

pkg/backup/show_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,9 +455,11 @@ func TestShowBackups(t *testing.T) {
455455
sqlDB.Exec(t, `BACKUP data.bank INTO LATEST IN $1 WITH incremental_location = $2`, full, remoteInc)
456456

457457
rows := sqlDBRestore.QueryStr(t, `SHOW BACKUPS IN $1`, full)
458+
rowsUsingIndex := sqlDBRestore.QueryStr(t, `SHOW BACKUPS IN $1 WITH INDEX`, full)
458459

459460
// assert that we see the three, and only the three, full backups.
460461
require.Equal(t, 3, len(rows))
462+
require.Equal(t, rows, rowsUsingIndex)
461463

462464
// check that we can show the inc layers in the individual full backups.
463465
b1 := sqlDBRestore.QueryStr(t, `SELECT * FROM [SHOW BACKUP FROM $1 IN $2] WHERE object_type='table'`, rows[0][0], full)

pkg/sql/parser/sql.y

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ func (u *sqlSymUnion) doBlockOption() tree.DoBlockOption {
14441444
%type <*tree.TenantReplicationOptions> opt_with_replication_options replication_options replication_options_list source_replication_options source_replication_options_list
14451445
%type <tree.ShowBackupDetails> show_backup_details
14461446
%type <*tree.ShowJobOptions> show_job_options show_job_options_list
1447-
%type <*tree.ShowBackupOptions> opt_with_show_backup_options show_backup_options show_backup_options_list
1447+
%type <*tree.ShowBackupOptions> opt_with_show_backup_options show_backup_options show_backup_options_list opt_with_show_backups_options show_backups_options show_backups_options_list
14481448
%type <*tree.CopyOptions> opt_with_copy_options copy_options copy_options_list copy_generic_options copy_generic_options_list
14491449
%type <str> import_format
14501450
%type <str> storage_parameter_key
@@ -3839,7 +3839,7 @@ alter_external_connection_stmt:
38393839
ConnectionLabelSpec: *($4.labelSpec()),
38403840
As: $6.expr(),
38413841
}
3842-
}
3842+
}
38433843
| ALTER EXTERNAL CONNECTION IF EXISTS /*$6=*/label_spec AS /*$8=*/string_or_placeholder
38443844
{
38453845
$$.val = &tree.AlterExternalConnection{
@@ -8771,10 +8771,11 @@ show_histogram_stmt:
87718771
// %Text: SHOW BACKUP [SCHEMAS|FILES|RANGES] <location>
87728772
// %SeeAlso: WEBDOCS/show-backup.html
87738773
show_backup_stmt:
8774-
SHOW BACKUPS IN string_or_placeholder_opt_list
8774+
SHOW BACKUPS IN string_or_placeholder_opt_list opt_with_show_backups_options
87758775
{
87768776
$$.val = &tree.ShowBackup{
87778777
InCollection: $4.stringOrPlaceholderOptList(),
8778+
Options: *$5.showBackupOptions(),
87788779
}
87798780
}
87808781
| SHOW BACKUP show_backup_details FROM string_or_placeholder IN string_or_placeholder_opt_list opt_with_show_backup_options
@@ -8857,6 +8858,38 @@ show_backup_details:
88578858
$$.val = tree.BackupValidateDetails
88588859
}
88598860

8861+
opt_with_show_backups_options:
8862+
WITH show_backups_options_list
8863+
{
8864+
$$.val = $2.showBackupOptions()
8865+
}
8866+
| WITH OPTIONS '(' show_backups_options_list ')'
8867+
{
8868+
$$.val = $4.showBackupOptions()
8869+
}
8870+
| /* EMPTY */
8871+
{
8872+
$$.val = &tree.ShowBackupOptions{}
8873+
}
8874+
8875+
show_backups_options_list:
8876+
show_backups_options
8877+
{
8878+
$$.val = $1.showBackupOptions()
8879+
}
8880+
| show_backups_options_list ',' show_backups_options
8881+
{
8882+
if err := $1.showBackupOptions().CombineWith($3.showBackupOptions()); err != nil {
8883+
return setErr(sqllex, err)
8884+
}
8885+
}
8886+
8887+
show_backups_options:
8888+
INDEX
8889+
{
8890+
$$.val = &tree.ShowBackupOptions{Index: true}
8891+
}
8892+
88608893
opt_with_show_backup_options:
88618894
WITH show_backup_options_list
88628895
{

0 commit comments

Comments
 (0)