diff --git a/changelog/unreleased/issue-5722 b/changelog/unreleased/issue-5722 new file mode 100644 index 000000000..ffe9338f2 --- /dev/null +++ b/changelog/unreleased/issue-5722 @@ -0,0 +1,10 @@ +Bugfix: Update `mount` latest symlink after snapshot reload + +When `restic mount` was kept running while new snapshots were +created, the new snapshots appeared in the mountpoint, but the `latest` +symlink could still point to the previously latest snapshot. Restic now +invalidates the cached snapshot directory entries after a snapshot reload so +that `latest` points to the newest snapshot. + +https://github.com/restic/restic/issues/5722 +https://github.com/restic/restic/pull/21873 diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index df558ac1f..94b18e2aa 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -225,7 +225,7 @@ func (d *dir) Lookup(ctx context.Context, name string) (fs.Node, error) { return nil, err } - return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { + return d.cache.lookupOrCreate(name, -1, func(forget forgetFn) (fs.Node, error) { node, ok := d.items[name] if !ok { debug.Log(" Lookup(%v) -> not found", name) diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index e1e8caf9d..719d6c2eb 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -247,6 +247,39 @@ func TestStableNodeObjects(t *testing.T) { testStableLookup(t, dir, "file-2") } +func TestSnapshotsDirLatestSymlinkUpdatesAfterReload(t *testing.T) { + repo := repository.TestRepository(t) + timeTemplate := "2006-01-02T15:04:05" + + firstTime, err := time.Parse(time.RFC3339, "2017-01-24T10:42:56Z") + rtest.OK(t, err) + secondTime := firstTime.Add(time.Hour) + + data.TestCreateSnapshot(t, repo, firstTime, 0) + root := NewRoot(repo, Config{ + TimeTemplate: timeTemplate, + PathTemplates: []string{"snapshots/%T"}, + }) + + snapshotsDir := testStableLookup(t, root, "snapshots") + rtest.Equals(t, firstTime.Format(timeTemplate), readLatestTarget(t, snapshotsDir)) + + data.TestCreateSnapshot(t, repo, secondTime, 0) + root.SnapshotsDir.dirStruct.lastCheck = time.Now().Add(-2 * minSnapshotsReloadTime) + + rtest.Equals(t, secondTime.Format(timeTemplate), readLatestTarget(t, snapshotsDir)) +} + +func readLatestTarget(t testing.TB, node fs.Node) string { + t.Helper() + + latest, err := node.(fs.NodeStringLookuper).Lookup(context.TODO(), "latest") + rtest.OK(t, err) + target, err := latest.(fs.NodeReadlinker).Readlink(context.TODO(), nil) + rtest.OK(t, err) + return target +} + // Test reporting of fuse.Attr.Blocks in multiples of 512. func TestBlocks(t *testing.T) { root := &Root{} diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index 645249417..e9733c2cb 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -61,7 +61,7 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { debug.Log("ReadDirAll()") // update snapshots - meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) + meta, _, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { return nil, unwrapCtxCanceled(err) } else if meta == nil { @@ -104,14 +104,14 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) { debug.Log("Lookup(%s)", name) - meta, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) + meta, gen, err := d.dirStruct.UpdatePrefix(ctx, d.prefix) if err != nil { return nil, unwrapCtxCanceled(err) } else if meta == nil { return nil, syscall.ENOENT } - return d.cache.lookupOrCreate(name, func(forget forgetFn) (fs.Node, error) { + return d.cache.lookupOrCreate(name, gen, func(forget forgetFn) (fs.Node, error) { entry := meta.names[name] if entry == nil { return nil, syscall.ENOENT diff --git a/internal/fuse/snapshots_dirstruct.go b/internal/fuse/snapshots_dirstruct.go index 5059f96fe..76ba75662 100644 --- a/internal/fuse/snapshots_dirstruct.go +++ b/internal/fuse/snapshots_dirstruct.go @@ -43,6 +43,10 @@ type SnapshotsDirStructure struct { hash [sha256.Size]byte // Hash at last check. lastCheck time.Time + + // generation is incremented whenever the directory structure is rebuilt. + // It allows treeCache instances to detect stale entries and reset themselves. + generation int64 } // NewSnapshotsDirStructure returns a new directory structure for snapshots. @@ -281,6 +285,7 @@ func (d *SnapshotsDirStructure) makeDirs(snapshots data.Snapshots) { } d.entries = entries + d.generation++ } const minSnapshotsReloadTime = 60 * time.Second @@ -338,13 +343,13 @@ func (d *SnapshotsDirStructure) updateSnapshots(ctx context.Context) error { return nil } -func (d *SnapshotsDirStructure) UpdatePrefix(ctx context.Context, prefix string) (*MetaDirData, error) { +func (d *SnapshotsDirStructure) UpdatePrefix(ctx context.Context, prefix string) (*MetaDirData, int64, error) { err := d.updateSnapshots(ctx) if err != nil { - return nil, err + return nil, 0, err } d.mutex.Lock() defer d.mutex.Unlock() - return d.entries[prefix], nil + return d.entries[prefix], d.generation, nil } diff --git a/internal/fuse/tree_cache.go b/internal/fuse/tree_cache.go index 4b60aeb11..e4fb2fe23 100644 --- a/internal/fuse/tree_cache.go +++ b/internal/fuse/tree_cache.go @@ -5,12 +5,15 @@ package fuse import ( "sync" + "github.com/restic/restic/internal/debug" + "github.com/anacrolix/fuse/fs" ) type treeCache struct { - nodes map[string]fs.Node - m sync.Mutex + nodes map[string]fs.Node + m sync.Mutex + generation int64 } type forgetFn func() @@ -21,19 +24,28 @@ func newTreeCache() *treeCache { } } -func (t *treeCache) lookupOrCreate(name string, create func(forget forgetFn) (fs.Node, error)) (fs.Node, error) { +func (t *treeCache) lookupOrCreate(name string, generation int64, create func(forget forgetFn) (fs.Node, error)) (fs.Node, error) { t.m.Lock() defer t.m.Unlock() + if generation >= 0 && generation != t.generation { + debug.Log("treeCache generation changed %d -> %d, resetting cache", t.generation, generation) + t.nodes = make(map[string]fs.Node) + t.generation = generation + } + if node, ok := t.nodes[name]; ok { return node, nil } + cacheGeneration := t.generation node, err := create(func() { t.m.Lock() defer t.m.Unlock() - delete(t.nodes, name) + if t.generation == cacheGeneration { + delete(t.nodes, name) + } }) if err != nil { return nil, err diff --git a/internal/fuse/tree_cache_test.go b/internal/fuse/tree_cache_test.go new file mode 100644 index 000000000..594b47336 --- /dev/null +++ b/internal/fuse/tree_cache_test.go @@ -0,0 +1,82 @@ +//go:build darwin || freebsd || linux + +package fuse + +import ( + "context" + "testing" + + "github.com/anacrolix/fuse" + "github.com/anacrolix/fuse/fs" + + "github.com/restic/restic/internal/test" +) + +type cacheTestNode struct { + id int +} + +func (n cacheTestNode) Attr(context.Context, *fuse.Attr) error { + return nil +} + +func TestTreeCacheGeneration(t *testing.T) { + cache := newTreeCache() + created := 0 + create := func(forgetFn) (fs.Node, error) { + created++ + return cacheTestNode{id: created}, nil + } + + node1, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + node2, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + test.Assert(t, node1 == node2, "lookup should reuse cached node") + test.Equals(t, 1, created) + + node3, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node1 != node3, "lookup should recreate node after generation change") + test.Equals(t, 2, created) + + node4, err := cache.lookupOrCreate("node", -1, create) + test.OK(t, err) + test.Assert(t, node3 == node4, "negative generation should not reset cache") + test.Equals(t, 2, created) + + node5, err := cache.lookupOrCreate("node", 3, create) + test.OK(t, err) + test.Assert(t, node4 != node5, "lookup should still track later generation changes") + test.Equals(t, 3, created) +} + +func TestTreeCacheForgetOnlyRemovesSameGeneration(t *testing.T) { + cache := newTreeCache() + created := 0 + var forgets []forgetFn + create := func(forget forgetFn) (fs.Node, error) { + created++ + forgets = append(forgets, forget) + return cacheTestNode{id: created}, nil + } + + node1, err := cache.lookupOrCreate("node", 1, create) + test.OK(t, err) + node2, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node1 != node2, "lookup should recreate node after generation change") + test.Equals(t, 2, created) + + forgets[0]() + node3, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node2 == node3, "forget from an old generation must not remove the current node") + test.Equals(t, 2, created) + + forgets[1]() + node4, err := cache.lookupOrCreate("node", 2, create) + test.OK(t, err) + test.Assert(t, node3 != node4, "forget from the current generation should remove the cached node") + test.Equals(t, 3, created) +}