From cb50832d508906be8a5845519147f01b921154df Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 20 Jan 2024 15:58:06 +0100 Subject: [PATCH] index: let MasterIndex.Save also delete obsolete indexes --- cmd/restic/cmd_prune.go | 32 +++++++++--------- cmd/restic/cmd_repair_index.go | 2 +- cmd/restic/cmd_repair_packs.go | 2 +- internal/index/master_index.go | 50 +++++++++++++++++++---------- internal/index/master_index_test.go | 12 +------ internal/repository/repack_test.go | 30 ++++++----------- internal/restic/repository.go | 9 +++++- 7 files changed, 69 insertions(+), 68 deletions(-) diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index 10abbf9f0..efd8f6e3a 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -15,6 +15,7 @@ import ( "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/ui" + "github.com/restic/restic/internal/ui/progress" "github.com/spf13/cobra" ) @@ -766,7 +767,7 @@ func doPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo r return errors.Fatalf("%s", err) } } else if len(plan.ignorePacks) != 0 { - err = rebuildIndexFiles(ctx, gopts, repo, plan.ignorePacks, nil) + err = rebuildIndexFiles(ctx, gopts, repo, plan.ignorePacks, nil, false) if err != nil { return errors.Fatalf("%s", err) } @@ -778,7 +779,7 @@ func doPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo r } if opts.unsafeRecovery { - _, err = writeIndexFiles(ctx, gopts, repo, plan.ignorePacks, nil) + err = rebuildIndexFiles(ctx, gopts, repo, plan.ignorePacks, nil, true) if err != nil { return errors.Fatalf("%s", err) } @@ -788,23 +789,22 @@ func doPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo r return nil } -func writeIndexFiles(ctx context.Context, gopts GlobalOptions, repo restic.Repository, removePacks restic.IDSet, extraObsolete restic.IDs) (restic.IDSet, error) { +func rebuildIndexFiles(ctx context.Context, gopts GlobalOptions, repo restic.Repository, removePacks restic.IDSet, extraObsolete restic.IDs, skipDeletion bool) error { Verbosef("rebuilding index\n") bar := newProgressMax(!gopts.Quiet, 0, "packs processed") - obsoleteIndexes, err := repo.Index().Save(ctx, repo, removePacks, extraObsolete, bar) - bar.Done() - return obsoleteIndexes, err -} - -func rebuildIndexFiles(ctx context.Context, gopts GlobalOptions, repo restic.Repository, removePacks restic.IDSet, extraObsolete restic.IDs) error { - obsoleteIndexes, err := writeIndexFiles(ctx, gopts, repo, removePacks, extraObsolete) - if err != nil { - return err - } - - Verbosef("deleting obsolete index files\n") - return DeleteFilesChecked(ctx, gopts, repo, obsoleteIndexes, restic.IndexFile) + return repo.Index().Save(ctx, repo, removePacks, extraObsolete, restic.MasterIndexSaveOpts{ + SaveProgress: bar, + DeleteProgress: func() *progress.Counter { + return newProgressMax(!gopts.Quiet, 0, "old indexes deleted") + }, + DeleteReport: func(id restic.ID, err error) { + if gopts.verbosity > 2 { + Verbosef("removed index %v\n", id.String()) + } + }, + SkipDeletion: skipDeletion, + }) } func getUsedBlobs(ctx context.Context, repo restic.Repository, ignoreSnapshots restic.IDSet, quiet bool) (usedBlobs restic.CountedBlobSet, err error) { diff --git a/cmd/restic/cmd_repair_index.go b/cmd/restic/cmd_repair_index.go index c8a94b470..fc5506b34 100644 --- a/cmd/restic/cmd_repair_index.go +++ b/cmd/restic/cmd_repair_index.go @@ -154,7 +154,7 @@ func rebuildIndex(ctx context.Context, opts RepairIndexOptions, gopts GlobalOpti } } - err = rebuildIndexFiles(ctx, gopts, repo, removePacks, obsoleteIndexes) + err = rebuildIndexFiles(ctx, gopts, repo, removePacks, obsoleteIndexes, false) if err != nil { return err } diff --git a/cmd/restic/cmd_repair_packs.go b/cmd/restic/cmd_repair_packs.go index 723bdbccb..c572e02c5 100644 --- a/cmd/restic/cmd_repair_packs.go +++ b/cmd/restic/cmd_repair_packs.go @@ -145,7 +145,7 @@ func repairPacks(ctx context.Context, gopts GlobalOptions, repo *repository.Repo bar.Done() // remove salvaged packs from index - err = rebuildIndexFiles(ctx, gopts, repo, ids, nil) + err = rebuildIndexFiles(ctx, gopts, repo, ids, nil, false) if err != nil { return errors.Fatalf("%s", err) } diff --git a/internal/index/master_index.go b/internal/index/master_index.go index 073c9ace4..4c114b955 100644 --- a/internal/index/master_index.go +++ b/internal/index/master_index.go @@ -9,7 +9,6 @@ import ( "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/restic" - "github.com/restic/restic/internal/ui/progress" "golang.org/x/sync/errgroup" ) @@ -267,23 +266,22 @@ func (mi *MasterIndex) MergeFinalIndexes() error { // Save saves all known indexes to index files, leaving out any // packs whose ID is contained in packBlacklist from finalized indexes. -// The new index contains the IDs of all known indexes in the "supersedes" -// field. The IDs are also returned in the IDSet obsolete. -// After calling this function, you should remove the obsolete index files. -func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, packBlacklist restic.IDSet, extraObsolete restic.IDs, p *progress.Counter) (obsolete restic.IDSet, err error) { - p.SetMax(uint64(len(mi.Packs(packBlacklist)))) +// It also removes the old index files and those listed in extraObsolete. +func (mi *MasterIndex) Save(ctx context.Context, repo restic.Repository, excludePacks restic.IDSet, extraObsolete restic.IDs, opts restic.MasterIndexSaveOpts) error { + p := opts.SaveProgress + p.SetMax(uint64(len(mi.Packs(excludePacks)))) mi.idxMutex.Lock() defer mi.idxMutex.Unlock() - debug.Log("start rebuilding index of %d indexes, pack blacklist: %v", len(mi.idx), packBlacklist) + debug.Log("start rebuilding index of %d indexes, excludePacks: %v", len(mi.idx), excludePacks) newIndex := NewIndex() - obsolete = restic.NewIDSet() + obsolete := restic.NewIDSet() // track spawned goroutines using wg, create a new context which is // cancelled as soon as an error occurs. - wg, ctx := errgroup.WithContext(ctx) + wg, wgCtx := errgroup.WithContext(ctx) ch := make(chan *Index) @@ -310,21 +308,21 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack debug.Log("adding index %d", i) - for pbs := range idx.EachByPack(ctx, packBlacklist) { + for pbs := range idx.EachByPack(wgCtx, excludePacks) { newIndex.StorePack(pbs.PackID, pbs.Blobs) p.Add(1) if IndexFull(newIndex, mi.compress) { select { case ch <- newIndex: - case <-ctx.Done(): - return ctx.Err() + case <-wgCtx.Done(): + return wgCtx.Err() } newIndex = NewIndex() } } } - err = newIndex.AddToSupersedes(extraObsolete...) + err := newIndex.AddToSupersedes(extraObsolete...) if err != nil { return err } @@ -332,7 +330,7 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack select { case ch <- newIndex: - case <-ctx.Done(): + case <-wgCtx.Done(): } return nil }) @@ -341,7 +339,7 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack worker := func() error { for idx := range ch { idx.Finalize() - if _, err := SaveIndex(ctx, repo, idx); err != nil { + if _, err := SaveIndex(wgCtx, repo, idx); err != nil { return err } } @@ -354,9 +352,27 @@ func (mi *MasterIndex) Save(ctx context.Context, repo restic.SaverUnpacked, pack for i := 0; i < workerCount; i++ { wg.Go(worker) } - err = wg.Wait() + err := wg.Wait() + p.Done() + if err != nil { + return err + } - return obsolete, err + if opts.SkipDeletion { + return nil + } + + p = nil + if opts.DeleteProgress != nil { + p = opts.DeleteProgress() + } + defer p.Done() + return restic.ParallelRemove(ctx, repo, obsolete, restic.IndexFile, func(id restic.ID, err error) error { + if opts.DeleteReport != nil { + opts.DeleteReport(id, err) + } + return err + }, p) } // SaveIndex saves an index in the repository. diff --git a/internal/index/master_index_test.go b/internal/index/master_index_test.go index f76feb5fa..dcf6a94f6 100644 --- a/internal/index/master_index_test.go +++ b/internal/index/master_index_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/checker" "github.com/restic/restic/internal/crypto" "github.com/restic/restic/internal/index" @@ -363,20 +362,11 @@ func testIndexSave(t *testing.T, version uint) { t.Fatal(err) } - obsoletes, err := repo.Index().Save(context.TODO(), repo, nil, nil, nil) + err = repo.Index().Save(context.TODO(), repo, nil, nil, restic.MasterIndexSaveOpts{}) if err != nil { t.Fatalf("unable to save new index: %v", err) } - for id := range obsoletes { - t.Logf("remove index %v", id.Str()) - h := backend.Handle{Type: restic.IndexFile, Name: id.String()} - err = repo.Backend().Remove(context.TODO(), h) - if err != nil { - t.Errorf("error removing index %v: %v", id, err) - } - } - checker := checker.New(repo, false) err = checker.LoadSnapshots(context.TODO()) if err != nil { diff --git a/internal/repository/repack_test.go b/internal/repository/repack_test.go index 20f0f2685..bab04f6b7 100644 --- a/internal/repository/repack_test.go +++ b/internal/repository/repack_test.go @@ -173,39 +173,27 @@ func flush(t *testing.T, repo restic.Repository) { func rebuildIndex(t *testing.T, repo restic.Repository) { err := repo.SetIndex(index.NewMasterIndex()) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, err) packs := make(map[restic.ID]int64) err = repo.List(context.TODO(), restic.PackFile, func(id restic.ID, size int64) error { packs[id] = size return nil }) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, err) _, err = repo.(*repository.Repository).CreateIndexFromPacks(context.TODO(), packs, nil) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, err) + var obsoleteIndexes restic.IDs err = repo.List(context.TODO(), restic.IndexFile, func(id restic.ID, size int64) error { - h := backend.Handle{ - Type: restic.IndexFile, - Name: id.String(), - } - return repo.Backend().Remove(context.TODO(), h) + obsoleteIndexes = append(obsoleteIndexes, id) + return nil }) - if err != nil { - t.Fatal(err) - } + rtest.OK(t, err) - _, err = repo.Index().Save(context.TODO(), repo, restic.NewIDSet(), nil, nil) - if err != nil { - t.Fatal(err) - } + err = repo.Index().Save(context.TODO(), repo, restic.NewIDSet(), obsoleteIndexes, restic.MasterIndexSaveOpts{}) + rtest.OK(t, err) } func reloadIndex(t *testing.T, repo restic.Repository) { diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 6818847c0..1c6c8d39d 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -89,6 +89,13 @@ type PackBlobs struct { Blobs []Blob } +type MasterIndexSaveOpts struct { + SaveProgress *progress.Counter + DeleteProgress func() *progress.Counter + DeleteReport func(id ID, err error) + SkipDeletion bool +} + // MasterIndex keeps track of the blobs are stored within files. type MasterIndex interface { Has(BlobHandle) bool @@ -99,7 +106,7 @@ type MasterIndex interface { Each(ctx context.Context, fn func(PackedBlob)) ListPacks(ctx context.Context, packs IDSet) <-chan PackBlobs - Save(ctx context.Context, repo SaverUnpacked, packBlacklist IDSet, extraObsolete IDs, p *progress.Counter) (obsolete IDSet, err error) + Save(ctx context.Context, repo Repository, excludePacks IDSet, extraObsolete IDs, opts MasterIndexSaveOpts) error } // Lister allows listing files in a backend.