diff --git a/src/internal/kopia/base_finder.go b/src/internal/kopia/base_finder.go index 83f4009c4..81082ded6 100644 --- a/src/internal/kopia/base_finder.go +++ b/src/internal/kopia/base_finder.go @@ -115,14 +115,6 @@ func (me ManifestEntry) GetTag(key string) (string, bool) { return v, ok } -type snapshotManager interface { - FindManifests( - ctx context.Context, - tags map[string]string, - ) ([]*manifest.EntryMetadata, error) - LoadSnapshot(ctx context.Context, id manifest.ID) (*snapshot.Manifest, error) -} - func serviceCatString(s path.ServiceType, c path.CategoryType) string { return s.String() + c.String() } diff --git a/src/internal/kopia/cleanup_backups.go b/src/internal/kopia/cleanup_backups.go new file mode 100644 index 000000000..b431b7a91 --- /dev/null +++ b/src/internal/kopia/cleanup_backups.go @@ -0,0 +1,156 @@ +package kopia + +import ( + "context" + "errors" + + "github.com/alcionai/clues" + "github.com/kopia/kopia/repo/manifest" + "github.com/kopia/kopia/snapshot" + "golang.org/x/exp/maps" + + "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/model" + "github.com/alcionai/corso/src/pkg/backup" + "github.com/alcionai/corso/src/pkg/logger" + "github.com/alcionai/corso/src/pkg/store" +) + +func cleanupOrphanedData( + ctx context.Context, + bs store.Storer, + mf manifestFinder, +) error { + // Get all snapshot manifests. + snaps, err := mf.FindManifests( + ctx, + map[string]string{ + manifest.TypeLabelKey: snapshot.ManifestType, + }) + if err != nil { + return clues.Wrap(err, "getting snapshots") + } + + var ( + // deets is a hash set of the ModelStoreID or snapshot IDs for backup + // details. It contains the IDs for both legacy details stored in the model + // store and newer details stored as a snapshot because it doesn't matter + // what the storage format is. We only need to know the ID so we can: + // 1. check if there's a corresponding backup for them + // 2. delete the details if they're orphaned + deets = map[manifest.ID]struct{}{} + // dataSnaps is a hash set of the snapshot IDs for item data snapshots. + dataSnaps = map[manifest.ID]struct{}{} + ) + + // TODO(ashmrtn): Exclude all snapshots and details younger than X . + // Doing so adds some buffer so that even if this is run concurrently with a + // backup it's not likely to delete models just being created. For example, + // running this when another corso instance has created an item data snapshot + // but hasn't yet created the details snapshot or the backup model would + // result in this instance of corso marking the newly created item data + // snapshot for deletion because it appears orphaned. + // + // Excluding only snapshots and details models works for now since the backup + // model is the last thing persisted out of them. If we switch the order of + // persistence then this will need updated as well. + // + // The buffer duration should be longer than the time it would take to do + // details merging and backup model creation. We don't have hard numbers on + // that, but it should be faster than creating the snapshot itself and + // probably happens O(minutes) or O(hours) instead of O(days). Of course, that + // assumes a non-adversarial setup where things such as machine hiberation, + // process freezing (i.e. paused at the OS level), etc. don't occur. + + // Sort all the snapshots as either details snapshots or item data snapshots. + for _, snap := range snaps { + k, _ := makeTagKV(TagBackupCategory) + if _, ok := snap.Labels[k]; ok { + dataSnaps[snap.ID] = struct{}{} + continue + } + + deets[snap.ID] = struct{}{} + } + + // Get all legacy backup details models. The initial version of backup delete + // didn't seem to delete them so they may also be orphaned if the repo is old + // enough. + deetsModels, err := bs.GetIDsForType(ctx, model.BackupDetailsSchema, nil) + if err != nil { + return clues.Wrap(err, "getting legacy backup details") + } + + for _, d := range deetsModels { + deets[d.ModelStoreID] = struct{}{} + } + + // Get all backup models. + bups, err := bs.GetIDsForType(ctx, model.BackupSchema, nil) + if err != nil { + return clues.Wrap(err, "getting all backup models") + } + + toDelete := maps.Clone(deets) + maps.Copy(toDelete, dataSnaps) + + for _, bup := range bups { + toDelete[manifest.ID(bup.ModelStoreID)] = struct{}{} + + bm := backup.Backup{} + + if err := bs.GetWithModelStoreID( + ctx, + model.BackupSchema, + bup.ModelStoreID, + &bm, + ); err != nil { + if !errors.Is(err, data.ErrNotFound) { + return clues.Wrap(err, "getting backup model"). + With("search_backup_id", bup.ID) + } + + // TODO(ashmrtn): This actually needs revised, see above TODO. Leaving it + // here for the moment to get the basic logic in. + // + // Safe to continue if the model wasn't found because that means that the + // possible item data and details for the backup are now orphaned. They'll + // be deleted since we won't remove them from the delete set. + // + // This isn't expected to really pop up, but it's possible if this + // function is run concurrently with either a backup delete or another + // instance of this function. + logger.Ctx(ctx).Debugw( + "backup model not found", + "search_backup_id", bup.ModelStoreID) + + continue + } + + ssid := bm.StreamStoreID + if len(ssid) == 0 { + ssid = bm.DetailsID + } + + _, dataOK := dataSnaps[manifest.ID(bm.SnapshotID)] + _, deetsOK := deets[manifest.ID(ssid)] + + // All data is present, we shouldn't garbage collect this backup. + if deetsOK && dataOK { + delete(toDelete, bup.ModelStoreID) + delete(toDelete, manifest.ID(bm.SnapshotID)) + delete(toDelete, manifest.ID(ssid)) + } + } + + // Use single atomic batch delete operation to cleanup to keep from making a + // bunch of manifest content blobs. + if err := bs.DeleteWithModelStoreIDs(ctx, maps.Keys(toDelete)...); err != nil { + return clues.Wrap(err, "deleting orphaned data") + } + + // TODO(ashmrtn): Do some pruning of assist backup models so we don't keep + // them around forever. + + return nil +} diff --git a/src/internal/kopia/cleanup_backups_test.go b/src/internal/kopia/cleanup_backups_test.go new file mode 100644 index 000000000..78bc6a164 --- /dev/null +++ b/src/internal/kopia/cleanup_backups_test.go @@ -0,0 +1,433 @@ +package kopia + +import ( + "context" + "fmt" + "testing" + + "github.com/alcionai/clues" + "github.com/kopia/kopia/repo/manifest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/model" + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/backup" +) + +type BackupCleanupUnitSuite struct { + tester.Suite +} + +func TestBackupCleanupUnitSuite(t *testing.T) { + suite.Run(t, &BackupCleanupUnitSuite{Suite: tester.NewUnitSuite(t)}) +} + +type mockManifestFinder struct { + t *testing.T + manifests []*manifest.EntryMetadata + err error +} + +func (mmf mockManifestFinder) FindManifests( + ctx context.Context, + tags map[string]string, +) ([]*manifest.EntryMetadata, error) { + assert.Equal( + mmf.t, + map[string]string{"type": "snapshot"}, + tags, + "snapshot search tags") + + return mmf.manifests, clues.Stack(mmf.err).OrNil() +} + +type mockStorer struct { + t *testing.T + + details []*model.BaseModel + detailsErr error + + backups []backupRes + backupListErr error + + expectDeleteIDs []manifest.ID + deleteErr error +} + +func (ms mockStorer) Delete(context.Context, model.Schema, model.StableID) error { + return clues.New("not implemented") +} + +func (ms mockStorer) Get(context.Context, model.Schema, model.StableID, model.Model) error { + return clues.New("not implemented") +} + +func (ms mockStorer) Put(context.Context, model.Schema, model.Model) error { + return clues.New("not implemented") +} + +func (ms mockStorer) Update(context.Context, model.Schema, model.Model) error { + return clues.New("not implemented") +} + +func (ms mockStorer) GetIDsForType( + _ context.Context, + s model.Schema, + tags map[string]string, +) ([]*model.BaseModel, error) { + assert.Empty(ms.t, tags, "model search tags") + + switch s { + case model.BackupDetailsSchema: + return ms.details, clues.Stack(ms.detailsErr).OrNil() + + case model.BackupSchema: + var bases []*model.BaseModel + + for _, b := range ms.backups { + bases = append(bases, &b.bup.BaseModel) + } + + return bases, clues.Stack(ms.backupListErr).OrNil() + } + + return nil, clues.New(fmt.Sprintf("unknown type: %s", s.String())) +} + +func (ms mockStorer) GetWithModelStoreID( + _ context.Context, + s model.Schema, + id manifest.ID, + m model.Model, +) error { + assert.Equal(ms.t, model.BackupSchema, s, "model get schema") + + d := m.(*backup.Backup) + + for _, b := range ms.backups { + if id == b.bup.ModelStoreID { + *d = *b.bup + return clues.Stack(b.err).OrNil() + } + } + + return clues.Stack(data.ErrNotFound) +} + +func (ms mockStorer) DeleteWithModelStoreIDs( + _ context.Context, + ids ...manifest.ID, +) error { + assert.ElementsMatch(ms.t, ms.expectDeleteIDs, ids, "model delete IDs") + return clues.Stack(ms.deleteErr).OrNil() +} + +// backupRes represents an individual return value for an item in GetIDsForType +// or the result of GetWithModelStoreID. err is used for GetWithModelStoreID +// only. +type backupRes struct { + bup *backup.Backup + err error +} + +func (suite *BackupCleanupUnitSuite) TestCleanupOrphanedData() { + backupTag, _ := makeTagKV(TagBackupCategory) + + // Current backup and snapshots. + bupCurrent := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID("current-bup-id"), + ModelStoreID: manifest.ID("current-bup-msid"), + }, + SnapshotID: "current-snap-msid", + StreamStoreID: "current-deets-msid", + } + + snapCurrent := &manifest.EntryMetadata{ + ID: "current-snap-msid", + Labels: map[string]string{ + backupTag: "0", + }, + } + + deetsCurrent := &manifest.EntryMetadata{ + ID: "current-deets-msid", + } + + // Legacy backup with details in separate model. + bupLegacy := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID("legacy-bup-id"), + ModelStoreID: manifest.ID("legacy-bup-msid"), + }, + SnapshotID: "legacy-snap-msid", + DetailsID: "legacy-deets-msid", + } + + snapLegacy := &manifest.EntryMetadata{ + ID: "legacy-snap-msid", + Labels: map[string]string{ + backupTag: "0", + }, + } + + deetsLegacy := &model.BaseModel{ + ID: "legacy-deets-id", + ModelStoreID: "legacy-deets-msid", + } + + // Incomplete backup missing data snapshot. + bupNoSnapshot := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID("ns-bup-id"), + ModelStoreID: manifest.ID("ns-bup-id-msid"), + }, + StreamStoreID: "ns-deets-msid", + } + + deetsNoSnapshot := &manifest.EntryMetadata{ + ID: "ns-deets-msid", + } + + // Legacy incomplete backup missing data snapshot. + bupLegacyNoSnapshot := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID("ns-legacy-bup-id"), + ModelStoreID: manifest.ID("ns-legacy-bup-id-msid"), + }, + DetailsID: "ns-legacy-deets-msid", + } + + deetsLegacyNoSnapshot := &model.BaseModel{ + ID: "ns-legacy-deets-id", + ModelStoreID: "ns-legacy-deets-msid", + } + + // Incomplete backup missing details. + bupNoDetails := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID("nssid-bup-id"), + ModelStoreID: manifest.ID("nssid-bup-msid"), + }, + SnapshotID: "nssid-snap-msid", + } + + snapNoDetails := &manifest.EntryMetadata{ + ID: "nssid-snap-msid", + Labels: map[string]string{ + backupTag: "0", + }, + } + + table := []struct { + name string + snapshots []*manifest.EntryMetadata + snapshotFetchErr error + // only need BaseModel here since we never look inside the details items. + detailsModels []*model.BaseModel + detailsModelListErr error + backups []backupRes + backupListErr error + deleteErr error + + expectDeleteIDs []manifest.ID + expectErr assert.ErrorAssertionFunc + }{ + { + name: "EmptyRepo", + expectErr: assert.NoError, + }, + { + name: "OnlyCompleteBackups Noops", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + deetsCurrent, + snapLegacy, + }, + detailsModels: []*model.BaseModel{ + deetsLegacy, + }, + backups: []backupRes{ + {bup: bupCurrent}, + {bup: bupLegacy}, + }, + expectErr: assert.NoError, + }, + { + name: "MissingFieldsInBackup CausesCleanup", + snapshots: []*manifest.EntryMetadata{ + snapNoDetails, + deetsNoSnapshot, + }, + detailsModels: []*model.BaseModel{ + deetsLegacyNoSnapshot, + }, + backups: []backupRes{ + {bup: bupNoSnapshot}, + {bup: bupLegacyNoSnapshot}, + {bup: bupNoDetails}, + }, + expectDeleteIDs: []manifest.ID{ + manifest.ID(bupNoSnapshot.ModelStoreID), + manifest.ID(bupLegacyNoSnapshot.ModelStoreID), + manifest.ID(bupNoDetails.ModelStoreID), + manifest.ID(deetsLegacyNoSnapshot.ModelStoreID), + snapNoDetails.ID, + deetsNoSnapshot.ID, + }, + expectErr: assert.NoError, + }, + { + name: "MissingSnapshot CausesCleanup", + snapshots: []*manifest.EntryMetadata{ + deetsCurrent, + }, + detailsModels: []*model.BaseModel{ + deetsLegacy, + }, + backups: []backupRes{ + {bup: bupCurrent}, + {bup: bupLegacy}, + }, + expectDeleteIDs: []manifest.ID{ + manifest.ID(bupCurrent.ModelStoreID), + deetsCurrent.ID, + manifest.ID(bupLegacy.ModelStoreID), + manifest.ID(deetsLegacy.ModelStoreID), + }, + expectErr: assert.NoError, + }, + { + name: "MissingDetails CausesCleanup", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + snapLegacy, + }, + backups: []backupRes{ + {bup: bupCurrent}, + {bup: bupLegacy}, + }, + expectDeleteIDs: []manifest.ID{ + manifest.ID(bupCurrent.ModelStoreID), + manifest.ID(bupLegacy.ModelStoreID), + snapCurrent.ID, + snapLegacy.ID, + }, + expectErr: assert.NoError, + }, + { + name: "SnapshotsListError Fails", + snapshotFetchErr: assert.AnError, + backups: []backupRes{ + {bup: bupCurrent}, + }, + expectErr: assert.Error, + }, + { + name: "LegacyDetailsListError Fails", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + }, + detailsModelListErr: assert.AnError, + backups: []backupRes{ + {bup: bupCurrent}, + }, + expectErr: assert.Error, + }, + { + name: "BackupIDsListError Fails", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + deetsCurrent, + }, + backupListErr: assert.AnError, + expectErr: assert.Error, + }, + { + name: "BackupModelGetErrorNotFound CausesCleanup", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + deetsCurrent, + snapLegacy, + snapNoDetails, + }, + detailsModels: []*model.BaseModel{ + deetsLegacy, + }, + backups: []backupRes{ + {bup: bupCurrent}, + { + bup: bupLegacy, + err: data.ErrNotFound, + }, + { + bup: bupNoDetails, + err: data.ErrNotFound, + }, + }, + // Backup IDs are still included in here because they're added to the + // deletion set prior to attempting to fetch models. The model store + // delete operation should ignore missing models though so there's no + // issue. + expectDeleteIDs: []manifest.ID{ + snapLegacy.ID, + manifest.ID(deetsLegacy.ModelStoreID), + manifest.ID(bupLegacy.ModelStoreID), + snapNoDetails.ID, + manifest.ID(bupNoDetails.ModelStoreID), + }, + expectErr: assert.NoError, + }, + { + name: "BackupModelGetError Fails", + snapshots: []*manifest.EntryMetadata{ + snapCurrent, + deetsCurrent, + snapLegacy, + snapNoDetails, + }, + detailsModels: []*model.BaseModel{ + deetsLegacy, + }, + backups: []backupRes{ + {bup: bupCurrent}, + { + bup: bupLegacy, + err: assert.AnError, + }, + {bup: bupNoDetails}, + }, + expectErr: assert.Error, + }, + } + + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + mbs := mockStorer{ + t: t, + details: test.detailsModels, + detailsErr: test.detailsModelListErr, + backups: test.backups, + backupListErr: test.backupListErr, + expectDeleteIDs: test.expectDeleteIDs, + deleteErr: test.deleteErr, + } + + mmf := mockManifestFinder{ + t: t, + manifests: test.snapshots, + err: test.snapshotFetchErr, + } + + err := cleanupOrphanedData(ctx, mbs, mmf) + test.expectErr(t, err, clues.ToCore(err)) + }) + } +} diff --git a/src/internal/kopia/conn.go b/src/internal/kopia/conn.go index 7eac9df5c..1703b466d 100644 --- a/src/internal/kopia/conn.go +++ b/src/internal/kopia/conn.go @@ -52,9 +52,26 @@ var ( } ) -type snapshotLoader interface { - SnapshotRoot(man *snapshot.Manifest) (fs.Entry, error) -} +type ( + manifestFinder interface { + FindManifests( + ctx context.Context, + tags map[string]string, + ) ([]*manifest.EntryMetadata, error) + } + + snapshotManager interface { + manifestFinder + LoadSnapshot( + ctx context.Context, + id manifest.ID, + ) (*snapshot.Manifest, error) + } + + snapshotLoader interface { + SnapshotRoot(man *snapshot.Manifest) (fs.Entry, error) + } +) var ( _ snapshotManager = &conn{}