From 1257570d0977d358978d7cd72fc2101afc6256ea Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Tue, 12 Sep 2023 18:11:42 -0700 Subject: [PATCH] Turn merge bases into assist bases when disabling them (#4182) Have disable merge bases turn all merge bases into assist bases instead of completely removing them. This allows us to continue getting benefits of kopia assisted incrementals and fixes the not having the backup models for details merging --- #### Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * fixes #4178 * #3943 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- CHANGELOG.md | 1 + src/internal/kopia/backup_bases.go | 22 +++++++++---------- src/internal/kopia/backup_bases_test.go | 28 ++++++++++++++++++------- src/internal/model/model.go | 19 +++++++++++++++-- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 775b0eea3..a75f3e835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Contacts backups no longer slices root-folder data if outlook is set to languages other than english. +- Failed backups if the --disable-incrementals flag was passed when there was a valid merge base under some conditions. ## [v0.12.0] (beta) - 2023-08-29 diff --git a/src/internal/kopia/backup_bases.go b/src/internal/kopia/backup_bases.go index d9e240132..18624236b 100644 --- a/src/internal/kopia/backup_bases.go +++ b/src/internal/kopia/backup_bases.go @@ -48,9 +48,6 @@ type backupBases struct { // disableAssistBases denote whether any assist bases should be returned to // kopia during snapshot operation. disableAssistBases bool - // disableMergeBases denotes whether any bases should be returned from calls - // to MergeBases(). - disableMergeBases bool } func (bb *backupBases) SnapshotAssistBases() []ManifestEntry { @@ -99,10 +96,6 @@ func (bb *backupBases) ConvertToAssistBase(manifestID manifest.ID) { } func (bb backupBases) Backups() []BackupEntry { - if bb.disableMergeBases { - return nil - } - return slices.Clone(bb.backups) } @@ -131,15 +124,20 @@ func (bb *backupBases) MinBackupVersion() int { } func (bb backupBases) MergeBases() []ManifestEntry { - if bb.disableMergeBases { - return nil - } - return slices.Clone(bb.mergeBases) } func (bb *backupBases) DisableMergeBases() { - bb.disableMergeBases = true + // Turn all merge bases into assist bases. We don't want to remove them + // completely because we still want to allow kopia assisted incrementals + // unless that's also explicitly disabled. However, we can't just leave them + // in the merge set since then we won't return the bases when merging backup + // details. + bb.assistBases = append(bb.assistBases, bb.mergeBases...) + bb.assistBackups = append(bb.assistBackups, bb.backups...) + + bb.mergeBases = nil + bb.backups = nil } func (bb backupBases) UniqueAssistBases() []ManifestEntry { diff --git a/src/internal/kopia/backup_bases_test.go b/src/internal/kopia/backup_bases_test.go index 7ef16ec13..3ff1488e5 100644 --- a/src/internal/kopia/backup_bases_test.go +++ b/src/internal/kopia/backup_bases_test.go @@ -8,6 +8,7 @@ import ( "github.com/kopia/kopia/snapshot" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" + "golang.org/x/exp/slices" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/tester" @@ -201,10 +202,20 @@ func (suite *BackupBasesUnitSuite) TestConvertToAssistBase() { func (suite *BackupBasesUnitSuite) TestDisableMergeBases() { t := suite.T() + + merge := []BackupEntry{ + {Backup: &backup.Backup{BaseModel: model.BaseModel{ID: "m1"}}}, + {Backup: &backup.Backup{BaseModel: model.BaseModel{ID: "m2"}}}, + } + assist := []BackupEntry{ + {Backup: &backup.Backup{BaseModel: model.BaseModel{ID: "a1"}}}, + {Backup: &backup.Backup{BaseModel: model.BaseModel{ID: "a2"}}}, + } + bb := &backupBases{ - backups: make([]BackupEntry, 2), + backups: slices.Clone(merge), mergeBases: make([]ManifestEntry, 2), - assistBackups: make([]BackupEntry, 2), + assistBackups: slices.Clone(assist), assistBases: make([]ManifestEntry, 2), } @@ -212,12 +223,15 @@ func (suite *BackupBasesUnitSuite) TestDisableMergeBases() { assert.Empty(t, bb.Backups()) assert.Empty(t, bb.MergeBases()) - // Assist base set should be unchanged. - assert.Len(t, bb.UniqueAssistBackups(), 2) - assert.Len(t, bb.UniqueAssistBases(), 2) - // Merge bases should still appear in the assist base set passed in for kopia - // snapshots. + // Assist base set now has what used to be merge bases. + assert.Len(t, bb.UniqueAssistBases(), 4) assert.Len(t, bb.SnapshotAssistBases(), 4) + // Merge bases should appear in the set of backup bases used for details + // merging. + assert.ElementsMatch( + t, + append(slices.Clone(merge), assist...), + bb.UniqueAssistBackups()) } func (suite *BackupBasesUnitSuite) TestDisableAssistBases() { diff --git a/src/internal/model/model.go b/src/internal/model/model.go index fb72e3613..0bb73d321 100644 --- a/src/internal/model/model.go +++ b/src/internal/model/model.go @@ -36,8 +36,23 @@ const ( const ( ServiceTag = "service" BackupTypeTag = "backup-type" - AssistBackup = "assist-backup" - MergeBackup = "merge-backup" + // AssistBackup denotes that this backup should only be used for kopia + // assisted incrementals since it doesn't contain the complete set of data + // being backed up. + // + // This tag should only be used for finding backups during a + // manifest search. It shouldn't be used to differentiate between backups once + // the manifest search completes. + AssistBackup = "assist-backup" + // MergeBackup denotes that this backup can be used as a merge base during an + // incremental backup. It contains a complete snapshot of the data in the + // external service. Merge bases can also be used as assist bases during an + // incremental backup or demoted to being only an assist base. + // + // This tag should only be used for finding backups during a + // manifest search. It shouldn't be used to differentiate between backups once + // the manifest search completes. + MergeBackup = "merge-backup" ) // Valid returns true if the ModelType value fits within the const range.