From f66624f5bf6a7c45b05c5b08b765ddd0500c767f Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 11 Aug 2024 01:42:13 +0530 Subject: [PATCH 1/8] cache: backend add List method and a cache clear functionality * removes files which are no longer in the repository, including index files, snapshot files and pack files from the cache. cache: fix ids set initialisation with NewIDSet() --- internal/backend/cache/backend.go | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/internal/backend/cache/backend.go b/internal/backend/cache/backend.go index 94f648cf4..92cca4d0e 100644 --- a/internal/backend/cache/backend.go +++ b/internal/backend/cache/backend.go @@ -2,11 +2,14 @@ package cache import ( "context" + "fmt" "io" + "os" "sync" "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/restic" ) // Backend wraps a restic.Backend and adds a cache. @@ -215,3 +218,40 @@ func (b *Backend) IsNotExist(err error) bool { func (b *Backend) Unwrap() backend.Backend { return b.Backend } + +func (b *Backend) List(ctx context.Context, t backend.FileType, fn func(f backend.FileInfo) error) error { + if !b.Cache.canBeCached(t) { + return b.Backend.List(ctx, t, fn) + } + + // will contain the IDs of the files that are in the repository + ids := restic.NewIDSet() + + // wrap the original function to also add the file to the ids set + wrapFn := func(f backend.FileInfo) error { + id, err := restic.ParseID(f.Name) + if err != nil { + // returning error here since, if we cannot parse the ID, the file + // is invalid and the list must exit. + return err + } + + ids.Insert(id) + + // execute the original function + return fn(f) + } + + err := b.Backend.List(ctx, t, wrapFn) + if err != nil { + return err + } + + // clear the cache for files that are not in the repo anymore, ignore errors + err = b.Cache.Clear(t, ids) + if err != nil { + fmt.Fprintf(os.Stderr, "error clearing %s files in cache: %v\n", t.String(), err) + } + + return nil +} From a23e7bfb82984fd88c42aa09365b25333089d071 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 11 Aug 2024 15:43:03 +0530 Subject: [PATCH 2/8] cache: check for context cancellation before clearing cache --- internal/backend/cache/backend.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/backend/cache/backend.go b/internal/backend/cache/backend.go index 92cca4d0e..58b03dd38 100644 --- a/internal/backend/cache/backend.go +++ b/internal/backend/cache/backend.go @@ -247,6 +247,10 @@ func (b *Backend) List(ctx context.Context, t backend.FileType, fn func(f backen return err } + if ctx.Err() != nil { + return ctx.Err() + } + // clear the cache for files that are not in the repo anymore, ignore errors err = b.Cache.Clear(t, ids) if err != nil { From 720609f8ba6dcf44b7fe51cd9b543ee44bbbaf38 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 11 Aug 2024 15:58:27 +0530 Subject: [PATCH 3/8] repository: removed redundant prepareCache method from Repository * remove the prepareCache method from the Repository * changed the signature of the SetIndex function to no longer return an error --- internal/checker/checker.go | 6 +---- internal/repository/repair_index.go | 6 ++--- internal/repository/repository.go | 37 ++--------------------------- internal/restic/repository.go | 2 +- 4 files changed, 6 insertions(+), 45 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 031e13807..d5e7fd1f8 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -146,11 +146,7 @@ func (c *Checker) LoadIndex(ctx context.Context, p *progress.Counter) (hints []e return hints, append(errs, err) } - err = c.repo.SetIndex(c.masterIndex) - if err != nil { - debug.Log("SetIndex returned error: %v", err) - errs = append(errs, err) - } + c.repo.SetIndex(c.masterIndex) // compute pack size using index entries c.packs, err = pack.Size(ctx, c.repo, false) diff --git a/internal/repository/repair_index.go b/internal/repository/repair_index.go index 770809254..c72dcfd00 100644 --- a/internal/repository/repair_index.go +++ b/internal/repository/repair_index.go @@ -52,10 +52,8 @@ func RepairIndex(ctx context.Context, repo *Repository, opts RepairIndexOptions, return err } - err = repo.SetIndex(mi) - if err != nil { - return err - } + repo.SetIndex(mi) + packSizeFromIndex, err = pack.Size(ctx, repo, false) if err != nil { return err diff --git a/internal/repository/repository.go b/internal/repository/repository.go index f7fd65c71..3dc248c5e 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "math" - "os" "runtime" "sort" "sync" @@ -586,9 +585,8 @@ func (r *Repository) ListPacksFromIndex(ctx context.Context, packs restic.IDSet) } // SetIndex instructs the repository to use the given index. -func (r *Repository) SetIndex(i restic.MasterIndex) error { +func (r *Repository) SetIndex(i restic.MasterIndex) { r.idx = i.(*index.MasterIndex) - return r.prepareCache() } func (r *Repository) clearIndex() { @@ -628,12 +626,8 @@ func (r *Repository) LoadIndex(ctx context.Context, p *progress.Counter) error { return errors.New("index uses feature not supported by repository version 1") } } - if ctx.Err() != nil { - return ctx.Err() - } - // remove index files from the cache which have been removed in the repo - return r.prepareCache() + return ctx.Err() } // createIndexFromPacks creates a new index by reading all given pack files (with sizes). @@ -699,33 +693,6 @@ func (r *Repository) createIndexFromPacks(ctx context.Context, packsize map[rest return invalid, nil } -// prepareCache initializes the local cache. indexIDs is the list of IDs of -// index files still present in the repo. -func (r *Repository) prepareCache() error { - if r.Cache == nil { - return nil - } - - indexIDs := r.idx.IDs() - debug.Log("prepare cache with %d index files", len(indexIDs)) - - // clear old index files - err := r.Cache.Clear(restic.IndexFile, indexIDs) - if err != nil { - fmt.Fprintf(os.Stderr, "error clearing index files in cache: %v\n", err) - } - - packs := r.idx.Packs(restic.NewIDSet()) - - // clear old packs - err = r.Cache.Clear(restic.PackFile, packs) - if err != nil { - fmt.Fprintf(os.Stderr, "error clearing pack files in cache: %v\n", err) - } - - return nil -} - // SearchKey finds a key with the supplied password, afterwards the config is // read and parsed. It tries at most maxKeys key files in the repo. func (r *Repository) SearchKey(ctx context.Context, password string, maxKeys int, keyHint string) error { diff --git a/internal/restic/repository.go b/internal/restic/repository.go index b18b036a7..ce8401b37 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -22,7 +22,7 @@ type Repository interface { Key() *crypto.Key LoadIndex(ctx context.Context, p *progress.Counter) error - SetIndex(mi MasterIndex) error + SetIndex(mi MasterIndex) LookupBlob(t BlobType, id ID) []PackedBlob LookupBlobSize(t BlobType, id ID) (size uint, exists bool) From 506e07127f1430b3aa1c7a1d98e2efdf58c57c65 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 11 Aug 2024 16:07:38 +0530 Subject: [PATCH 4/8] changelog: add unrelease changelog --- changelog/unreleased/issue-4934 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/issue-4934 diff --git a/changelog/unreleased/issue-4934 b/changelog/unreleased/issue-4934 new file mode 100644 index 000000000..03194168e --- /dev/null +++ b/changelog/unreleased/issue-4934 @@ -0,0 +1,9 @@ +Enhancement: Clear removed snapshots, index and pack files from the local cache + +Restic did not clear removed snapshots from the cache after the `forget` +operation; only indexes and pack files were removed automatically. +Restic now automatically clears removed indexes, packs and snapshots from the +local cache. + +https://github.com/restic/restic/issues/4934 +https://github.com/restic/restic/pull/4981 \ No newline at end of file From 5fd984ba6f2bb0f5a92ec14eef36eb28da6e99a2 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 11 Aug 2024 16:44:43 +0530 Subject: [PATCH 5/8] cache: add test for the automated cache clear to cache backend --- internal/backend/cache/backend_test.go | 58 ++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/internal/backend/cache/backend_test.go b/internal/backend/cache/backend_test.go index 7addc275d..dca51c2bf 100644 --- a/internal/backend/cache/backend_test.go +++ b/internal/backend/cache/backend_test.go @@ -57,6 +57,13 @@ func randomData(n int) (backend.Handle, []byte) { return h, data } +func list(t testing.TB, be backend.Backend, fn func(backend.FileInfo) error) { + err := be.List(context.TODO(), backend.IndexFile, fn) + if err != nil { + t.Fatal(err) + } +} + func TestBackend(t *testing.T) { be := mem.New() c := TestNewCache(t) @@ -238,3 +245,54 @@ func TestErrorBackend(t *testing.T) { wg.Wait() } + +func TestAutomaticCacheClear(t *testing.T) { + be := mem.New() + c := TestNewCache(t) + wbe := c.Wrap(be) + + // add two handles h1 and h2 + h1, data := randomData(2000) + // save h1 directly to the backend + save(t, be, h1, data) + if c.Has(h1) { + t.Errorf("cache has file1 too early") + } + + h2, data2 := randomData(3000) + + // save h2 directly to the backend + save(t, be, h2, data2) + if c.Has(h2) { + t.Errorf("cache has file2 too early") + } + + loadAndCompare(t, wbe, h1, data) + if !c.Has(h1) { + t.Errorf("cache doesn't have file1 after load") + } + + loadAndCompare(t, wbe, h2, data2) + if !c.Has(h2) { + t.Errorf("cache doesn't have file2 after load") + } + + // remove h1 directly from the backend + remove(t, be, h1) + if !c.Has(h1) { + t.Errorf("file1 not in cache any more, should be removed from cache only after list") + } + + // list all files in the backend + list(t, wbe, func(_ backend.FileInfo) error { return nil }) + + // h1 should be removed from the cache + if c.Has(h1) { + t.Errorf("cache has file1 after remove") + } + + // h2 should still be in the cache + if !c.Has(h2) { + t.Errorf("cache doesn't have file2 after list") + } +} From b7d014b68528e84bc817c0299c5dd04731b1e84b Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 17 Aug 2024 00:18:13 +0530 Subject: [PATCH 6/8] Revert "repository: removed redundant prepareCache method from Repository" This reverts commit 720609f8ba6dcf44b7fe51cd9b543ee44bbbaf38. --- internal/checker/checker.go | 6 ++++- internal/repository/repair_index.go | 6 +++-- internal/repository/repository.go | 37 +++++++++++++++++++++++++++-- internal/restic/repository.go | 2 +- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/internal/checker/checker.go b/internal/checker/checker.go index d5e7fd1f8..031e13807 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -146,7 +146,11 @@ func (c *Checker) LoadIndex(ctx context.Context, p *progress.Counter) (hints []e return hints, append(errs, err) } - c.repo.SetIndex(c.masterIndex) + err = c.repo.SetIndex(c.masterIndex) + if err != nil { + debug.Log("SetIndex returned error: %v", err) + errs = append(errs, err) + } // compute pack size using index entries c.packs, err = pack.Size(ctx, c.repo, false) diff --git a/internal/repository/repair_index.go b/internal/repository/repair_index.go index c72dcfd00..770809254 100644 --- a/internal/repository/repair_index.go +++ b/internal/repository/repair_index.go @@ -52,8 +52,10 @@ func RepairIndex(ctx context.Context, repo *Repository, opts RepairIndexOptions, return err } - repo.SetIndex(mi) - + err = repo.SetIndex(mi) + if err != nil { + return err + } packSizeFromIndex, err = pack.Size(ctx, repo, false) if err != nil { return err diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 3dc248c5e..f7fd65c71 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "math" + "os" "runtime" "sort" "sync" @@ -585,8 +586,9 @@ func (r *Repository) ListPacksFromIndex(ctx context.Context, packs restic.IDSet) } // SetIndex instructs the repository to use the given index. -func (r *Repository) SetIndex(i restic.MasterIndex) { +func (r *Repository) SetIndex(i restic.MasterIndex) error { r.idx = i.(*index.MasterIndex) + return r.prepareCache() } func (r *Repository) clearIndex() { @@ -626,8 +628,12 @@ func (r *Repository) LoadIndex(ctx context.Context, p *progress.Counter) error { return errors.New("index uses feature not supported by repository version 1") } } + if ctx.Err() != nil { + return ctx.Err() + } - return ctx.Err() + // remove index files from the cache which have been removed in the repo + return r.prepareCache() } // createIndexFromPacks creates a new index by reading all given pack files (with sizes). @@ -693,6 +699,33 @@ func (r *Repository) createIndexFromPacks(ctx context.Context, packsize map[rest return invalid, nil } +// prepareCache initializes the local cache. indexIDs is the list of IDs of +// index files still present in the repo. +func (r *Repository) prepareCache() error { + if r.Cache == nil { + return nil + } + + indexIDs := r.idx.IDs() + debug.Log("prepare cache with %d index files", len(indexIDs)) + + // clear old index files + err := r.Cache.Clear(restic.IndexFile, indexIDs) + if err != nil { + fmt.Fprintf(os.Stderr, "error clearing index files in cache: %v\n", err) + } + + packs := r.idx.Packs(restic.NewIDSet()) + + // clear old packs + err = r.Cache.Clear(restic.PackFile, packs) + if err != nil { + fmt.Fprintf(os.Stderr, "error clearing pack files in cache: %v\n", err) + } + + return nil +} + // SearchKey finds a key with the supplied password, afterwards the config is // read and parsed. It tries at most maxKeys key files in the repo. func (r *Repository) SearchKey(ctx context.Context, password string, maxKeys int, keyHint string) error { diff --git a/internal/restic/repository.go b/internal/restic/repository.go index ce8401b37..b18b036a7 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -22,7 +22,7 @@ type Repository interface { Key() *crypto.Key LoadIndex(ctx context.Context, p *progress.Counter) error - SetIndex(mi MasterIndex) + SetIndex(mi MasterIndex) error LookupBlob(t BlobType, id ID) []PackedBlob LookupBlobSize(t BlobType, id ID) (size uint, exists bool) From 88174cd0a480c6cd0be97771c8c82c7678444790 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 17 Aug 2024 00:21:49 +0530 Subject: [PATCH 7/8] cache: remove redundant index file cleanup addressing code review comments --- internal/repository/repository.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index f7fd65c71..d408e3105 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -706,19 +706,10 @@ func (r *Repository) prepareCache() error { return nil } - indexIDs := r.idx.IDs() - debug.Log("prepare cache with %d index files", len(indexIDs)) - - // clear old index files - err := r.Cache.Clear(restic.IndexFile, indexIDs) - if err != nil { - fmt.Fprintf(os.Stderr, "error clearing index files in cache: %v\n", err) - } - packs := r.idx.Packs(restic.NewIDSet()) // clear old packs - err = r.Cache.Clear(restic.PackFile, packs) + err := r.Cache.Clear(restic.PackFile, packs) if err != nil { fmt.Fprintf(os.Stderr, "error clearing pack files in cache: %v\n", err) } From c9097994b97991cf2070b482cce1f92b5321c89c Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 17 Aug 2024 00:24:19 +0530 Subject: [PATCH 8/8] changelog: update changelog --- changelog/unreleased/issue-4934 | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/changelog/unreleased/issue-4934 b/changelog/unreleased/issue-4934 index 03194168e..6891ca204 100644 --- a/changelog/unreleased/issue-4934 +++ b/changelog/unreleased/issue-4934 @@ -1,9 +1,8 @@ -Enhancement: Clear removed snapshots, index and pack files from the local cache +Enhancement: Clear removed snapshots from local cache of the current host -Restic did not clear removed snapshots from the cache after the `forget` -operation; only indexes and pack files were removed automatically. -Restic now automatically clears removed indexes, packs and snapshots from the -local cache. +Restic only removed snapshots from the cache on the host that runs the `forget` command. +On other hosts that use the same repository, the old snapshots remained in the cache. +Restic now, automatically clears old snapshots from the local cache of the current host. https://github.com/restic/restic/issues/4934 https://github.com/restic/restic/pull/4981 \ No newline at end of file