From e734746f752c89e27cad722640deb342695ac7b3 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Thu, 9 May 2024 18:24:28 +0200
Subject: [PATCH] cache: forget cached file at most once

This is inspired by the circuit breaker pattern used for distributed
systems. If too many requests fails, then it is better to immediately
fail new requests for a limited time to give the backend time to
recover.

By only forgetting a file in the cache at most once, we can ensure that
a broken file is only retrieved once again from the backend. If the file
stored there is broken, previously it would be cached and deleted
continuously. Now, it is retrieved only once again, all later requests
just use the cached copy and either succeed or fail immediately.
---
 internal/cache/backend.go |  6 +++---
 internal/cache/cache.go   |  3 +++
 internal/cache/file.go    | 22 ++++++++++++++++++----
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/internal/cache/backend.go b/internal/cache/backend.go
index b0edfcbe5..24a44d750 100644
--- a/internal/cache/backend.go
+++ b/internal/cache/backend.go
@@ -40,7 +40,7 @@ func (b *Backend) Remove(ctx context.Context, h backend.Handle) error {
 		return err
 	}
 
-	err = b.Cache.remove(h)
+	_, err = b.Cache.remove(h)
 	return err
 }
 
@@ -124,7 +124,7 @@ func (b *Backend) cacheFile(ctx context.Context, h backend.Handle) error {
 		})
 		if err != nil {
 			// try to remove from the cache, ignore errors
-			_ = b.Cache.remove(h)
+			_, _ = b.Cache.remove(h)
 		}
 		return err
 	}
@@ -197,7 +197,7 @@ func (b *Backend) Stat(ctx context.Context, h backend.Handle) (backend.FileInfo,
 	fi, err := b.Backend.Stat(ctx, h)
 	if err != nil && b.Backend.IsNotExist(err) {
 		// try to remove from the cache, ignore errors
-		_ = b.Cache.remove(h)
+		_, _ = b.Cache.remove(h)
 	}
 
 	return fi, err
diff --git a/internal/cache/cache.go b/internal/cache/cache.go
index 19b3182df..a55b51c70 100644
--- a/internal/cache/cache.go
+++ b/internal/cache/cache.go
@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strconv"
+	"sync"
 	"time"
 
 	"github.com/pkg/errors"
@@ -20,6 +21,8 @@ type Cache struct {
 	path    string
 	Base    string
 	Created bool
+
+	forgotten sync.Map
 }
 
 const dirMode = 0700
diff --git a/internal/cache/file.go b/internal/cache/file.go
index b54bd806d..12f5f23c5 100644
--- a/internal/cache/file.go
+++ b/internal/cache/file.go
@@ -1,6 +1,7 @@
 package cache
 
 import (
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -140,20 +141,33 @@ func (c *Cache) save(h backend.Handle, rd io.Reader) error {
 }
 
 func (c *Cache) Forget(h backend.Handle) error {
-	return c.remove(h)
+	h.IsMetadata = false
+
+	if _, ok := c.forgotten.Load(h); ok {
+		// Delete a file at most once while restic runs.
+		// This prevents repeatedly caching and forgetting broken files
+		return fmt.Errorf("circuit breaker prevents repeated deletion of cached file %v", h)
+	}
+
+	removed, err := c.remove(h)
+	if removed {
+		c.forgotten.Store(h, struct{}{})
+	}
+	return err
 }
 
 // remove deletes a file. When the file is not cached, no error is returned.
-func (c *Cache) remove(h backend.Handle) error {
+func (c *Cache) remove(h backend.Handle) (bool, error) {
 	if !c.canBeCached(h.Type) {
-		return nil
+		return false, nil
 	}
 
 	err := fs.Remove(c.filename(h))
+	removed := err == nil
 	if errors.Is(err, os.ErrNotExist) {
 		err = nil
 	}
-	return err
+	return removed, err
 }
 
 // Clear removes all files of type t from the cache that are not contained in