From b25978a53c6a7e2dedaf3f63e2a10a41b029ebfc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 22 Apr 2020 22:23:02 +0200 Subject: [PATCH] backup: Fix reporting of directory count in summary Previously the directory stats were reported immediately after calling `SaveDir`. However, as the latter method saves the tree asynchronously the stats were still initialized to their nil value. The stats are now reported via a callback similar to the one used for the fileSaver. --- changelog/unreleased/issue-1863 | 6 ++++++ internal/archiver/archiver.go | 13 ++++++------ internal/archiver/archiver_test.go | 31 ++++++++++++++++++++++++++-- internal/archiver/tree_saver.go | 23 +++++++++++++-------- internal/archiver/tree_saver_test.go | 4 ++-- 5 files changed, 58 insertions(+), 19 deletions(-) create mode 100644 changelog/unreleased/issue-1863 diff --git a/changelog/unreleased/issue-1863 b/changelog/unreleased/issue-1863 new file mode 100644 index 000000000..0361de1f4 --- /dev/null +++ b/changelog/unreleased/issue-1863 @@ -0,0 +1,6 @@ +Bugfix: Report correct number of directories processed by backup + +The directory statistics calculation was fixed to report the actual number +of processed directories instead of always zero. + +https://github.com/restic/restic/issues/1863 diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 51ec94f15..f334a1af2 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -208,7 +208,7 @@ func (arch *Archiver) loadSubtree(ctx context.Context, node *restic.Node) *resti // SaveDir stores a directory in the repo and returns the node. snPath is the // path within the current snapshot. -func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo, dir string, previous *restic.Tree) (d FutureTree, err error) { +func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo, dir string, previous *restic.Tree, complete CompleteFunc) (d FutureTree, err error) { debug.Log("%v %v", snPath, dir) treeNode, err := arch.nodeFromFileInfo(dir, fi) @@ -254,7 +254,7 @@ func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo nodes = append(nodes, fn) } - ft := arch.treeSaver.Save(ctx, snPath, treeNode, nodes) + ft := arch.treeSaver.Save(ctx, snPath, treeNode, nodes, complete) return ft, nil } @@ -438,10 +438,11 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous oldSubtree := arch.loadSubtree(ctx, previous) fn.isTree = true - fn.tree, err = arch.SaveDir(ctx, snPath, fi, target, oldSubtree) - if err == nil { - arch.CompleteItem(snItem, previous, fn.node, fn.stats, time.Since(start)) - } else { + fn.tree, err = arch.SaveDir(ctx, snPath, fi, target, oldSubtree, + func(node *restic.Node, stats ItemStats) { + arch.CompleteItem(snItem, previous, node, stats, time.Since(start)) + }) + if err != nil { debug.Log("SaveDir for %v returned error: %v", snPath, err) return FutureNode{}, false, err } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 76c814bc3..d2e9c175b 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -811,7 +811,7 @@ func TestArchiverSaveDir(t *testing.T) { t.Fatal(err) } - ft, err := arch.SaveDir(ctx, "/", fi, test.target, nil) + ft, err := arch.SaveDir(ctx, "/", fi, test.target, nil, nil) if err != nil { t.Fatal(err) } @@ -888,7 +888,7 @@ func TestArchiverSaveDirIncremental(t *testing.T) { t.Fatal(err) } - ft, err := arch.SaveDir(ctx, "/", fi, tempdir, nil) + ft, err := arch.SaveDir(ctx, "/", fi, tempdir, nil, nil) if err != nil { t.Fatal(err) } @@ -947,6 +947,14 @@ func TestArchiverSaveDirIncremental(t *testing.T) { } } +// bothZeroOrNeither fails the test if only one of exp, act is zero. +func bothZeroOrNeither(tb testing.TB, exp, act uint64) { + if (exp == 0 && act != 0) || (exp != 0 && act == 0) { + _, file, line, _ := runtime.Caller(1) + tb.Fatalf("\033[31m%s:%d:\n\n\texp: %#v\n\n\tgot: %#v\033[39m\n\n", filepath.Base(file), line, exp, act) + } +} + func TestArchiverSaveTree(t *testing.T) { symlink := func(from, to string) func(t testing.TB) { return func(t testing.TB) { @@ -957,11 +965,13 @@ func TestArchiverSaveTree(t *testing.T) { } } + // The toplevel directory is not counted in the ItemStats var tests = []struct { src TestDir prepare func(t testing.TB) targets []string want TestDir + stat ItemStats }{ { src: TestDir{ @@ -971,6 +981,7 @@ func TestArchiverSaveTree(t *testing.T) { want: TestDir{ "targetfile": TestFile{Content: string("foobar")}, }, + stat: ItemStats{1, 6, 0, 0}, }, { src: TestDir{ @@ -982,6 +993,7 @@ func TestArchiverSaveTree(t *testing.T) { "targetfile": TestFile{Content: string("foobar")}, "filesymlink": TestSymlink{Target: "targetfile"}, }, + stat: ItemStats{1, 6, 0, 0}, }, { src: TestDir{ @@ -1001,6 +1013,7 @@ func TestArchiverSaveTree(t *testing.T) { "symlink": TestSymlink{Target: "subdir"}, }, }, + stat: ItemStats{0, 0, 1, 0x154}, }, { src: TestDir{ @@ -1024,6 +1037,7 @@ func TestArchiverSaveTree(t *testing.T) { }, }, }, + stat: ItemStats{1, 6, 3, 0x47f}, }, } @@ -1038,6 +1052,15 @@ func TestArchiverSaveTree(t *testing.T) { testFS := fs.Track{FS: fs.Local{}} arch := New(repo, testFS, Options{}) + + var stat ItemStats + lock := &sync.Mutex{} + arch.CompleteItem = func(item string, previous, current *restic.Node, s ItemStats, d time.Duration) { + lock.Lock() + defer lock.Unlock() + stat.Add(s) + } + arch.runWorkers(ctx, &tmb) back := fs.TestChdir(t, tempdir) @@ -1078,6 +1101,10 @@ func TestArchiverSaveTree(t *testing.T) { want = test.src } TestEnsureTree(ctx, t, "/", repo, treeID, want) + bothZeroOrNeither(t, uint64(test.stat.DataBlobs), uint64(stat.DataBlobs)) + bothZeroOrNeither(t, uint64(test.stat.TreeBlobs), uint64(stat.TreeBlobs)) + bothZeroOrNeither(t, test.stat.DataSize, stat.DataSize) + bothZeroOrNeither(t, test.stat.TreeSize, stat.TreeSize) }) } } diff --git a/internal/archiver/tree_saver.go b/internal/archiver/tree_saver.go index a956f4a12..c41392643 100644 --- a/internal/archiver/tree_saver.go +++ b/internal/archiver/tree_saver.go @@ -66,13 +66,14 @@ func NewTreeSaver(ctx context.Context, t *tomb.Tomb, treeWorkers uint, saveTree } // Save stores the dir d and returns the data once it has been completed. -func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node, nodes []FutureNode) FutureTree { +func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node, nodes []FutureNode, complete CompleteFunc) FutureTree { ch := make(chan saveTreeResponse, 1) job := saveTreeJob{ - snPath: snPath, - node: node, - nodes: nodes, - ch: ch, + snPath: snPath, + node: node, + nodes: nodes, + ch: ch, + complete: complete, } select { case s.ch <- job: @@ -86,10 +87,11 @@ func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node, } type saveTreeJob struct { - snPath string - nodes []FutureNode - node *restic.Node - ch chan<- saveTreeResponse + snPath string + nodes []FutureNode + node *restic.Node + ch chan<- saveTreeResponse + complete CompleteFunc } type saveTreeResponse struct { @@ -156,6 +158,9 @@ func (s *TreeSaver) worker(ctx context.Context, jobs <-chan saveTreeJob) error { return err } + if job.complete != nil { + job.complete(node, stats) + } job.ch <- saveTreeResponse{ node: node, stats: stats, diff --git a/internal/archiver/tree_saver_test.go b/internal/archiver/tree_saver_test.go index bc8c2612e..c9c589d1c 100644 --- a/internal/archiver/tree_saver_test.go +++ b/internal/archiver/tree_saver_test.go @@ -36,7 +36,7 @@ func TestTreeSaver(t *testing.T) { Name: fmt.Sprintf("file-%d", i), } - fb := b.Save(ctx, "/", node, nil) + fb := b.Save(ctx, "/", node, nil, nil) results = append(results, fb) } @@ -97,7 +97,7 @@ func TestTreeSaverError(t *testing.T) { Name: fmt.Sprintf("file-%d", i), } - fb := b.Save(ctx, "/", node, nil) + fb := b.Save(ctx, "/", node, nil, nil) results = append(results, fb) }