From 78a1757e5ad57b842ada4b7d154cb2eb0ff42b30 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 10 Feb 2023 22:39:40 +0100 Subject: [PATCH] Cancel current command if cache becomes unusable If the cache suddenly disappears, the current command will now fail. --- changelog/unreleased/pull-4166 | 7 +++++++ internal/cache/backend.go | 35 ++++++++++++++++++---------------- internal/cache/file_test.go | 18 +++++++++++++++++ 3 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 changelog/unreleased/pull-4166 diff --git a/changelog/unreleased/pull-4166 b/changelog/unreleased/pull-4166 new file mode 100644 index 000000000..6714fdf7f --- /dev/null +++ b/changelog/unreleased/pull-4166 @@ -0,0 +1,7 @@ +Enhancement: Cancel current command if cache becomes unusable + +If the cache directory was removed or ran out of space while restic was +running, this caused further caching attempts to fail and drastically slow down +the command execution. Now, the currently running command is canceled instead. + +https://github.com/restic/restic/pull/4166 diff --git a/internal/cache/backend.go b/internal/cache/backend.go index 08ec1facd..7c9df0f8e 100644 --- a/internal/cache/backend.go +++ b/internal/cache/backend.go @@ -83,7 +83,7 @@ func (b *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindRea if err != nil { debug.Log("unable to save %v to cache: %v", h, err) _ = b.Cache.remove(h) - return nil + return err } return nil @@ -106,11 +106,19 @@ func (b *Backend) cacheFile(ctx context.Context, h restic.Handle) error { return nil } + defer func() { + // signal other waiting goroutines that the file may now be cached + close(finish) + + // remove the finish channel from the map + b.inProgressMutex.Lock() + delete(b.inProgress, h) + b.inProgressMutex.Unlock() + }() + // test again, maybe the file was cached in the meantime if !b.Cache.Has(h) { - // nope, it's still not in the cache, pull it from the repo and save it - err := b.Backend.Load(ctx, h, 0, 0, func(rd io.Reader) error { return b.Cache.Save(h, rd) }) @@ -118,16 +126,9 @@ func (b *Backend) cacheFile(ctx context.Context, h restic.Handle) error { // try to remove from the cache, ignore errors _ = b.Cache.remove(h) } + return err } - // signal other waiting goroutines that the file may now be cached - close(finish) - - // remove the finish channel from the map - b.inProgressMutex.Lock() - delete(b.inProgress, h) - b.inProgressMutex.Unlock() - return nil } @@ -178,11 +179,13 @@ func (b *Backend) Load(ctx context.Context, h restic.Handle, length int, offset debug.Log("auto-store %v in the cache", h) err = b.cacheFile(ctx, h) - if err == nil { - inCache, err = b.loadFromCache(ctx, h, length, offset, consumer) - if inCache { - return err - } + if err != nil { + return err + } + + inCache, err = b.loadFromCache(ctx, h, length, offset, consumer) + if inCache { + return err } debug.Log("error caching %v: %v, falling back to backend", h, err) diff --git a/internal/cache/file_test.go b/internal/cache/file_test.go index 111a2430f..335e78aba 100644 --- a/internal/cache/file_test.go +++ b/internal/cache/file_test.go @@ -11,8 +11,10 @@ import ( "time" "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/test" + rtest "github.com/restic/restic/internal/test" "golang.org/x/sync/errgroup" ) @@ -266,3 +268,19 @@ func TestFileSaveConcurrent(t *testing.T) { saved := load(t, c, h) test.Equals(t, data, saved) } + +func TestFileSaveAfterDamage(t *testing.T) { + c := TestNewCache(t) + rtest.OK(t, fs.RemoveAll(c.path)) + + // save a few bytes of data in the cache + data := test.Random(123456789, 42) + id := restic.Hash(data) + h := restic.Handle{ + Type: restic.PackFile, + Name: id.String(), + } + if err := c.Save(h, bytes.NewReader(data)); err == nil { + t.Fatal("Missing error when saving to deleted cache directory") + } +}