From 9e24154ec9f6f33a9ec0f7dd1b9c6345c0027f3d Mon Sep 17 00:00:00 2001 From: Igor Fedorenko Date: Fri, 14 Sep 2018 20:55:30 -0400 Subject: [PATCH] restore: significantly reduce memory footprint reworked restore error callback to use file location path instead of much heavier Node. this reduced restore memory usage by as much as 50% in some of my tests. Signed-off-by: Igor Fedorenko --- cmd/restic/cmd_restore.go | 4 +- internal/restorer/restorer.go | 18 +++----- internal/restorer/restorer_test.go | 70 +++++++++++++++--------------- 3 files changed, 45 insertions(+), 47 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 4bf59c06f..477192eab 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -113,8 +113,8 @@ func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { } totalErrors := 0 - res.Error = func(dir string, node *restic.Node, err error) error { - Warnf("ignoring error for %s: %s\n", dir, err) + res.Error = func(location string, err error) error { + Warnf("ignoring error for %s: %s\n", location, err) totalErrors++ return nil } diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index ea61d3862..e328402c3 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -18,11 +18,11 @@ type Restorer struct { repo restic.Repository sn *restic.Snapshot - Error func(dir string, node *restic.Node, err error) error + Error func(location string, err error) error SelectFilter func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) } -var restorerAbortOnAllErrors = func(str string, node *restic.Node, err error) error { return err } +var restorerAbortOnAllErrors = func(location string, err error) error { return err } // NewRestorer creates a restorer preloaded with the content from the snapshot id. func NewRestorer(repo restic.Repository, id restic.ID) (*Restorer, error) { @@ -55,7 +55,7 @@ func (res *Restorer) traverseTree(ctx context.Context, target, location string, tree, err := res.repo.LoadTree(ctx, treeID) if err != nil { debug.Log("error loading tree %v: %v", treeID, err) - return res.Error(location, nil, err) + return res.Error(location, err) } for _, node := range tree.Nodes { @@ -65,7 +65,7 @@ func (res *Restorer) traverseTree(ctx context.Context, target, location string, nodeName := filepath.Base(filepath.Join(string(filepath.Separator), node.Name)) if nodeName != node.Name { debug.Log("node %q has invalid name %q", node.Name, nodeName) - err := res.Error(location, node, errors.New("node has invalid name")) + err := res.Error(location, errors.Errorf("invalid child node name %s", node.Name)) if err != nil { return err } @@ -78,7 +78,7 @@ func (res *Restorer) traverseTree(ctx context.Context, target, location string, if target == nodeTarget || !fs.HasPathPrefix(target, nodeTarget) { debug.Log("target: %v %v", target, nodeTarget) debug.Log("node %q has invalid target path %q", node.Name, nodeTarget) - err := res.Error(nodeLocation, node, errors.New("node has invalid path")) + err := res.Error(nodeLocation, errors.New("node has invalid path")) if err != nil { return err } @@ -95,7 +95,7 @@ func (res *Restorer) traverseTree(ctx context.Context, target, location string, sanitizeError := func(err error) error { if err != nil { - err = res.Error(nodeTarget, node, err) + err = res.Error(nodeLocation, err) } return err } @@ -194,9 +194,6 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { filerestorer := newFileRestorer(dst, res.repo.Backend().Load, res.repo.Key(), filePackTraverser{lookup: res.repo.Index().Lookup}) - // path->node map, only needed to call res.Error, which uses the node during tests - nodes := make(map[string]*restic.Node) - // first tree pass: create directories and collect all files to restore err = res.traverseTree(ctx, dst, string(filepath.Separator), *res.sn.Tree, treeVisitor{ enterDir: func(node *restic.Node, target, location string) error { @@ -224,7 +221,6 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { idx.Add(node.Inode, node.DeviceID, location) } - nodes[target] = node filerestorer.addFile(location, node.Content) return nil @@ -235,7 +231,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { return err } - err = filerestorer.restoreFiles(ctx, func(path string, err error) { res.Error(path, nodes[path], err) }) + err = filerestorer.restoreFiles(ctx, func(location string, err error) { res.Error(location, err) }) if err != nil { return err } diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index a0194fa30..1e73674cd 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -134,8 +134,8 @@ func TestRestorer(t *testing.T) { var tests = []struct { Snapshot Files map[string]string - ErrorsMust map[string]string - ErrorsMay map[string]string + ErrorsMust map[string]map[string]struct{} + ErrorsMay map[string]map[string]struct{} Select func(item string, dstpath string, node *restic.Node) (selectForRestore bool, childMayBeSelected bool) }{ // valid test cases @@ -249,9 +249,11 @@ func TestRestorer(t *testing.T) { `..\..\foo\..\bar\..\xx\test2`: File{"test2\n"}, }, }, - ErrorsMay: map[string]string{ - `/#..\test`: "node has invalid name", - `/#..\..\foo\..\bar\..\xx\test2`: "node has invalid name", + ErrorsMay: map[string]map[string]struct{}{ + `/`: { + `invalid child node name ..\test`: struct{}{}, + `invalid child node name ..\..\foo\..\bar\..\xx\test2`: struct{}{}, + }, }, }, { @@ -261,9 +263,11 @@ func TestRestorer(t *testing.T) { `../../foo/../bar/../xx/test2`: File{"test2\n"}, }, }, - ErrorsMay: map[string]string{ - `/#../test`: "node has invalid name", - `/#../../foo/../bar/../xx/test2`: "node has invalid name", + ErrorsMay: map[string]map[string]struct{}{ + `/`: { + `invalid child node name ../test`: struct{}{}, + `invalid child node name ../../foo/../bar/../xx/test2`: struct{}{}, + }, }, }, { @@ -290,8 +294,10 @@ func TestRestorer(t *testing.T) { Files: map[string]string{ "top": "toplevel file", }, - ErrorsMust: map[string]string{ - `/x#..`: "node has invalid name", + ErrorsMust: map[string]map[string]struct{}{ + `/x`: { + `invalid child node name ..`: struct{}{}, + }, }, }, } @@ -329,11 +335,14 @@ func TestRestorer(t *testing.T) { return true, true } - errors := make(map[string]string) - res.Error = func(dir string, node *restic.Node, err error) error { - t.Logf("restore returned error for %q in dir %v: %v", node.Name, dir, err) - dir = toSlash(dir) - errors[dir+"#"+node.Name] = err.Error() + errors := make(map[string]map[string]struct{}) + res.Error = func(location string, err error) error { + location = toSlash(location) + t.Logf("restore returned error for %q: %v", location, err) + if errors[location] == nil { + errors[location] = make(map[string]struct{}) + } + errors[location][err.Error()] = struct{}{} return nil } @@ -345,33 +354,27 @@ func TestRestorer(t *testing.T) { t.Fatal(err) } - for filename, errorMessage := range test.ErrorsMust { - msg, ok := errors[filename] + for location, expectedErrors := range test.ErrorsMust { + actualErrors, ok := errors[location] if !ok { - t.Errorf("expected error for %v, found none", filename) + t.Errorf("expected error(s) for %v, found none", location) continue } - if msg != "" && msg != errorMessage { - t.Errorf("wrong error message for %v: got %q, want %q", - filename, msg, errorMessage) - } + rtest.Equals(t, expectedErrors, actualErrors) - delete(errors, filename) + delete(errors, location) } - for filename, errorMessage := range test.ErrorsMay { - msg, ok := errors[filename] + for location, expectedErrors := range test.ErrorsMay { + actualErrors, ok := errors[location] if !ok { continue } - if msg != "" && msg != errorMessage { - t.Errorf("wrong error message for %v: got %q, want %q", - filename, msg, errorMessage) - } + rtest.Equals(t, expectedErrors, actualErrors) - delete(errors, filename) + delete(errors, location) } for filename, err := range errors { @@ -436,10 +439,9 @@ func TestRestorerRelative(t *testing.T) { defer cleanup() errors := make(map[string]string) - res.Error = func(dir string, node *restic.Node, err error) error { - t.Logf("restore returned error for %q in dir %v: %v", node.Name, dir, err) - dir = toSlash(dir) - errors[dir+"#"+node.Name] = err.Error() + res.Error = func(location string, err error) error { + t.Logf("restore returned error for %q: %v", location, err) + errors[location] = err.Error() return nil }