restore: Removed legacy restore implementation

Signed-off-by: Igor Fedorenko <igor@ifedorenko.com>

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

cmd/restic/cmd_restore.go

@@ -28,14 +28,13 @@ repository.
 
 // RestoreOptions collects all options for the restore command.
 type RestoreOptions struct {
-	Exclude        []string
+	Exclude []string
-	Include        []string
+	Include []string
-	Target         string
+	Target  string
-	Host           string
+	Host    string
-	Paths          []string
+	Paths   []string
-	Tags           restic.TagLists
+	Tags    restic.TagLists
-	Verify         bool
+	Verify  bool
-	SingleThreaded 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

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 {
+func (node Node) createFileAt(ctx context.Context, path string, repo Repository) 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
-	}
-
 	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
 }
 

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" {

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
 

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 {
+			if idx.Has(node.Inode, node.DeviceID) && idx.GetFilename(node.Inode, node.DeviceID) != target {
-				return 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() {
+			if node.Type != "file" {
-				return res.restoreNodeTo(ctx, node, target, location, idx)
+				return res.restoreNodeTo(ctx, node, target, location)
 			}
 
-			return node.RestoreMetadata(target)
+			return res.restoreNodeMetadataTo(node, target, location)
 		},
 		leaveDir: restoreNodeMetadata,
 	})

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)
 			}