Skip to content

Commit d559420

Browse files
committed
backup: write index full subdirs in reverse chronological order
Pre-requisite work outlined in the faster `SHOW BACKUP` [design doc](https://docs.google.com/document/d/1_HepOulhmHcKvBrc0o7fSTJ9RXF8wkKG72W0Qgtfca8). Closes:#158891 Release note: None
1 parent 84fdb98 commit d559420

File tree

6 files changed

+120
-48
lines changed

6 files changed

+120
-48
lines changed

pkg/backup/backupbase/constants.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,7 @@ const (
6363

6464
// BackupIndexDirectoryName is the path from the root of the backup collection
6565
// to the directory containing the index files for the backup collection.
66-
BackupIndexDirectoryPath = "index/"
67-
68-
// BackupIndexFlattenedSubdir is the format used for the top-level
69-
// subdirectories within the index directory.
70-
BackupIndexFlattenedSubdir = "2006-01-02-150405.00"
66+
BackupIndexDirectoryPath = backupMetadataDirectory + "/index/"
7167

7268
// BackupIndexFilenameTimestampFormat is the format used for the human
7369
// readable start and end times in the index file names.

pkg/backup/backupinfo/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ go_test(
7272
"//pkg/backup/backupbase",
7373
"//pkg/backup/backuppb",
7474
"//pkg/backup/backuptestutils",
75+
"//pkg/backup/backuputils",
7576
"//pkg/base",
7677
"//pkg/blobs",
7778
"//pkg/ccl",

pkg/backup/backupinfo/backup_index.go

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ func ListSubdirsFromIndex(ctx context.Context, store cloud.ExternalStorage) ([]s
332332
"/",
333333
func(indexSubdir string) error {
334334
indexSubdir = strings.TrimSuffix(indexSubdir, "/")
335-
subdir, err := unflattenIndexSubdir(indexSubdir)
335+
subdir, err := convertIndexSubdirToSubdir(indexSubdir)
336336
if err != nil {
337337
return err
338338
}
@@ -342,6 +342,10 @@ func ListSubdirsFromIndex(ctx context.Context, store cloud.ExternalStorage) ([]s
342342
); err != nil {
343343
return nil, errors.Wrapf(err, "listing index subdirs")
344344
}
345+
// Index subdirs are in descending order and we want chronological order.
346+
// TODO (kev-cao): In the new faster `SHOW BACKUP` effort, we will want
347+
// the subdirs in descending order.
348+
slices.Reverse(subdirs)
345349
return subdirs, nil
346350
}
347351

@@ -422,49 +426,71 @@ func getBackupIndexFileName(startTime, endTime hlc.Timestamp) string {
422426
// collection URI and does not contain a trailing slash. It assumes that subdir
423427
// has been resolved and is not `LATEST`.
424428
func indexSubdir(subdir string) (string, error) {
425-
flattened, err := flattenSubdirForIndex(subdir)
429+
flattened, err := convertSubdirToIndexSubdir(subdir)
426430
if err != nil {
427431
return "", err
428432
}
429433
return path.Join(backupbase.BackupIndexDirectoryPath, flattened), nil
430434
}
431435

432-
// flattenSubdirForIndex flattens a full backup subdirectory to be used in the
433-
// index. Note that this path does not contain a trailing or leading slash.
436+
// convertSubdirToIndexSubdir flattens a full backup subdirectory to be used in
437+
// the index. Note that this path does not contain a trailing or leading slash.
434438
// It assumes subdir is not `LATEST` and has been resolved.
435439
// We flatten the subdir so that when listing from the index, we can list with
436440
// the `index/` prefix and delimit on `/`. e.g.:
437441
//
438-
// index/
442+
// metadata/index/
439443
//
440-
// |_ 2025-08-13-120000.00/
444+
// |_ <desc_end_time>_20250813-120000.00/
441445
// | |_ <index_meta>.pb
442-
// |_ 2025-08-14-120000.00/
446+
// |_ <desc_end_time>_20250814-120000.00/
443447
// | |_ <index_meta>.pb
444-
// |_ 2025-08-14-120000.00/
448+
// |_ <desc_end_time>_20250814-120000.00/
445449
// |_ <index_meta>.pb
446450
//
447451
// Listing on `index/` and delimiting on `/` will return the subdirectories
448452
// without listing the files in them.
449-
func flattenSubdirForIndex(subdir string) (string, error) {
453+
func convertSubdirToIndexSubdir(subdir string) (string, error) {
450454
subdirTime, err := time.Parse(backupbase.DateBasedIntoFolderName, subdir)
451455
if err != nil {
452456
return "", errors.Wrapf(
453-
err, "subdir does not match format '%s'", backupbase.DateBasedIntoFolderName,
457+
err, "invalid subdir format: %s", subdir,
454458
)
455459
}
456-
return subdirTime.Format(backupbase.BackupIndexFlattenedSubdir), nil
460+
return fmt.Sprintf(
461+
"%s_%s",
462+
backuputils.EncodeDescendingTS(subdirTime),
463+
subdirTime.Format(backupbase.BackupIndexFilenameTimestampFormat),
464+
), nil
457465
}
458466

459-
// unflattenIndexSubdir is the inverse of flattenSubdirForIndex. It converts a
460-
// flattened index subdir back to the original full backup subdir.
461-
func unflattenIndexSubdir(flattened string) (string, error) {
462-
subdirTime, err := time.Parse(backupbase.BackupIndexFlattenedSubdir, flattened)
467+
// convertIndexSubdirToSubdir converts an index subdir back to the
468+
// original full backup subdir.
469+
func convertIndexSubdirToSubdir(flattened string) (string, error) {
470+
parts := strings.Split(flattened, "_")
471+
if len(parts) != 2 {
472+
return "", errors.Newf(
473+
"invalid index subdir format: %s", flattened,
474+
)
475+
}
476+
descSubdirTime, err := backuputils.DecodeDescendingTS(parts[0])
477+
if err != nil {
478+
return "", errors.Wrapf(
479+
err, "index subdir %s could not be decoded", flattened,
480+
)
481+
}
482+
// Validate that the two parts of the index subdir correspond to the same time.
483+
subdirTime, err := time.Parse(backupbase.BackupIndexFilenameTimestampFormat, parts[1])
463484
if err != nil {
464485
return "", errors.Wrapf(
465-
err, "index subdir does not match format %s", backupbase.BackupIndexFlattenedSubdir,
486+
err, "index subdir %s could not be decoded", flattened,
487+
)
488+
}
489+
if !descSubdirTime.Equal(subdirTime) {
490+
return "", errors.Newf(
491+
"index subdir %s has mismatched timestamps", flattened,
466492
)
467493
}
468-
unflattened := subdirTime.Format(backupbase.DateBasedIntoFolderName)
494+
unflattened := descSubdirTime.Format(backupbase.DateBasedIntoFolderName)
469495
return unflattened, nil
470496
}

pkg/backup/backupinfo/backup_index_test.go

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
2424
"github.com/cockroachdb/cockroach/pkg/backup/backuppb"
2525
"github.com/cockroachdb/cockroach/pkg/backup/backuptestutils"
26+
"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
2627
"github.com/cockroachdb/cockroach/pkg/base"
2728
"github.com/cockroachdb/cockroach/pkg/blobs"
2829
"github.com/cockroachdb/cockroach/pkg/cloud"
@@ -95,29 +96,6 @@ func TestGetBackupIndexFileName(t *testing.T) {
9596
)
9697
}
9798

98-
func TestGetBackupIndexFilePath(t *testing.T) {
99-
defer leaktest.AfterTest(t)()
100-
defer log.Scope(t).Close(t)
101-
102-
start, end := hlc.Timestamp{WallTime: 10}, hlc.Timestamp{WallTime: 20}
103-
t.Run("fails if subdir is 'LATEST' and unresolved", func(t *testing.T) {
104-
_, err := getBackupIndexFilePath("LATEST", start, end)
105-
require.Error(t, err)
106-
})
107-
t.Run("returns correct path for resolved subdir", func(t *testing.T) {
108-
subdir := "/2025/07/17-152115.00"
109-
flattenedSubdir := "2025-07-17-152115.00"
110-
indexPath, err := getBackupIndexFilePath(subdir, start, end)
111-
require.NoError(t, err)
112-
require.True(
113-
t, strings.HasPrefix(
114-
indexPath,
115-
path.Join(backupbase.BackupIndexDirectoryPath, flattenedSubdir),
116-
),
117-
)
118-
})
119-
}
120-
12199
func TestWriteBackupIndexMetadata(t *testing.T) {
122100
defer leaktest.AfterTest(t)()
123101
defer log.Scope(t).Close(t)
@@ -224,7 +202,7 @@ func TestWriteBackupIndexMetadataWithLocalityAwareBackups(t *testing.T) {
224202
))
225203
}
226204

227-
func TestWriteBackupindexMetadataWithSpecifiedIncrementalLocation(t *testing.T) {
205+
func TestWriteBackupIndexMetadataWithSpecifiedIncrementalLocation(t *testing.T) {
228206
defer leaktest.AfterTest(t)()
229207
defer log.Scope(t).Close(t)
230208

@@ -648,6 +626,66 @@ func TestGetBackupTreeIndexMetadata(t *testing.T) {
648626
}
649627
}
650628

629+
func TestConvertIndexSubdirToSubdir(t *testing.T) {
630+
defer leaktest.AfterTest(t)()
631+
defer log.Scope(t).Close(t)
632+
633+
endTime := time.Date(2025, 12, 5, 0, 0, 0, 0, time.UTC)
634+
endTimeAsSubdir := endTime.Format(backupbase.DateBasedIntoFolderName)
635+
endTimeDescEnc := backuputils.EncodeDescendingTS(endTime)
636+
endTimeIndexSuffix := endTime.Format(backupbase.BackupIndexFilenameTimestampFormat)
637+
638+
testcases := []struct {
639+
name string
640+
indexSubdir string
641+
expectedSubdir string
642+
error string
643+
}{
644+
{
645+
name: "valid index subdir",
646+
indexSubdir: endTimeDescEnc + "_" + endTimeIndexSuffix,
647+
expectedSubdir: endTimeAsSubdir,
648+
},
649+
{
650+
name: "index subdir missing two parts",
651+
indexSubdir: endTimeDescEnc,
652+
error: "invalid index subdir format",
653+
},
654+
{
655+
name: "index subdir with extra parts",
656+
indexSubdir: endTimeDescEnc + "_" + endTimeIndexSuffix + "_extra",
657+
error: "invalid index subdir format",
658+
},
659+
{
660+
name: "index subdir with invalid descending timestamp",
661+
indexSubdir: "invalid" + "_" + endTimeIndexSuffix,
662+
error: "could not be decoded",
663+
},
664+
{
665+
name: "index subdir with invalid timestamp suffix",
666+
indexSubdir: endTimeDescEnc + "_invalid",
667+
error: "could not be decoded",
668+
},
669+
{
670+
name: "index subdir with mismatched timestamps",
671+
indexSubdir: endTimeDescEnc + "_" + endTime.Add(time.Second).Format(backupbase.BackupIndexFilenameTimestampFormat),
672+
error: "mismatched timestamps",
673+
},
674+
}
675+
676+
for _, tc := range testcases {
677+
t.Run(tc.name, func(t *testing.T) {
678+
subdir, err := convertIndexSubdirToSubdir(tc.indexSubdir)
679+
if tc.error != "" {
680+
require.ErrorContains(t, err, tc.error)
681+
return
682+
}
683+
require.NoError(t, err)
684+
require.Equal(t, tc.expectedSubdir, subdir)
685+
})
686+
}
687+
}
688+
651689
type fakeExternalStorage struct {
652690
cloud.ExternalStorage
653691
files map[string]*closableBytesWriter

pkg/backup/backuputils/utils.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,19 @@ func EncodeDescendingTS(ts time.Time) string {
111111
return hex.EncodeToString(buffer)
112112
}
113113

114+
// DecodeDescendingTS decodes a time.Time encoded with EncodeDescendingTS.
115+
func DecodeDescendingTS(encoded string) (time.Time, error) {
116+
buffer, err := hex.DecodeString(encoded)
117+
if err != nil {
118+
return time.Time{}, err
119+
}
120+
_, tsMillis, err := encoding.DecodeUvarintDescending(buffer)
121+
if err != nil {
122+
return time.Time{}, err
123+
}
124+
return time.UnixMilli(int64(tsMillis)), nil
125+
}
126+
114127
// AbsoluteBackupPathInCollectionURI returns the absolute path of a backup
115128
// assuming the root is the collection URI. Backup URI represents the URI that
116129
// points to the directory containing the backup manifest of the backup. Since

pkg/backup/show_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -706,8 +706,6 @@ func TestShowBackupPathIsCollectionRoot(t *testing.T) {
706706
_, sqlDB, _, cleanupFn := backupRestoreTestSetup(t, singleNode, numAccounts, InitManualReplication)
707707
defer cleanupFn()
708708

709-
// Error output changes depending on whether or not the index is used. This
710-
// deterministically enables the index to make the error output consistent.
711709
sqlDB.Exec(t, "SET CLUSTER SETTING backup.index.read.enabled = true")
712710

713711
// Make an initial backup.

0 commit comments

Comments
 (0)