diff --git a/changelog/unreleased/issue-3428 b/changelog/unreleased/issue-3428 new file mode 100644 index 000000000..6d2b055c8 --- /dev/null +++ b/changelog/unreleased/issue-3428 @@ -0,0 +1,14 @@ +Bugfix: List snapshots in backend at most once to resolve snapshot ids + +Many commands support specifying a list of snapshot ids which are then used to +determine the snapshot accessed by the command. To resolve snapshot ids or +"latest" and check that these exist, restic listed all snapshots stored in the +repository. Depending on the backend this can be a slow and/or expensive +operation. + +Restic now lists the snapshots only once and remembers the result to resolve +all further snapshot ids. + +https://github.com/restic/restic/issues/3428 +https://github.com/restic/restic/pull/3570 +https://github.com/restic/restic/pull/3395 diff --git a/changelog/unreleased/issue-3432 b/changelog/unreleased/issue-3432 new file mode 100644 index 000000000..b874e7da6 --- /dev/null +++ b/changelog/unreleased/issue-3432 @@ -0,0 +1,14 @@ +Bugfix: Fix rare 'not found in repository' error for copy command + +In rare cases copy (and other commands) could report that LoadTree(...) +returned a `id [...] not found in repository` error. This could be caused by a +backup or copy command running concurrently. The error is only temporary, +running the failed restic command a second time as a workaround solves the +error. + +This issue has been fixed by correcting the order in which restic reads data +from the repository. It is now guaranteed that restic only loads snapshots for +which all necessary data is already available. + +https://github.com/restic/restic/issues/3432 +https://github.com/restic/restic/pull/3570 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index 873fce163..7ce4f52de 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -475,7 +475,7 @@ func collectTargets(opts BackupOptions, args []string) (targets []string, err er func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string, timeStampLimit time.Time) (parentID *restic.ID, err error) { // Force using a parent if !opts.Force && opts.Parent != "" { - id, err := restic.FindSnapshot(ctx, repo, opts.Parent) + id, err := restic.FindSnapshot(ctx, repo.Backend(), opts.Parent) if err != nil { return nil, errors.Fatalf("invalid id %q: %v", opts.Parent, err) } @@ -485,7 +485,7 @@ func findParentSnapshot(ctx context.Context, repo restic.Repository, opts Backup // Find last snapshot to set it as parent, if not already set if !opts.Force && parentID == nil { - id, err := restic.FindLatestSnapshot(ctx, repo, targets, []restic.TagList{}, []string{opts.Host}, &timeStampLimit) + id, err := restic.FindLatestSnapshot(ctx, repo.Backend(), repo, targets, []restic.TagList{}, []string{opts.Host}, &timeStampLimit) if err == nil { parentID = &id } else if err != restic.ErrNoSnapshotFound { @@ -571,14 +571,6 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, term *termstatus.Termina return err } - if !gopts.JSON { - progressPrinter.V("load index files") - } - err = repo.LoadIndex(gopts.ctx) - if err != nil { - return err - } - var parentSnapshotID *restic.ID if !opts.Stdin { parentSnapshotID, err = findParentSnapshot(gopts.ctx, repo, opts, targets, timeStamp) @@ -595,6 +587,14 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, term *termstatus.Termina } } + if !gopts.JSON { + progressPrinter.V("load index files") + } + err = repo.LoadIndex(gopts.ctx) + if err != nil { + return err + } + selectByNameFilter := func(item string) bool { for _, reject := range rejectByNameFuncs { if reject(item) { diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index ec0535db0..86d538e70 100644 --- a/cmd/restic/cmd_cat.go +++ b/cmd/restic/cmd_cat.go @@ -62,7 +62,7 @@ func runCat(gopts GlobalOptions, args []string) error { } // find snapshot id with prefix - id, err = restic.FindSnapshot(gopts.ctx, repo, args[1]) + id, err = restic.FindSnapshot(gopts.ctx, repo.Backend(), args[1]) if err != nil { return errors.Fatalf("could not find snapshot: %v\n", err) } diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index fa71bdba8..e7edff39e 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -211,6 +211,10 @@ func runCheck(opts CheckOptions, gopts GlobalOptions, args []string) error { } chkr := checker.New(repo, opts.CheckUnused) + err = chkr.LoadSnapshots(gopts.ctx) + if err != nil { + return err + } Verbosef("load indexes\n") hints, errs := chkr.LoadIndex(gopts.ctx) diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 048210ba8..03b34ecca 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" @@ -88,6 +89,16 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { return err } + srcSnapshotLister, err := backend.MemorizeList(ctx, srcRepo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + + dstSnapshotLister, err := backend.MemorizeList(ctx, dstRepo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + debug.Log("Loading source index") if err := srcRepo.LoadIndex(ctx); err != nil { return err @@ -99,7 +110,7 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { } dstSnapshotByOriginal := make(map[restic.ID][]*restic.Snapshot) - for sn := range FindFilteredSnapshots(ctx, dstRepo, opts.Hosts, opts.Tags, opts.Paths, nil) { + for sn := range FindFilteredSnapshots(ctx, dstSnapshotLister, dstRepo, opts.Hosts, opts.Tags, opts.Paths, nil) { if sn.Original != nil && !sn.Original.IsNull() { dstSnapshotByOriginal[*sn.Original] = append(dstSnapshotByOriginal[*sn.Original], sn) } @@ -110,7 +121,7 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { // remember already processed trees across all snapshots visitedTrees := restic.NewIDSet() - for sn := range FindFilteredSnapshots(ctx, srcRepo, opts.Hosts, opts.Tags, opts.Paths, args) { + for sn := range FindFilteredSnapshots(ctx, srcSnapshotLister, srcRepo, opts.Hosts, opts.Tags, opts.Paths, args) { Verbosef("\nsnapshot %s of %v at %s)\n", sn.ID().Str(), sn.Paths, sn.Time) // check whether the destination has a snapshot with the same persistent ID which has similar snapshot fields diff --git a/cmd/restic/cmd_debug.go b/cmd/restic/cmd_debug.go index a6e83f98f..7947f789f 100644 --- a/cmd/restic/cmd_debug.go +++ b/cmd/restic/cmd_debug.go @@ -73,7 +73,7 @@ func prettyPrintJSON(wr io.Writer, item interface{}) error { } func debugPrintSnapshots(ctx context.Context, repo *repository.Repository, wr io.Writer) error { - return restic.ForAllSnapshots(ctx, repo, nil, func(id restic.ID, snapshot *restic.Snapshot, err error) error { + return restic.ForAllSnapshots(ctx, repo.Backend(), repo, nil, func(id restic.ID, snapshot *restic.Snapshot, err error) error { if err != nil { return err } diff --git a/cmd/restic/cmd_diff.go b/cmd/restic/cmd_diff.go index f83c87132..8099bf296 100644 --- a/cmd/restic/cmd_diff.go +++ b/cmd/restic/cmd_diff.go @@ -7,9 +7,9 @@ import ( "reflect" "sort" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" "github.com/spf13/cobra" ) @@ -53,8 +53,8 @@ func init() { f.BoolVar(&diffOptions.ShowMetadata, "metadata", false, "print changes in metadata") } -func loadSnapshot(ctx context.Context, repo *repository.Repository, desc string) (*restic.Snapshot, error) { - id, err := restic.FindSnapshot(ctx, repo, desc) +func loadSnapshot(ctx context.Context, be restic.Lister, repo restic.Repository, desc string) (*restic.Snapshot, error) { + id, err := restic.FindSnapshot(ctx, be, desc) if err != nil { return nil, errors.Fatal(err.Error()) } @@ -334,10 +334,6 @@ func runDiff(opts DiffOptions, gopts GlobalOptions, args []string) error { return err } - if err = repo.LoadIndex(ctx); err != nil { - return err - } - if !gopts.NoLock { lock, err := lockRepo(ctx, repo) defer unlockRepo(lock) @@ -346,12 +342,17 @@ func runDiff(opts DiffOptions, gopts GlobalOptions, args []string) error { } } - sn1, err := loadSnapshot(ctx, repo, args[0]) + // cache snapshots listing + be, err := backend.MemorizeList(ctx, repo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + sn1, err := loadSnapshot(ctx, be, repo, args[0]) if err != nil { return err } - sn2, err := loadSnapshot(ctx, repo, args[1]) + sn2, err := loadSnapshot(ctx, be, repo, args[1]) if err != nil { return err } @@ -360,6 +361,10 @@ func runDiff(opts DiffOptions, gopts GlobalOptions, args []string) error { Verbosef("comparing snapshot %v to %v:\n\n", sn1.ID().Str(), sn2.ID().Str()) } + if err = repo.LoadIndex(ctx); err != nil { + return err + } + if sn1.Tree == nil { return errors.Errorf("snapshot %v has nil tree", sn1.ID().Str()) } diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 4449af5ce..b4ce299cd 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -144,20 +144,15 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { } } - err = repo.LoadIndex(ctx) - if err != nil { - return err - } - var id restic.ID if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts, nil) + id, err = restic.FindLatestSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil) if err != nil { Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) } } else { - id, err = restic.FindSnapshot(ctx, repo, snapshotIDString) + id, err = restic.FindSnapshot(ctx, repo.Backend(), snapshotIDString) if err != nil { Exitf(1, "invalid id %q: %v", snapshotIDString, err) } @@ -168,6 +163,11 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { Exitf(2, "loading snapshot %q failed: %v", snapshotIDString, err) } + err = repo.LoadIndex(ctx) + if err != nil { + return err + } + tree, err := repo.LoadTree(ctx, *sn.Tree) if err != nil { Exitf(2, "loading tree for snapshot %q failed: %v", snapshotIDString, err) diff --git a/cmd/restic/cmd_find.go b/cmd/restic/cmd_find.go index 033915ab0..7171314e2 100644 --- a/cmd/restic/cmd_find.go +++ b/cmd/restic/cmd_find.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/filter" @@ -584,6 +585,11 @@ func runFind(opts FindOptions, gopts GlobalOptions, args []string) error { } } + snapshotLister, err := backend.MemorizeList(gopts.ctx, repo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + if err = repo.LoadIndex(gopts.ctx); err != nil { return err } @@ -618,7 +624,7 @@ func runFind(opts FindOptions, gopts GlobalOptions, args []string) error { } } - for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, opts.Snapshots) { + for sn := range FindFilteredSnapshots(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, opts.Snapshots) { if f.blobIDs != nil || f.treeIDs != nil { if err = f.findIDs(ctx, sn); err != nil && err.Error() != "OK" { return err diff --git a/cmd/restic/cmd_forget.go b/cmd/restic/cmd_forget.go index c814472f7..1feb39994 100644 --- a/cmd/restic/cmd_forget.go +++ b/cmd/restic/cmd_forget.go @@ -128,7 +128,7 @@ func runForget(opts ForgetOptions, gopts GlobalOptions, args []string) error { var snapshots restic.Snapshots removeSnIDs := restic.NewIDSet() - for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, args) { + for sn := range FindFilteredSnapshots(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, args) { snapshots = append(snapshots, sn) } diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index 71a44949e..ec6695714 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" @@ -169,6 +170,11 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { return err } + snapshotLister, err := backend.MemorizeList(gopts.ctx, repo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + if err = repo.LoadIndex(gopts.ctx); err != nil { return err } @@ -211,7 +217,7 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { } } - for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, args[:1]) { + for sn := range FindFilteredSnapshots(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, args[:1]) { printSnapshot(sn) err := walker.Walk(ctx, repo, *sn.Tree, nil, func(_ restic.ID, nodepath string, node *restic.Node, err error) (bool, error) { diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index 76d7d7ea9..195b2554d 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -150,6 +150,7 @@ func runPruneWithRepo(opts PruneOptions, gopts GlobalOptions, repo *repository.R } Verbosef("loading indexes...\n") + // loading the index before the snapshots is ok, as we use an exclusive lock here err := repo.LoadIndex(gopts.ctx) if err != nil { return err @@ -554,7 +555,7 @@ func getUsedBlobs(gopts GlobalOptions, repo restic.Repository, ignoreSnapshots r var snapshotTrees restic.IDs Verbosef("loading all snapshots...\n") - err = restic.ForAllSnapshots(gopts.ctx, repo, ignoreSnapshots, + err = restic.ForAllSnapshots(gopts.ctx, repo.Backend(), repo, ignoreSnapshots, func(id restic.ID, sn *restic.Snapshot, err error) error { debug.Log("add snapshot %v (tree %v, error %v)", id, *sn.Tree, err) if err != nil { diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 860c45a57..93810ce0f 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -5,6 +5,7 @@ import ( "os" "time" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" "github.com/spf13/cobra" @@ -50,6 +51,11 @@ func runRecover(gopts GlobalOptions) error { return err } + snapshotLister, err := backend.MemorizeList(gopts.ctx, repo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + Verbosef("load index files\n") if err = repo.LoadIndex(gopts.ctx); err != nil { return err @@ -84,7 +90,7 @@ func runRecover(gopts GlobalOptions) error { bar.Done() Verbosef("load snapshots\n") - err = restic.ForAllSnapshots(gopts.ctx, repo, nil, func(id restic.ID, sn *restic.Snapshot, err error) error { + err = restic.ForAllSnapshots(gopts.ctx, snapshotLister, repo, nil, func(id restic.ID, sn *restic.Snapshot, err error) error { trees[*sn.Tree] = true return nil }) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 4d5818593..3c7baeb0f 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -110,25 +110,25 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { } } - err = repo.LoadIndex(ctx) - if err != nil { - return err - } - var id restic.ID if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo, opts.Paths, opts.Tags, opts.Hosts, nil) + id, err = restic.FindLatestSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil) if err != nil { Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) } } else { - id, err = restic.FindSnapshot(ctx, repo, snapshotIDString) + id, err = restic.FindSnapshot(ctx, repo.Backend(), snapshotIDString) if err != nil { Exitf(1, "invalid id %q: %v", snapshotIDString, err) } } + err = repo.LoadIndex(ctx) + if err != nil { + return err + } + res, err := restorer.NewRestorer(ctx, repo, id) if err != nil { Exitf(2, "creating restorer failed: %v\n", err) diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index 70db09417..d305878da 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -79,7 +79,7 @@ func runSnapshots(opts SnapshotOptions, gopts GlobalOptions, args []string) erro defer cancel() var snapshots restic.Snapshots - for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, args) { + for sn := range FindFilteredSnapshots(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, args) { snapshots = append(snapshots, sn) } snapshotGroups, grouped, err := restic.GroupSnapshots(snapshots, opts.GroupBy) diff --git a/cmd/restic/cmd_stats.go b/cmd/restic/cmd_stats.go index deb649e26..59dc75852 100644 --- a/cmd/restic/cmd_stats.go +++ b/cmd/restic/cmd_stats.go @@ -6,6 +6,7 @@ import ( "fmt" "path/filepath" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/walker" @@ -86,10 +87,6 @@ func runStats(gopts GlobalOptions, args []string) error { return err } - if err = repo.LoadIndex(ctx); err != nil { - return err - } - if !gopts.NoLock { lock, err := lockRepo(ctx, repo) defer unlockRepo(lock) @@ -98,6 +95,15 @@ func runStats(gopts GlobalOptions, args []string) error { } } + snapshotLister, err := backend.MemorizeList(gopts.ctx, repo.Backend(), restic.SnapshotFile) + if err != nil { + return err + } + + if err = repo.LoadIndex(ctx); err != nil { + return err + } + if !gopts.JSON { Printf("scanning...\n") } @@ -111,7 +117,7 @@ func runStats(gopts GlobalOptions, args []string) error { snapshotsCount: 0, } - for sn := range FindFilteredSnapshots(ctx, repo, statsOptions.Hosts, statsOptions.Tags, statsOptions.Paths, args) { + for sn := range FindFilteredSnapshots(ctx, snapshotLister, repo, statsOptions.Hosts, statsOptions.Tags, statsOptions.Paths, args) { err = statsWalkSnapshot(ctx, sn, repo, stats) if err != nil { return fmt.Errorf("error walking snapshot: %v", err) diff --git a/cmd/restic/cmd_tag.go b/cmd/restic/cmd_tag.go index 13842c077..e6688b2fc 100644 --- a/cmd/restic/cmd_tag.go +++ b/cmd/restic/cmd_tag.go @@ -129,7 +129,7 @@ func runTag(opts TagOptions, gopts GlobalOptions, args []string) error { changeCnt := 0 ctx, cancel := context.WithCancel(gopts.ctx) defer cancel() - for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, args) { + for sn := range FindFilteredSnapshots(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, args) { changed, err := changeTags(ctx, repo, sn, opts.SetTags.Flatten(), opts.AddTags.Flatten(), opts.RemoveTags.Flatten()) if err != nil { Warnf("unable to modify the tags for snapshot ID %q, ignoring: %v\n", sn.ID(), err) diff --git a/cmd/restic/find.go b/cmd/restic/find.go index 792f1f6a2..f93f37278 100644 --- a/cmd/restic/find.go +++ b/cmd/restic/find.go @@ -3,33 +3,39 @@ package main import ( "context" - "github.com/restic/restic/internal/repository" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/restic" ) // FindFilteredSnapshots yields Snapshots, either given explicitly by `snapshotIDs` or filtered from the list of all snapshots. -func FindFilteredSnapshots(ctx context.Context, repo *repository.Repository, hosts []string, tags []restic.TagList, paths []string, snapshotIDs []string) <-chan *restic.Snapshot { +func FindFilteredSnapshots(ctx context.Context, be restic.Lister, loader restic.LoadJSONUnpackeder, hosts []string, tags []restic.TagList, paths []string, snapshotIDs []string) <-chan *restic.Snapshot { out := make(chan *restic.Snapshot) go func() { defer close(out) if len(snapshotIDs) != 0 { + // memorize snapshots list to prevent repeated backend listings + be, err := backend.MemorizeList(ctx, be, restic.SnapshotFile) + if err != nil { + Warnf("could not load snapshots: %v\n", err) + return + } + var ( id restic.ID usedFilter bool - err error ) ids := make(restic.IDs, 0, len(snapshotIDs)) // Process all snapshot IDs given as arguments. for _, s := range snapshotIDs { if s == "latest" { usedFilter = true - id, err = restic.FindLatestSnapshot(ctx, repo, paths, tags, hosts, nil) + id, err = restic.FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) if err != nil { Warnf("Ignoring %q, no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)\n", s, paths, tags, hosts) continue } } else { - id, err = restic.FindSnapshot(ctx, repo, s) + id, err = restic.FindSnapshot(ctx, be, s) if err != nil { Warnf("Ignoring %q: %v\n", s, err) continue @@ -44,7 +50,7 @@ func FindFilteredSnapshots(ctx context.Context, repo *repository.Repository, hos } for _, id := range ids.Uniq() { - sn, err := restic.LoadSnapshot(ctx, repo, id) + sn, err := restic.LoadSnapshot(ctx, loader, id) if err != nil { Warnf("Ignoring %q, could not load snapshot: %v\n", id, err) continue @@ -58,7 +64,7 @@ func FindFilteredSnapshots(ctx context.Context, repo *repository.Repository, hos return } - snapshots, err := restic.FindFilteredSnapshots(ctx, repo, hosts, tags, paths) + snapshots, err := restic.FindFilteredSnapshots(ctx, be, loader, hosts, tags, paths) if err != nil { Warnf("could not load snapshots: %v\n", err) return diff --git a/cmd/restic/integration_fuse_test.go b/cmd/restic/integration_fuse_test.go index 1337be88e..156a8abae 100644 --- a/cmd/restic/integration_fuse_test.go +++ b/cmd/restic/integration_fuse_test.go @@ -154,6 +154,8 @@ func TestMount(t *testing.T) { } env, cleanup := withTestEnvironment(t) + // must list snapshots more than once + env.gopts.backendTestHook = nil defer cleanup() testRunInit(t, env.gopts) @@ -197,6 +199,8 @@ func TestMountSameTimestamps(t *testing.T) { } env, cleanup := withTestEnvironment(t) + // must list snapshots more than once + env.gopts.backendTestHook = nil defer cleanup() rtest.SetupTarTestFixture(t, env.base, filepath.Join("testdata", "repo-same-timestamps.tar.gz")) diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index df9893350..e87baddca 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/cmd/restic/integration_helpers_test.go @@ -198,6 +198,9 @@ func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) { stdout: os.Stdout, stderr: os.Stderr, extended: make(options.Options), + + // replace this hook with "nil" if listing a filetype more than once is necessary + backendTestHook: func(r restic.Backend) (restic.Backend, error) { return newOrderedListOnceBackend(r), nil }, } // always overwrite global options diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index f926306eb..49121bb1d 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -275,6 +275,11 @@ func testRunForgetJSON(t testing.TB, gopts GlobalOptions, args ...string) { } func testRunPrune(t testing.TB, gopts GlobalOptions, opts PruneOptions) { + oldHook := gopts.backendTestHook + gopts.backendTestHook = func(r restic.Backend) (restic.Backend, error) { return newListOnceBackend(r), nil } + defer func() { + gopts.backendTestHook = oldHook + }() rtest.OK(t, runPrune(opts, gopts)) } @@ -1065,6 +1070,8 @@ func TestKeyAddRemove(t *testing.T) { } env, cleanup := withTestEnvironment(t) + // must list keys more than once + env.gopts.backendTestHook = nil defer cleanup() testRunInit(t, env.gopts) @@ -1659,6 +1666,11 @@ func TestPruneWithDamagedRepository(t *testing.T) { rtest.Assert(t, len(snapshotIDs) == 1, "expected one snapshot, got %v", snapshotIDs) + oldHook := env.gopts.backendTestHook + env.gopts.backendTestHook = func(r restic.Backend) (restic.Backend, error) { return newListOnceBackend(r), nil } + defer func() { + env.gopts.backendTestHook = oldHook + }() // prune should fail rtest.Assert(t, runPrune(pruneDefaultOptions, env.gopts) == errorPacksMissing, "prune should have reported index not complete error") @@ -1752,12 +1764,22 @@ func testEdgeCaseRepo(t *testing.T, tarfile string, optionsCheck CheckOptions, o type listOnceBackend struct { restic.Backend listedFileType map[restic.FileType]bool + strictOrder bool } func newListOnceBackend(be restic.Backend) *listOnceBackend { return &listOnceBackend{ Backend: be, listedFileType: make(map[restic.FileType]bool), + strictOrder: false, + } +} + +func newOrderedListOnceBackend(be restic.Backend) *listOnceBackend { + return &listOnceBackend{ + Backend: be, + listedFileType: make(map[restic.FileType]bool), + strictOrder: true, } } @@ -1765,6 +1787,9 @@ func (be *listOnceBackend) List(ctx context.Context, t restic.FileType, fn func( if t != restic.LockFile && be.listedFileType[t] { return errors.Errorf("tried listing type %v the second time", t) } + if be.strictOrder && t == restic.SnapshotFile && be.listedFileType[restic.IndexFile] { + return errors.Errorf("tried listing type snapshots after index") + } be.listedFileType[t] = true return be.Backend.List(ctx, t, fn) } @@ -2136,3 +2161,37 @@ func TestBackendLoadWriteTo(t *testing.T) { rtest.Assert(t, len(firstSnapshot) == 1, "expected one snapshot, got %v", firstSnapshot) } + +func TestFindListOnce(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + env.gopts.backendTestHook = func(r restic.Backend) (restic.Backend, error) { + return newListOnceBackend(r), nil + } + + testSetupBackupData(t, env) + opts := BackupOptions{} + + testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9")}, opts, env.gopts) + testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9", "2")}, opts, env.gopts) + secondSnapshot := testRunList(t, "snapshots", env.gopts) + testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9", "3")}, opts, env.gopts) + thirdSnapshot := restic.NewIDSet(testRunList(t, "snapshots", env.gopts)...) + + repo, err := OpenRepository(env.gopts) + rtest.OK(t, err) + + snapshotIDs := restic.NewIDSet() + // specify the two oldest snapshots explicitly and use "latest" to reference the newest one + for sn := range FindFilteredSnapshots(context.TODO(), repo.Backend(), repo, nil, nil, nil, []string{ + secondSnapshot[0].String(), + secondSnapshot[1].String()[:8], + "latest", + }) { + snapshotIDs.Insert(*sn.ID()) + } + + // the snapshots can only be listed once, if both lists match then the there has been only a single List() call + rtest.Equals(t, thirdSnapshot, snapshotIDs) +} diff --git a/internal/backend/utils.go b/internal/backend/utils.go index 39c68b4ce..be1c2a9e0 100644 --- a/internal/backend/utils.go +++ b/internal/backend/utils.go @@ -3,6 +3,7 @@ package backend import ( "bytes" "context" + "fmt" "io" "github.com/restic/restic/internal/restic" @@ -57,3 +58,44 @@ func DefaultLoad(ctx context.Context, h restic.Handle, length int, offset int64, } return rd.Close() } + +type memorizedLister struct { + fileInfos []restic.FileInfo + tpe restic.FileType +} + +func (m *memorizedLister) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error { + if t != m.tpe { + return fmt.Errorf("filetype mismatch, expected %s got %s", m.tpe, t) + } + for _, fi := range m.fileInfos { + if ctx.Err() != nil { + break + } + err := fn(fi) + if err != nil { + return err + } + } + return ctx.Err() +} + +func MemorizeList(ctx context.Context, be restic.Lister, t restic.FileType) (restic.Lister, error) { + if _, ok := be.(*memorizedLister); ok { + return be, nil + } + + var fileInfos []restic.FileInfo + err := be.List(ctx, t, func(fi restic.FileInfo) error { + fileInfos = append(fileInfos, fi) + return nil + }) + if err != nil { + return nil, err + } + + return &memorizedLister{ + fileInfos: fileInfos, + tpe: t, + }, nil +} diff --git a/internal/backend/utils_test.go b/internal/backend/utils_test.go index 1030537bc..0841a57e4 100644 --- a/internal/backend/utils_test.go +++ b/internal/backend/utils_test.go @@ -3,6 +3,7 @@ package backend_test import ( "bytes" "context" + "fmt" "io" "math/rand" "testing" @@ -10,6 +11,7 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/backend/mem" "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/mock" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -157,3 +159,47 @@ func TestDefaultLoad(t *testing.T) { rtest.Equals(t, true, rd.closed) rtest.Equals(t, "consumer error", err.Error()) } + +func TestMemoizeList(t *testing.T) { + // setup backend to serve as data source for memoized list + be := mock.NewBackend() + files := []restic.FileInfo{ + {Size: 42, Name: restic.NewRandomID().String()}, + {Size: 45, Name: restic.NewRandomID().String()}, + } + be.ListFn = func(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error { + for _, fi := range files { + if err := fn(fi); err != nil { + return err + } + } + return nil + } + + mem, err := backend.MemorizeList(context.TODO(), be, restic.SnapshotFile) + rtest.OK(t, err) + + err = mem.List(context.TODO(), restic.IndexFile, func(fi restic.FileInfo) error { + t.Fatal("file type mismatch") + return nil // the memoized lister must return an error by itself + }) + rtest.Assert(t, err != nil, "missing error on file typ mismatch") + + var memFiles []restic.FileInfo + err = mem.List(context.TODO(), restic.SnapshotFile, func(fi restic.FileInfo) error { + memFiles = append(memFiles, fi) + return nil + }) + rtest.OK(t, err) + rtest.Equals(t, files, memFiles) +} + +func TestMemoizeListError(t *testing.T) { + // setup backend to serve as data source for memoized list + be := mock.NewBackend() + be.ListFn = func(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error { + return fmt.Errorf("list error") + } + _, err := backend.MemorizeList(context.TODO(), be, restic.SnapshotFile) + rtest.Assert(t, err != nil, "missing error on list error") +} diff --git a/internal/checker/checker.go b/internal/checker/checker.go index ebf38b632..8e49209f5 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -11,6 +11,7 @@ import ( "sync" "github.com/minio/sha256-simd" + "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/hashing" @@ -35,6 +36,7 @@ type Checker struct { trackUnused bool masterIndex *repository.MasterIndex + snapshots restic.Lister repo restic.Repository } @@ -75,6 +77,12 @@ func (err ErrOldIndexFormat) Error() string { return fmt.Sprintf("index %v has old format", err.ID.Str()) } +func (c *Checker) LoadSnapshots(ctx context.Context) error { + var err error + c.snapshots, err = backend.MemorizeList(ctx, c.repo.Backend(), restic.SnapshotFile) + return err +} + // LoadIndex loads all index files. func (c *Checker) LoadIndex(ctx context.Context) (hints []error, errs []error) { debug.Log("Start") @@ -278,8 +286,8 @@ func (c *Checker) checkTreeWorker(ctx context.Context, trees <-chan restic.TreeI } } -func loadSnapshotTreeIDs(ctx context.Context, repo restic.Repository) (ids restic.IDs, errs []error) { - err := restic.ForAllSnapshots(ctx, repo, nil, func(id restic.ID, sn *restic.Snapshot, err error) error { +func loadSnapshotTreeIDs(ctx context.Context, lister restic.Lister, repo restic.Repository) (ids restic.IDs, errs []error) { + err := restic.ForAllSnapshots(ctx, lister, repo, nil, func(id restic.ID, sn *restic.Snapshot, err error) error { if err != nil { errs = append(errs, err) return nil @@ -300,7 +308,7 @@ func loadSnapshotTreeIDs(ctx context.Context, repo restic.Repository) (ids resti // subtrees are available in the index. errChan is closed after all trees have // been traversed. func (c *Checker) Structure(ctx context.Context, p *progress.Counter, errChan chan<- error) { - trees, errs := loadSnapshotTreeIDs(ctx, c.repo) + trees, errs := loadSnapshotTreeIDs(ctx, c.snapshots, c.repo) p.SetMax(uint64(len(trees))) debug.Log("need to check %d trees from snapshots, %d errs returned", len(trees), len(errs)) diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index 6d17a4593..1330211eb 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -44,6 +44,10 @@ func checkPacks(chkr *checker.Checker) []error { } func checkStruct(chkr *checker.Checker) []error { + err := chkr.LoadSnapshots(context.TODO()) + if err != nil { + return []error{err} + } return collectErrors(context.TODO(), func(ctx context.Context, errChan chan<- error) { chkr.Structure(ctx, nil, errChan) }) @@ -587,7 +591,7 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { chkr, repo, cleanup := loadBenchRepository(t) defer cleanup() - snID, err := restic.FindSnapshot(context.TODO(), repo, "51d249d2") + snID, err := restic.FindSnapshot(context.TODO(), repo.Backend(), "51d249d2") if err != nil { t.Fatal(err) } diff --git a/internal/checker/testing.go b/internal/checker/testing.go index d672911b1..0668406d8 100644 --- a/internal/checker/testing.go +++ b/internal/checker/testing.go @@ -20,6 +20,11 @@ func TestCheckRepo(t testing.TB, repo restic.Repository) { t.Fatalf("errors loading index: %v", hints) } + err := chkr.LoadSnapshots(context.TODO()) + if err != nil { + t.Error(err) + } + // packs errChan := make(chan error) go chkr.Packs(context.TODO(), errChan) diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index 40854082e..88638d50c 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -223,7 +223,7 @@ func updateSnapshots(ctx context.Context, root *Root) error { return nil } - snapshots, err := restic.FindFilteredSnapshots(ctx, root.repo, root.cfg.Hosts, root.cfg.Tags, root.cfg.Paths) + snapshots, err := restic.FindFilteredSnapshots(ctx, root.repo.Backend(), root.repo, root.cfg.Hosts, root.cfg.Tags, root.cfg.Paths) if err != nil { return err } diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index f1fe9af7e..2470dadfc 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -369,6 +369,11 @@ func TestIndexSave(t *testing.T) { } checker := checker.New(repo, false) + err = checker.LoadSnapshots(context.TODO()) + if err != nil { + t.Error(err) + } + hints, errs := checker.LoadIndex(context.TODO()) for _, h := range hints { t.Logf("hint: %v\n", h) diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 35f2f997b..61654fa1d 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -60,6 +60,11 @@ type Lister interface { List(context.Context, FileType, func(FileInfo) error) error } +// LoadJSONUnpackeder allows loading a JSON file not stored in a pack file +type LoadJSONUnpackeder interface { + LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error +} + type PackBlobs struct { PackID ID Blobs []Blob diff --git a/internal/restic/snapshot.go b/internal/restic/snapshot.go index ac5f2cf44..a74893778 100644 --- a/internal/restic/snapshot.go +++ b/internal/restic/snapshot.go @@ -59,9 +59,9 @@ func NewSnapshot(paths []string, tags []string, hostname string, time time.Time) } // LoadSnapshot loads the snapshot with the id and returns it. -func LoadSnapshot(ctx context.Context, repo Repository, id ID) (*Snapshot, error) { +func LoadSnapshot(ctx context.Context, loader LoadJSONUnpackeder, id ID) (*Snapshot, error) { sn := &Snapshot{id: &id} - err := repo.LoadJSONUnpacked(ctx, SnapshotFile, id, sn) + err := loader.LoadJSONUnpacked(ctx, SnapshotFile, id, sn) if err != nil { return nil, err } @@ -76,7 +76,7 @@ const loadSnapshotParallelism = 5 // If the called function returns an error, this function is cancelled and // also returns this error. // If a snapshot ID is in excludeIDs, it will be ignored. -func ForAllSnapshots(ctx context.Context, repo Repository, excludeIDs IDSet, fn func(ID, *Snapshot, error) error) error { +func ForAllSnapshots(ctx context.Context, be Lister, loader LoadJSONUnpackeder, excludeIDs IDSet, fn func(ID, *Snapshot, error) error) error { var m sync.Mutex // track spawned goroutines using wg, create a new context which is @@ -88,7 +88,13 @@ func ForAllSnapshots(ctx context.Context, repo Repository, excludeIDs IDSet, fn // send list of snapshot files through ch, which is closed afterwards wg.Go(func() error { defer close(ch) - return repo.List(ctx, SnapshotFile, func(id ID, size int64) error { + return be.List(ctx, SnapshotFile, func(fi FileInfo) error { + id, err := ParseID(fi.Name) + if err != nil { + debug.Log("unable to parse %v as an ID", fi.Name) + return nil + } + if excludeIDs.Has(id) { return nil } @@ -107,7 +113,7 @@ func ForAllSnapshots(ctx context.Context, repo Repository, excludeIDs IDSet, fn worker := func() error { for id := range ch { debug.Log("load snapshot %v", id) - sn, err := LoadSnapshot(ctx, repo, id) + sn, err := LoadSnapshot(ctx, loader, id) m.Lock() err = fn(id, sn, err) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 1f309f0bd..74275b552 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -14,7 +14,9 @@ import ( var ErrNoSnapshotFound = errors.New("no snapshot found") // FindLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. -func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, tagLists []TagList, hostnames []string, timeStampLimit *time.Time) (ID, error) { +func FindLatestSnapshot(ctx context.Context, be Lister, loader LoadJSONUnpackeder, targets []string, + tagLists []TagList, hostnames []string, timeStampLimit *time.Time) (ID, error) { + var err error absTargets := make([]string, 0, len(targets)) for _, target := range targets { @@ -33,7 +35,7 @@ func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, found bool ) - err = ForAllSnapshots(ctx, repo, nil, func(id ID, snapshot *Snapshot, err error) error { + err = ForAllSnapshots(ctx, be, loader, nil, func(id ID, snapshot *Snapshot, err error) error { if err != nil { return errors.Errorf("Error loading snapshot %v: %v", id.Str(), err) } @@ -77,10 +79,10 @@ func FindLatestSnapshot(ctx context.Context, repo Repository, targets []string, // FindSnapshot takes a string and tries to find a snapshot whose ID matches // the string as closely as possible. -func FindSnapshot(ctx context.Context, repo Repository, s string) (ID, error) { +func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { // find snapshot id with prefix - name, err := Find(ctx, repo.Backend(), SnapshotFile, s) + name, err := Find(ctx, be, SnapshotFile, s) if err != nil { return ID{}, err } @@ -90,10 +92,10 @@ func FindSnapshot(ctx context.Context, repo Repository, s string) (ID, error) { // FindFilteredSnapshots yields Snapshots filtered from the list of all // snapshots. -func FindFilteredSnapshots(ctx context.Context, repo Repository, hosts []string, tags []TagList, paths []string) (Snapshots, error) { +func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoadJSONUnpackeder, hosts []string, tags []TagList, paths []string) (Snapshots, error) { results := make(Snapshots, 0, 20) - err := ForAllSnapshots(ctx, repo, nil, func(id ID, sn *Snapshot, err error) error { + err := ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error { if err != nil { fmt.Fprintf(os.Stderr, "could not load snapshot %v: %v\n", id.Str(), err) return nil diff --git a/internal/restic/snapshot_find_test.go b/internal/restic/snapshot_find_test.go index d4fedcc9d..534eb456d 100644 --- a/internal/restic/snapshot_find_test.go +++ b/internal/restic/snapshot_find_test.go @@ -16,7 +16,7 @@ func TestFindLatestSnapshot(t *testing.T) { restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0) latestSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0) - id, err := restic.FindLatestSnapshot(context.TODO(), repo, []string{}, []restic.TagList{}, []string{"foo"}, nil) + id, err := restic.FindLatestSnapshot(context.TODO(), repo.Backend(), repo, []string{}, []restic.TagList{}, []string{"foo"}, nil) if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } @@ -36,7 +36,7 @@ func TestFindLatestSnapshotWithMaxTimestamp(t *testing.T) { maxTimestamp := parseTimeUTC("2018-08-08 08:08:08") - id, err := restic.FindLatestSnapshot(context.TODO(), repo, []string{}, []restic.TagList{}, []string{"foo"}, &maxTimestamp) + id, err := restic.FindLatestSnapshot(context.TODO(), repo.Backend(), repo, []string{}, []restic.TagList{}, []string{"foo"}, &maxTimestamp) if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } diff --git a/internal/restic/testing_test.go b/internal/restic/testing_test.go index 5607dc5fa..7ee7461a5 100644 --- a/internal/restic/testing_test.go +++ b/internal/restic/testing_test.go @@ -20,7 +20,7 @@ const ( // LoadAllSnapshots returns a list of all snapshots in the repo. // If a snapshot ID is in excludeIDs, it will not be included in the result. func loadAllSnapshots(ctx context.Context, repo restic.Repository, excludeIDs restic.IDSet) (snapshots restic.Snapshots, err error) { - err = restic.ForAllSnapshots(ctx, repo, excludeIDs, func(id restic.ID, sn *restic.Snapshot, err error) error { + err = restic.ForAllSnapshots(ctx, repo.Backend(), repo, excludeIDs, func(id restic.ID, sn *restic.Snapshot, err error) error { if err != nil { return err }