Skip to content

Commit ef18807

Browse files
committed
backup: remove endtime filter from GetBackupTreeIndexMetadata
This patch removes the `endTime` parameter from `GetBackupTreeIndexMetadata`. The filtering will already be done by `ValidateEndTimeAndTruncate` and so is unnecessary. The addition of this filtering results in a change in the error messages when an invalid end time is provided, so for the sake of simplicity, it's better to remove the parameter. Epic: None Release note: None
1 parent d8d00d8 commit ef18807

File tree

2 files changed

+8
-100
lines changed

2 files changed

+8
-100
lines changed

pkg/backup/backupdest/backup_index.go

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -204,18 +204,11 @@ func ListIndexes(
204204
}
205205

206206
// GetBackupTreeIndexMetadata concurrently retrieves the index metadata for all
207-
// backups within the specified subdir, up to the specified end time, inclusive.
208-
// The store should be rooted at the collection URI that contains the `index/`
209-
// directory. Indexes are returned in ascending end time order, with ties broken
210-
// by ascending start time order. If the end time is not covered by the backups
211-
// in the subdir, an error is returned.
212-
//
213-
// Note: If endTime is provided, GetBackupTreeIndexMetadata will return ALL
214-
// backups that could be used to restore to endTime. So even if a compacted
215-
// backup can be used to restore to endTime, the incremental backups that
216-
// make up the compacted backup will also be returned.
207+
// backups within the specified subdir. The store should be rooted at the
208+
// collection URI that contains the `index/` directory. Indexes are returned in
209+
// ascending end time order, with ties broken by ascending start time order.
217210
func GetBackupTreeIndexMetadata(
218-
ctx context.Context, store cloud.ExternalStorage, subdir string, endTime hlc.Timestamp,
211+
ctx context.Context, store cloud.ExternalStorage, subdir string,
219212
) ([]backuppb.BackupIndexMetadata, error) {
220213
indexBasenames, err := ListIndexes(ctx, store, subdir)
221214
if err != nil {
@@ -252,25 +245,7 @@ func GetBackupTreeIndexMetadata(
252245
return nil, errors.Wrapf(err, "getting backup index metadata")
253246
}
254247

255-
if endTime.IsEmpty() {
256-
return indexes, nil
257-
}
258-
259-
coveringIdx := slices.IndexFunc(indexes, func(index backuppb.BackupIndexMetadata) bool {
260-
return index.StartTime.Less(endTime) && endTime.LessEq(index.EndTime)
261-
})
262-
if coveringIdx == -1 {
263-
return nil, errors.Newf(`backups in "%s" do not cover end time %s`, subdir, endTime)
264-
}
265-
coverEndTime := indexes[coveringIdx].EndTime
266-
// To include all components of a compacted backup, we need to include all
267-
// backups with the same end time.
268-
for ; coveringIdx < len(indexes); coveringIdx++ {
269-
if !indexes[coveringIdx].EndTime.Equal(coverEndTime) {
270-
break
271-
}
272-
}
273-
return indexes[:coveringIdx], nil
248+
return indexes, nil
274249
}
275250

276251
// ParseBackupFilePathFromIndexFileName parses the path to a backup given the

pkg/backup/backupdest/backup_index_test.go

Lines changed: 3 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -565,94 +565,28 @@ func TestGetBackupTreeIndexMetadata(t *testing.T) {
565565
testcases := []struct {
566566
name string
567567
chain chain
568-
// endTime filter. Set to 0 for no filter.
569-
endTime int
570-
error string
568+
error string
571569
// expectedIndexTimes should be sorted in ascending order by end time, with
572570
// ties broken by ascending start time.
573571
expectedIndexTimes chain
574572
}{
575573
{
576-
name: "fetch all indexes from subdir",
574+
name: "fetch all indexes from chain with no compacted backups",
577575
chain: simpleChain,
578576
expectedIndexTimes: [][2]int{{0, 2}, {2, 4}, {4, 6}, {6, 8}},
579577
},
580-
{
581-
name: "exact end time match",
582-
chain: simpleChain,
583-
endTime: 6,
584-
expectedIndexTimes: [][2]int{{0, 2}, {2, 4}, {4, 6}},
585-
},
586-
{
587-
name: "end time between an incremental",
588-
chain: simpleChain,
589-
endTime: 5,
590-
expectedIndexTimes: [][2]int{{0, 2}, {2, 4}, {4, 6}},
591-
},
592-
{
593-
name: "end time before full backup end",
594-
chain: simpleChain,
595-
endTime: 1,
596-
expectedIndexTimes: chain{{0, 2}},
597-
},
598-
{
599-
name: "end time after the chain",
600-
chain: simpleChain,
601-
endTime: 10,
602-
error: "do not cover end time",
603-
},
604578
{
605579
name: "fetch all indexes from tree with compacted backups",
606580
chain: compactedChain,
607581
expectedIndexTimes: chain{{0, 10}, {10, 11}, {10, 12}, {11, 12}, {12, 14}, {14, 16}},
608582
},
609-
{
610-
name: "end time of compacted backup",
611-
chain: compactedChain,
612-
endTime: 12,
613-
expectedIndexTimes: chain{{0, 10}, {10, 11}, {10, 12}, {11, 12}},
614-
},
615-
{
616-
name: "end time between incremental after compacted backup",
617-
chain: compactedChain,
618-
endTime: 13,
619-
expectedIndexTimes: chain{{0, 10}, {10, 11}, {10, 12}, {11, 12}, {12, 14}},
620-
},
621-
{
622-
name: "end time between compacted backup",
623-
chain: compactedChain,
624-
endTime: 11,
625-
expectedIndexTimes: chain{{0, 10}, {10, 11}},
626-
},
627-
{
628-
name: "end time before compacted backup",
629-
chain: compactedChain,
630-
endTime: 11,
631-
expectedIndexTimes: chain{{0, 10}, {10, 11}},
632-
},
633583
{
634584
name: "fetch all indexes from tree with double compacted backups",
635585
chain: doubleCompactedChain,
636586
expectedIndexTimes: chain{
637587
{0, 18}, {18, 20}, {18, 22}, {20, 22}, {22, 24}, {18, 26}, {24, 26},
638588
},
639589
},
640-
{
641-
name: "end time before second compacted backup but after first",
642-
chain: doubleCompactedChain,
643-
endTime: 24,
644-
expectedIndexTimes: chain{
645-
{0, 18}, {18, 20}, {18, 22}, {20, 22}, {22, 24},
646-
},
647-
},
648-
{
649-
name: "end time of second compacted backup",
650-
chain: doubleCompactedChain,
651-
endTime: 26,
652-
expectedIndexTimes: chain{
653-
{0, 18}, {18, 20}, {18, 22}, {20, 22}, {22, 24}, {18, 26}, {24, 26},
654-
},
655-
},
656590
{
657591
name: "index only contains a full backup",
658592
chain: fullOnly,
@@ -664,9 +598,8 @@ func TestGetBackupTreeIndexMetadata(t *testing.T) {
664598
t.Run(tc.name, func(t *testing.T) {
665599
subdirTS := intToTime(tc.chain[0][1]).GoTime()
666600
subdir := subdirTS.Format(backupbase.DateBasedIntoFolderName)
667-
end := intToTime(tc.endTime)
668601

669-
metadatas, err := GetBackupTreeIndexMetadata(ctx, externalStorage, subdir, end)
602+
metadatas, err := GetBackupTreeIndexMetadata(ctx, externalStorage, subdir)
670603
if tc.error != "" {
671604
require.ErrorContains(t, err, tc.error)
672605
return

0 commit comments

Comments
 (0)