fix(cache): re-fetch when a cache entry outlives its data file

fscache's Remove drops the in-memory entry, releases the lock, and only then
unlinks - blocking until every outstanding reader closes. A Get landing in that
window re-creates the file at the same path under a fresh entry, and the deferred
unlink deletes those new bytes. The entry survives pointing at nothing, and since
a present entry is treated as a hit, every later Get for that key returns ENOENT
for the rest of the process's life. Only a restart, which rebuilds the map from
disk, cleared it.

Get now drops such an entry and retries once, so a vanished data file costs one
re-fetch instead of poisoning the key permanently. This also covers a file
disappearing for reasons unrelated to that race, such as external deletion or a
restored backup.

Specs cover an in-process entry, one adopted at startup, and the deferred-removal
race itself.
This commit is contained in:
Deluan 2026-07-26 01:07:30 -04:00
parent 0e05e70823
commit cabbbc0ced
2 changed files with 97 additions and 0 deletions

View File

@ -2,8 +2,10 @@ package cache
import (
"context"
"errors"
"fmt"
"io"
"io/fs"
"path/filepath"
"sync"
"sync/atomic"
@ -158,6 +160,13 @@ func (fc *fileCache) Get(ctx context.Context, arg Item) (*CachedStream, error) {
key := arg.Key()
r, w, err := fc.cache.Get(key)
if errors.Is(err, fs.ErrNotExist) {
// The entry outlived its data file. Drop it and retry, or every future Get
// for this key fails for the rest of the process's life.
log.Debug(ctx, "Cache entry lost its data file. Re-fetching", "cache", fc.name, "key", key)
_ = fc.invalidate(ctx, key)
r, w, err = fc.cache.Get(key)
}
if err != nil {
return nil, err
}

View File

@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"strings"
"sync/atomic"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -278,6 +279,93 @@ var _ = Describe("File Caches", func() {
}).Should(BeTrue())
})
})
Context("entry outliving its data file", func() {
It("re-fetches when the data file vanished behind the cache's back", func() {
var calls atomic.Int32
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
calls.Add(1)
return strings.NewReader("payload"), nil
})
s, err := fc.Get(context.Background(), &testArg{"vanish"})
Expect(err).To(BeNil())
Expect(io.ReadAll(s)).To(Equal([]byte("payload")))
Expect(s.Close()).To(Succeed())
dataPath := fcSpreadFS(fc).KeyMapper((&testArg{"vanish"}).Key())
Eventually(func() error { _, e := os.Stat(dataPath); return e }).Should(Succeed())
Expect(os.Remove(dataPath)).To(Succeed())
s2, err := fc.Get(context.Background(), &testArg{"vanish"})
Expect(err).ToNot(HaveOccurred())
Expect(io.ReadAll(s2)).To(Equal([]byte("payload")))
_ = s2.Close()
Expect(calls.Load()).To(BeNumerically("==", 2))
})
It("survives an invalidated entry's deferred file removal", func() {
// invalidate() drops the map entry but defers the unlink until readers close;
// a Get in that window re-creates the file, which the deferred unlink then eats.
var n atomic.Int32
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
if n.Add(1) == 1 {
return &partialThenErrReader{data: []byte("PARTIAL"), err: errors.New("died")}, nil
}
return strings.NewReader("GOOD"), nil
})
key := (&testArg{"deferred"}).Key()
s1, err := fc.Get(context.Background(), &testArg{"deferred"})
Expect(err).To(BeNil())
// The failed write invalidates the entry; the removal now waits on s1.
Eventually(func() bool { return fc.cache.Exists(key) }).Should(BeFalse())
s2, err := fc.Get(context.Background(), &testArg{"deferred"})
Expect(err).To(BeNil())
Expect(io.ReadAll(s2)).To(Equal([]byte("GOOD")))
Expect(s2.Close()).To(Succeed())
Expect(s1.Close()).To(Succeed())
dataPath := fcSpreadFS(fc).KeyMapper(key)
Eventually(func() bool {
_, e := os.Stat(dataPath)
return os.IsNotExist(e)
}).Should(BeTrue(), "expected the deferred removal to take the re-created file")
s3, err := fc.Get(context.Background(), &testArg{"deferred"})
Expect(err).ToNot(HaveOccurred())
Expect(io.ReadAll(s3)).To(Equal([]byte("GOOD")))
_ = s3.Close()
})
It("re-fetches when an adopted entry's data file vanished", func() {
// Entries adopted on startup take a different code path than ones
// created in-process, so cover both.
var calls atomic.Int32
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
calls.Add(1)
return strings.NewReader("payload"), nil
})
dataPath := fcSpreadFS(fc).KeyMapper((&testArg{"adopted"}).Key())
Expect(os.MkdirAll(filepath.Dir(dataPath), 0755)).To(Succeed())
Expect(os.WriteFile(dataPath, []byte("payload"), 0600)).To(Succeed())
Expect(fcSpreadFS(fc).MarkComplete(dataPath)).To(Succeed())
adopted := callNewFileCache("test2", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
calls.Add(1)
return strings.NewReader("payload"), nil
})
Expect(os.Remove(dataPath)).To(Succeed())
s, err := adopted.Get(context.Background(), &testArg{"adopted"})
Expect(err).ToNot(HaveOccurred())
Expect(io.ReadAll(s)).To(Equal([]byte("payload")))
_ = s.Close()
})
})
})
})