diff --git a/core/artwork/image_store.go b/core/artwork/image_store.go index 53662f686..77c69de7d 100644 --- a/core/artwork/image_store.go +++ b/core/artwork/image_store.go @@ -1,6 +1,7 @@ package artwork import ( + "context" "errors" "fmt" "io" @@ -137,12 +138,16 @@ func (s *ImageStore) Remove(hash, mimeType string, olderThan time.Time) error { // Sweep removes store files not accepted by keep. Files modified after cutoff // (including temp files) are always kept: their acquisition row may not be committed yet. -func (s *ImageStore) Sweep(cutoff time.Time, keep func(hash, ext string) bool) (int, error) { +// The walk is cancellable: it runs under the worker's prune lock, which shutdown waits on. +func (s *ImageStore) Sweep(ctx context.Context, cutoff time.Time, keep func(hash, ext string) bool) (int, error) { removed := 0 err := filepath.WalkDir(s.root, func(path string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { return err } + if err := ctx.Err(); err != nil { + return err + } info, err := d.Info() if err != nil { return err diff --git a/core/artwork/image_store_test.go b/core/artwork/image_store_test.go index 6537ca906..80c8a14f9 100644 --- a/core/artwork/image_store_test.go +++ b/core/artwork/image_store_test.go @@ -2,6 +2,7 @@ package artwork import ( "bytes" + "context" "io" "os" "path/filepath" @@ -14,10 +15,12 @@ import ( var _ = Describe("ImageStore", func() { var store *ImageStore var root string + var ctx context.Context BeforeEach(func() { root = GinkgoT().TempDir() store = NewImageStore(root) + ctx = context.Background() }) It("hashes deterministically", func() { @@ -141,7 +144,7 @@ var _ = Describe("ImageStore", func() { old := time.Now().Add(-2 * time.Hour) Expect(os.Chtimes(store.path(h2, "image/jpeg"), old, old)).To(Succeed()) - removed, err := store.Sweep(time.Now().Add(-time.Hour), func(h, _ string) bool { return h == h1 }) + removed, err := store.Sweep(ctx, time.Now().Add(-time.Hour), func(h, _ string) bool { return h == h1 }) Expect(err).ToNot(HaveOccurred()) Expect(removed).To(Equal(1)) _, err = store.Open(h2, "image/jpeg") @@ -161,7 +164,7 @@ var _ = Describe("ImageStore", func() { Expect(os.Chtimes(store.path(h, "image/jpeg"), old, old)).To(Succeed()) // The recorded mime is image/jpeg, so the .png variant is obsolete. - removed, err := store.Sweep(time.Now().Add(-time.Hour), func(hash, ext string) bool { + removed, err := store.Sweep(ctx, time.Now().Add(-time.Hour), func(hash, ext string) bool { return hash == h && ext == ".jpg" }) Expect(err).ToNot(HaveOccurred()) @@ -178,7 +181,7 @@ var _ = Describe("ImageStore", func() { h, _ := HashImage(bytes.NewReader(d)) Expect(store.Write(h, "image/jpeg", bytes.NewReader(d))).To(Succeed()) - removed, err := store.Sweep(time.Now().Add(-time.Hour), func(string, string) bool { return false }) + removed, err := store.Sweep(ctx, time.Now().Add(-time.Hour), func(string, string) bool { return false }) Expect(err).ToNot(HaveOccurred()) Expect(removed).To(Equal(0)) rc, err := store.Open(h, "image/jpeg") @@ -195,10 +198,28 @@ var _ = Describe("ImageStore", func() { freshTmp := filepath.Join(root, ".fresh.tmp") Expect(os.WriteFile(freshTmp, []byte("y"), 0600)).To(Succeed()) - removed, err := store.Sweep(time.Now().Add(-time.Hour), func(string, string) bool { return true }) + removed, err := store.Sweep(ctx, time.Now().Add(-time.Hour), func(string, string) bool { return true }) Expect(err).ToNot(HaveOccurred()) Expect(removed).To(Equal(1)) Expect(oldTmp).ToNot(BeAnExistingFile()) Expect(freshTmp).To(BeAnExistingFile()) }) + + // Prune holds the worker's write lock for the whole sweep, and shutdown waits on the + // worker, so an uncancellable walk over a large store stalls it until SIGKILL. + It("abandons the walk when the context is cancelled", func() { + old := time.Now().Add(-2 * time.Hour) + for _, name := range []string{"a", "b", "c", "d"} { + p := filepath.Join(root, name+".jpg") + Expect(os.WriteFile(p, []byte("x"), 0600)).To(Succeed()) + Expect(os.Chtimes(p, old, old)).To(Succeed()) + } + cancelCtx, cancel := context.WithCancel(ctx) + cancel() + + _, err := store.Sweep(cancelCtx, time.Now().Add(-time.Hour), func(string, string) bool { return false }) + Expect(err).To(MatchError(context.Canceled)) + matches, _ := filepath.Glob(filepath.Join(root, "*.jpg")) + Expect(matches).To(HaveLen(4), "a cancelled sweep must not keep deleting") + }) }) diff --git a/core/artwork/prune.go b/core/artwork/prune.go index 35eddb525..f6bc53186 100644 --- a/core/artwork/prune.go +++ b/core/artwork/prune.go @@ -71,7 +71,7 @@ func Prune(ctx context.Context, ds model.DataStore, store *ImageStore) error { if err != nil { return err } - removed, err := store.Sweep(cutoff, func(hash, ext string) bool { + removed, err := store.Sweep(ctx, cutoff, func(hash, ext string) bool { // A known hash under a stale extension is a superseded mime variant — reclaim it. m, ok := mimes[hash] return ok && ext == extForMime(m)