From 963d0e3fef96c59b94e1bb6fdd2c27f8e10070c9 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Wed, 24 Jan 2024 09:05:34 -0800 Subject: [PATCH] Skip base finding steps if we don't need bases (#5066) Minor optimization where we skip trying to find BackupBases if we know we're going to end up dropping them anyway. This can save us from loading all the kopia manifests only to discard the information we pulled from them. Note that this optimization only kicks in for backups that request no merge bases and no assist bases ("full backup") Existing test coverage in `operations/manifests_test.go` ensures this doesn't break base selection altogether As this is an optimization we don't need to merge this PR if it seems like it will make code maintenance more difficult going forward --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup * # #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/kopia/backup_bases.go | 4 ++++ src/internal/operations/manifests.go | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/internal/kopia/backup_bases.go b/src/internal/kopia/backup_bases.go index f18515d8f..e908e9aec 100644 --- a/src/internal/kopia/backup_bases.go +++ b/src/internal/kopia/backup_bases.go @@ -75,6 +75,10 @@ type BackupBases interface { SnapshotAssistBases() []BackupBase } +func EmptyBackupBase() BackupBases { + return &backupBases{} +} + type backupBases struct { mergeBases []BackupBase assistBases []BackupBase diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index ecabe6d97..f1305e825 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -24,6 +24,17 @@ func produceManifestsAndMetadata( tenantID string, getMetadata, dropAssistBases bool, ) (kopia.BackupBases, []data.RestoreCollection, bool, error) { + // Just return early if we're going to end up dropping all the bases anyway. + // This will avoid loading kopia manifest data in the case that we're doing a + // full enumeration and refetching all item data for the resource. + // + // It's easier to work through later logic if we know that the returned + // kopia.BackupBases is non-nil for non-error cases so return a trivial + // implementation here. + if !getMetadata && dropAssistBases { + return kopia.EmptyBackupBase(), nil, false, nil + } + bb, meta, useMergeBases, err := getManifestsAndMetadata( ctx, bf,