From da57302fca0d80586d6151f5eaf12dd9a61667c8 Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Fri, 11 May 2018 00:45:14 -0400 Subject: [PATCH] restore: Removed legacy restore implementation Signed-off-by: Igor Fedorenko --- changelog/unreleased/issue-1605 | 4 --- cmd/restic/cmd_restore.go | 18 +++++------ internal/restic/node.go | 21 ++---------- internal/restic/node_test.go | 4 +-- internal/restorer/filerestorer.go | 2 -- internal/restorer/restorer.go | 52 +++++++++++------------------- internal/restorer/restorer_test.go | 4 +-- 7 files changed, 33 insertions(+), 72 deletions(-) diff --git a/changelog/unreleased/issue-1605 b/changelog/unreleased/issue-1605 index ba93051ac..2d8597610 100644 --- a/changelog/unreleased/issue-1605 +++ b/changelog/unreleased/issue-1605 @@ -7,9 +7,5 @@ Implementation uses threads to download and process miltiple remote files concurrently. To further reduce restore time, each remote file is downloaded using single repository request. -Old restore implementation can be enabled with `--signethreaded` flag. -Use `--verify` restore flag to read restored files and verify their -content checksum. - https://github.com/restic/restic/issues/1605 https://github.com/restic/restic/pull/1719 diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index c538f33ae..4bf59c06f 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -28,14 +28,13 @@ repository. // RestoreOptions collects all options for the restore command. type RestoreOptions struct { - Exclude []string - Include []string - Target string - Host string - Paths []string - Tags restic.TagLists - Verify bool - SingleThreaded bool + Exclude []string + Include []string + Target string + Host string + Paths []string + Tags restic.TagLists + Verify bool } var restoreOptions RestoreOptions @@ -52,7 +51,6 @@ func init() { flags.Var(&restoreOptions.Tags, "tag", "only consider snapshots which include this `taglist` for snapshot ID \"latest\"") flags.StringArrayVar(&restoreOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` for snapshot ID \"latest\"") flags.BoolVar(&restoreOptions.Verify, "verify", false, "verify restored files content") - flags.BoolVar(&restoreOptions.SingleThreaded, "singlethreaded", false, "use single-threaded (legacy) restore implementation") } func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { @@ -157,7 +155,7 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { Verbosef("restoring %s to %s\n", res.Snapshot(), opts.Target) - err = res.RestoreTo(ctx, opts.Target, opts.SingleThreaded) + err = res.RestoreTo(ctx, opts.Target) if err == nil && opts.Verify { Verbosef("verifying files in %s\n", opts.Target) var count int diff --git a/internal/restic/node.go b/internal/restic/node.go index c8b089791..638306eac 100644 --- a/internal/restic/node.go +++ b/internal/restic/node.go @@ -135,7 +135,7 @@ func (node Node) GetExtendedAttribute(a string) []byte { } // CreateAt creates the node at the given path but does NOT restore node meta data. -func (node *Node) CreateAt(ctx context.Context, path string, repo Repository, idx *HardlinkIndex) error { +func (node *Node) CreateAt(ctx context.Context, path string, repo Repository) error { debug.Log("create node %v at %v", node.Name, path) switch node.Type { @@ -144,7 +144,7 @@ func (node *Node) CreateAt(ctx context.Context, path string, repo Repository, id return err } case "file": - if err := node.createFileAt(ctx, path, repo, idx); err != nil { + if err := node.createFileAt(ctx, path, repo); err != nil { return err } case "symlink": @@ -259,18 +259,7 @@ func (node Node) createDirAt(path string) error { return nil } -func (node Node) createFileAt(ctx context.Context, path string, repo Repository, idx *HardlinkIndex) error { - if node.Links > 1 && idx.Has(node.Inode, node.DeviceID) { - if err := fs.Remove(path); !os.IsNotExist(err) { - return errors.Wrap(err, "RemoveCreateHardlink") - } - err := fs.Link(idx.GetFilename(node.Inode, node.DeviceID), path) - if err != nil { - return errors.Wrap(err, "CreateHardlink") - } - return nil - } - +func (node Node) createFileAt(ctx context.Context, path string, repo Repository) error { f, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) if err != nil { return errors.Wrap(err, "OpenFile") @@ -287,10 +276,6 @@ func (node Node) createFileAt(ctx context.Context, path string, repo Repository, return errors.Wrap(closeErr, "Close") } - if node.Links > 1 { - idx.Add(node.Inode, node.DeviceID, path) - } - return nil } diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index e262a36c6..f12353a0a 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -177,11 +177,9 @@ func TestNodeRestoreAt(t *testing.T) { } }() - idx := restic.NewHardlinkIndex() - for _, test := range nodeTests { nodePath := filepath.Join(tempdir, test.Name) - rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil, idx)) + rtest.OK(t, test.CreateAt(context.TODO(), nodePath, nil)) rtest.OK(t, test.RestoreMetadata(nodePath)) if test.Type == "symlink" && runtime.GOOS == "windows" { diff --git a/internal/restorer/filerestorer.go b/internal/restorer/filerestorer.go index 702f5a12e..14cf9117a 100644 --- a/internal/restorer/filerestorer.go +++ b/internal/restorer/filerestorer.go @@ -18,8 +18,6 @@ import ( // TODO evaluate memory footprint for larger repositories, say 10M packs/10M files // TODO consider replacing pack file cache with blob cache // TODO avoid decrypting the same blob multiple times -// TODO remove `restore --singlethreaded` and review/simplify hardlink support -// (node.CreateAt shouldn't take HardlinkIndex) // TODO evaluate disabled debug logging overhead for large repositories // TODO consider logging snapshot-relative path to reduce log clutter diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 8d35b015f..6986c8549 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -140,10 +140,10 @@ func (res *Restorer) traverseTree(ctx context.Context, target, location string, return nil } -func (res *Restorer) restoreNodeTo(ctx context.Context, node *restic.Node, target, location string, idx *restic.HardlinkIndex) error { +func (res *Restorer) restoreNodeTo(ctx context.Context, node *restic.Node, target, location string) error { debug.Log("restoreNode %v %v %v", node.Name, target, location) - err := node.CreateAt(ctx, target, res.repo, idx) + err := node.CreateAt(ctx, target, res.repo) if err != nil { debug.Log("node.CreateAt(%s) error %v", target, err) } @@ -163,9 +163,20 @@ func (res *Restorer) restoreNodeMetadataTo(node *restic.Node, target, location s return err } +func (res *Restorer) restoreHardlinkAt(node *restic.Node, target, path, location string) error { + if err := fs.Remove(path); !os.IsNotExist(err) { + return errors.Wrap(err, "RemoveCreateHardlink") + } + err := fs.Link(target, path) + if err != nil { + return errors.Wrap(err, "CreateHardlink") + } + return res.restoreNodeMetadataTo(node, target, location) +} + // RestoreTo creates the directories and files in the snapshot below dst. // Before an item is created, res.Filter is called. -func (res *Restorer) RestoreTo(ctx context.Context, dst string, singlethreaded bool) error { +func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { var err error if !filepath.IsAbs(dst) { dst, err = filepath.Abs(dst) @@ -180,31 +191,6 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string, singlethreaded b noop := func(node *restic.Node, target, location string) error { return nil } idx := restic.NewHardlinkIndex() - if singlethreaded { - return res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ - enterDir: func(node *restic.Node, target, location string) error { - // create dir with default permissions - // #leaveDir restores dir metadata after visiting all children - return fs.MkdirAll(target, 0700) - }, - - visitNode: func(node *restic.Node, target, location string) error { - // create parent dir with default permissions - // #leaveDir restores dir metadata after visiting all children - err := fs.MkdirAll(filepath.Dir(target), 0700) - if err != nil { - return err - } - - return res.restoreNodeTo(ctx, node, target, location, idx) - }, - - // Restore directory permissions and timestamp at the end. If we did it earlier - // - children restore could fail because of restictive directory permission - // - children restore could overwrite the timestamp of the directory they are in - leaveDir: restoreNodeMetadata, - }) - } filerestorer := newFileRestorer(res.repo.Backend().Load, res.repo.Key(), filePackTraverser{lookup: res.repo.Index().Lookup}) @@ -258,15 +244,15 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string, singlethreaded b return res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ enterDir: noop, visitNode: func(node *restic.Node, target, location string) error { - isHardlink := func() bool { - return idx.Has(node.Inode, node.DeviceID) && idx.GetFilename(node.Inode, node.DeviceID) != target + if idx.Has(node.Inode, node.DeviceID) && idx.GetFilename(node.Inode, node.DeviceID) != target { + return res.restoreHardlinkAt(node, idx.GetFilename(node.Inode, node.DeviceID), target, location) } - if node.Type != "file" || isHardlink() { - return res.restoreNodeTo(ctx, node, target, location, idx) + if node.Type != "file" { + return res.restoreNodeTo(ctx, node, target, location) } - return node.RestoreMetadata(target) + return res.restoreNodeMetadataTo(node, target, location) }, leaveDir: restoreNodeMetadata, }) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 776dbf82e..a0194fa30 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -340,7 +340,7 @@ func TestRestorer(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, tempdir, false) + err = res.RestoreTo(ctx, tempdir) if err != nil { t.Fatal(err) } @@ -446,7 +446,7 @@ func TestRestorerRelative(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, "restore", false) + err = res.RestoreTo(ctx, "restore") if err != nil { t.Fatal(err) }