From a59f654fa68dfe5b1a186b1fb81265ba8268c623 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 22 Feb 2024 22:14:48 +0100 Subject: [PATCH] archiver: refactor summary collection from ui into the archiver --- cmd/restic/cmd_backup.go | 4 +- internal/archiver/archiver.go | 79 ++++++++++++++++++++++++----- internal/archiver/archiver_test.go | 21 +++++--- internal/archiver/testing.go | 2 +- internal/archiver/testing_test.go | 2 +- internal/dump/common_test.go | 2 +- internal/restorer/restorer_test.go | 2 +- internal/ui/backup/json.go | 2 +- internal/ui/backup/progress.go | 49 ++---------------- internal/ui/backup/progress_test.go | 5 +- internal/ui/backup/text.go | 2 +- 11 files changed, 92 insertions(+), 78 deletions(-) diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index 318d17796..4c03b7e8c 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -649,7 +649,7 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter if !gopts.JSON { progressPrinter.V("start backup on %v", targets) } - _, id, err := arch.Snapshot(ctx, targets, snapshotOpts) + _, id, summary, err := arch.Snapshot(ctx, targets, snapshotOpts) // cleanly shutdown all running goroutines cancel() @@ -663,7 +663,7 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter } // Report finished execution - progressReporter.Finish(id, opts.DryRun) + progressReporter.Finish(id, summary, opts.DryRun) if !gopts.JSON && !opts.DryRun { progressPrinter.P("snapshot %s saved\n", id.Str()) } diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 9d7d68913..b8d1d45bb 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -8,6 +8,7 @@ import ( "runtime" "sort" "strings" + "sync" "time" "github.com/restic/restic/internal/debug" @@ -40,6 +41,16 @@ type ItemStats struct { TreeSizeInRepo uint64 // sum of the bytes added to the repo (including compression and crypto overhead) } +type Summary struct { + Files, Dirs struct { + New uint + Changed uint + Unchanged uint + } + ProcessedBytes uint64 + ItemStats +} + // Add adds other to the current ItemStats. func (s *ItemStats) Add(other ItemStats) { s.DataBlobs += other.DataBlobs @@ -61,6 +72,8 @@ type Archiver struct { blobSaver *BlobSaver fileSaver *FileSaver treeSaver *TreeSaver + mu sync.Mutex + summary *Summary // Error is called for all errors that occur during backup. Error ErrorFunc @@ -182,6 +195,44 @@ func (arch *Archiver) error(item string, err error) error { return errf } +func (arch *Archiver) trackItem(item string, previous, current *restic.Node, s ItemStats, d time.Duration) { + arch.CompleteItem(item, previous, current, s, d) + + arch.mu.Lock() + defer arch.mu.Unlock() + + arch.summary.ItemStats.Add(s) + + if current != nil { + arch.summary.ProcessedBytes += current.Size + } else { + // last item or an error occurred + return + } + + switch current.Type { + case "dir": + switch { + case previous == nil: + arch.summary.Dirs.New++ + case previous.Equals(*current): + arch.summary.Dirs.Unchanged++ + default: + arch.summary.Dirs.Changed++ + } + + case "file": + switch { + case previous == nil: + arch.summary.Files.New++ + case previous.Equals(*current): + arch.summary.Files.Unchanged++ + default: + arch.summary.Files.Changed++ + } + } +} + // nodeFromFileInfo returns the restic node from an os.FileInfo. func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo) (*restic.Node, error) { node, err := restic.NodeFromFileInfo(filename, fi) @@ -380,7 +431,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous if previous != nil && !fileChanged(fi, previous, arch.ChangeIgnoreFlags) { if arch.allBlobsPresent(previous) { debug.Log("%v hasn't changed, using old list of blobs", target) - arch.CompleteItem(snPath, previous, previous, ItemStats{}, time.Since(start)) + arch.trackItem(snPath, previous, previous, ItemStats{}, time.Since(start)) arch.CompleteBlob(previous.Size) node, err := arch.nodeFromFileInfo(snPath, target, fi) if err != nil { @@ -445,9 +496,9 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous fn = arch.fileSaver.Save(ctx, snPath, target, file, fi, func() { arch.StartFile(snPath) }, func() { - arch.CompleteItem(snPath, nil, nil, ItemStats{}, 0) + arch.trackItem(snPath, nil, nil, ItemStats{}, 0) }, func(node *restic.Node, stats ItemStats) { - arch.CompleteItem(snPath, previous, node, stats, time.Since(start)) + arch.trackItem(snPath, previous, node, stats, time.Since(start)) }) case fi.IsDir(): @@ -464,7 +515,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous fn, err = arch.saveDir(ctx, snPath, target, fi, oldSubtree, func(node *restic.Node, stats ItemStats) { - arch.CompleteItem(snItem, previous, node, stats, time.Since(start)) + arch.trackItem(snItem, previous, node, stats, time.Since(start)) }) if err != nil { debug.Log("SaveDir for %v returned error: %v", snPath, err) @@ -620,7 +671,7 @@ func (arch *Archiver) saveTree(ctx context.Context, snPath string, atree *Tree, // not a leaf node, archive subtree fn, _, err := arch.saveTree(ctx, join(snPath, name), &subatree, oldSubtree, func(n *restic.Node, is ItemStats) { - arch.CompleteItem(snItem, oldNode, n, is, time.Since(start)) + arch.trackItem(snItem, oldNode, n, is, time.Since(start)) }) if err != nil { return FutureNode{}, 0, err @@ -738,15 +789,17 @@ func (arch *Archiver) stopWorkers() { } // Snapshot saves several targets and returns a snapshot. -func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts SnapshotOptions) (*restic.Snapshot, restic.ID, error) { +func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts SnapshotOptions) (*restic.Snapshot, restic.ID, *Summary, error) { + arch.summary = &Summary{} + cleanTargets, err := resolveRelativeTargets(arch.FS, targets) if err != nil { - return nil, restic.ID{}, err + return nil, restic.ID{}, nil, err } atree, err := NewTree(arch.FS, cleanTargets) if err != nil { - return nil, restic.ID{}, err + return nil, restic.ID{}, nil, err } var rootTreeID restic.ID @@ -763,7 +816,7 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps debug.Log("starting snapshot") fn, nodeCount, err := arch.saveTree(wgCtx, "/", atree, arch.loadParentTree(wgCtx, opts.ParentSnapshot), func(_ *restic.Node, is ItemStats) { - arch.CompleteItem("/", nil, nil, is, time.Since(start)) + arch.trackItem("/", nil, nil, is, time.Since(start)) }) if err != nil { return err @@ -799,12 +852,12 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps }) err = wgUp.Wait() if err != nil { - return nil, restic.ID{}, err + return nil, restic.ID{}, nil, err } sn, err := restic.NewSnapshot(targets, opts.Tags, opts.Hostname, opts.Time) if err != nil { - return nil, restic.ID{}, err + return nil, restic.ID{}, nil, err } sn.ProgramVersion = opts.ProgramVersion @@ -816,8 +869,8 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps id, err := restic.SaveSnapshot(ctx, arch.Repo, sn) if err != nil { - return nil, restic.ID{}, err + return nil, restic.ID{}, nil, err } - return sn, id, nil + return sn, id, arch.summary, nil } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 158768323..0ae7ca05f 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -226,6 +226,7 @@ func TestArchiverSave(t *testing.T) { return err } arch.runWorkers(ctx, wg) + arch.summary = &Summary{} node, excluded, err := arch.save(ctx, "/", filepath.Join(tempdir, "file"), nil) if err != nil { @@ -303,6 +304,7 @@ func TestArchiverSaveReaderFS(t *testing.T) { return err } arch.runWorkers(ctx, wg) + arch.summary = &Summary{} node, excluded, err := arch.save(ctx, "/", filename, nil) t.Logf("Save returned %v %v", node, err) @@ -831,6 +833,7 @@ func TestArchiverSaveDir(t *testing.T) { arch := New(repo, fs.Track{FS: fs.Local{}}, Options{}) arch.runWorkers(ctx, wg) + arch.summary = &Summary{} chdir := tempdir if test.chdir != "" { @@ -912,6 +915,7 @@ func TestArchiverSaveDirIncremental(t *testing.T) { arch := New(repo, fs.Track{FS: fs.Local{}}, Options{}) arch.runWorkers(ctx, wg) + arch.summary = &Summary{} fi, err := fs.Lstat(tempdir) if err != nil { @@ -1094,6 +1098,7 @@ func TestArchiverSaveTree(t *testing.T) { repo.StartPackUploader(ctx, wg) arch.runWorkers(ctx, wg) + arch.summary = &Summary{} back := restictest.Chdir(t, tempdir) defer back() @@ -1395,7 +1400,7 @@ func TestArchiverSnapshot(t *testing.T) { } t.Logf("targets: %v", targets) - sn, snapshotID, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()}) + sn, snapshotID, _, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()}) if err != nil { t.Fatal(err) } @@ -1543,7 +1548,7 @@ func TestArchiverSnapshotSelect(t *testing.T) { defer back() targets := []string{"."} - _, snapshotID, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()}) + _, snapshotID, _, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()}) if test.err != "" { if err == nil { t.Fatalf("expected error not found, got %v, wanted %q", err, test.err) @@ -1648,7 +1653,7 @@ func TestArchiverParent(t *testing.T) { back := restictest.Chdir(t, tempdir) defer back() - firstSnapshot, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) + firstSnapshot, firstSnapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) if err != nil { t.Fatal(err) } @@ -1678,7 +1683,7 @@ func TestArchiverParent(t *testing.T) { Time: time.Now(), ParentSnapshot: firstSnapshot, } - _, secondSnapshotID, err := arch.Snapshot(ctx, []string{"."}, opts) + _, secondSnapshotID, _, err := arch.Snapshot(ctx, []string{"."}, opts) if err != nil { t.Fatal(err) } @@ -1814,7 +1819,7 @@ func TestArchiverErrorReporting(t *testing.T) { arch := New(repo, fs.Track{FS: fs.Local{}}, Options{}) arch.Error = test.errFn - _, snapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) + _, snapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) if test.mustError { if err != nil { t.Logf("found expected error (%v), skipping further checks", err) @@ -1887,7 +1892,7 @@ func TestArchiverContextCanceled(t *testing.T) { arch := New(repo, fs.Track{FS: fs.Local{}}, Options{}) - _, snapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) + _, snapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) if err != nil { t.Logf("found expected error (%v)", err) @@ -2026,7 +2031,7 @@ func TestArchiverAbortEarlyOnError(t *testing.T) { SaveBlobConcurrency: 1, }) - _, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) + _, _, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) if !errors.Is(err, test.err) { t.Errorf("expected error (%v) not found, got %v", test.err, err) } @@ -2054,7 +2059,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent *restic.Sna Time: time.Now(), ParentSnapshot: parent, } - snapshot, _, err := arch.Snapshot(ctx, []string{filename}, sopts) + snapshot, _, _, err := arch.Snapshot(ctx, []string{filename}, sopts) if err != nil { t.Fatal(err) } diff --git a/internal/archiver/testing.go b/internal/archiver/testing.go index 111c1e68c..1feaa82fc 100644 --- a/internal/archiver/testing.go +++ b/internal/archiver/testing.go @@ -31,7 +31,7 @@ func TestSnapshot(t testing.TB, repo restic.Repository, path string, parent *res } opts.ParentSnapshot = sn } - sn, _, err := arch.Snapshot(context.TODO(), []string{path}, opts) + sn, _, _, err := arch.Snapshot(context.TODO(), []string{path}, opts) if err != nil { t.Fatal(err) } diff --git a/internal/archiver/testing_test.go b/internal/archiver/testing_test.go index ada7261f1..e48b41ec7 100644 --- a/internal/archiver/testing_test.go +++ b/internal/archiver/testing_test.go @@ -473,7 +473,7 @@ func TestTestEnsureSnapshot(t *testing.T) { Hostname: "localhost", Tags: []string{"test"}, } - _, id, err := arch.Snapshot(ctx, []string{"."}, opts) + _, id, _, err := arch.Snapshot(ctx, []string{"."}, opts) if err != nil { t.Fatal(err) } diff --git a/internal/dump/common_test.go b/internal/dump/common_test.go index 3ee9112af..afd19df63 100644 --- a/internal/dump/common_test.go +++ b/internal/dump/common_test.go @@ -78,7 +78,7 @@ func WriteTest(t *testing.T, format string, cd CheckDump) { back := rtest.Chdir(t, tmpdir) defer back() - sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) + sn, _, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) rtest.OK(t, err) tree, err := restic.LoadTree(ctx, repo, *sn.Tree) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index c33214bc3..29f8920c5 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -840,7 +840,7 @@ func TestRestorerSparseFiles(t *testing.T) { rtest.OK(t, err) arch := archiver.New(repo, target, archiver.Options{}) - sn, _, err := arch.Snapshot(context.Background(), []string{"/zeros"}, + sn, _, _, err := arch.Snapshot(context.Background(), []string{"/zeros"}, archiver.SnapshotOptions{}) rtest.OK(t, err) diff --git a/internal/ui/backup/json.go b/internal/ui/backup/json.go index 10f0e91fa..3393a3d48 100644 --- a/internal/ui/backup/json.go +++ b/internal/ui/backup/json.go @@ -163,7 +163,7 @@ func (b *JSONProgress) ReportTotal(start time.Time, s archiver.ScanStats) { } // Finish prints the finishing messages. -func (b *JSONProgress) Finish(snapshotID restic.ID, start time.Time, summary *Summary, dryRun bool) { +func (b *JSONProgress) Finish(snapshotID restic.ID, start time.Time, summary *archiver.Summary, dryRun bool) { b.print(summaryOutput{ MessageType: "summary", FilesNew: summary.Files.New, diff --git a/internal/ui/backup/progress.go b/internal/ui/backup/progress.go index da0d401a3..1d494bf14 100644 --- a/internal/ui/backup/progress.go +++ b/internal/ui/backup/progress.go @@ -17,7 +17,7 @@ type ProgressPrinter interface { ScannerError(item string, err error) error CompleteItem(messageType string, item string, s archiver.ItemStats, d time.Duration) ReportTotal(start time.Time, s archiver.ScanStats) - Finish(snapshotID restic.ID, start time.Time, summary *Summary, dryRun bool) + Finish(snapshotID restic.ID, start time.Time, summary *archiver.Summary, dryRun bool) Reset() P(msg string, args ...interface{}) @@ -28,16 +28,6 @@ type Counter struct { Files, Dirs, Bytes uint64 } -type Summary struct { - Files, Dirs struct { - New uint - Changed uint - Unchanged uint - } - ProcessedBytes uint64 - archiver.ItemStats -} - // Progress reports progress for the `backup` command. type Progress struct { progress.Updater @@ -52,7 +42,6 @@ type Progress struct { processed, total Counter errors uint - summary Summary printer ProgressPrinter } @@ -126,16 +115,6 @@ func (p *Progress) CompleteBlob(bytes uint64) { // CompleteItem is the status callback function for the archiver when a // file/dir has been saved successfully. func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s archiver.ItemStats, d time.Duration) { - p.mu.Lock() - p.summary.ItemStats.Add(s) - - // for the last item "/", current is nil - if current != nil { - p.summary.ProcessedBytes += current.Size - } - - p.mu.Unlock() - if current == nil { // error occurred, tell the status display to remove the line p.mu.Lock() @@ -153,21 +132,10 @@ func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s a switch { case previous == nil: p.printer.CompleteItem("dir new", item, s, d) - p.mu.Lock() - p.summary.Dirs.New++ - p.mu.Unlock() - case previous.Equals(*current): p.printer.CompleteItem("dir unchanged", item, s, d) - p.mu.Lock() - p.summary.Dirs.Unchanged++ - p.mu.Unlock() - default: p.printer.CompleteItem("dir modified", item, s, d) - p.mu.Lock() - p.summary.Dirs.Changed++ - p.mu.Unlock() } case "file": @@ -179,21 +147,10 @@ func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s a switch { case previous == nil: p.printer.CompleteItem("file new", item, s, d) - p.mu.Lock() - p.summary.Files.New++ - p.mu.Unlock() - case previous.Equals(*current): p.printer.CompleteItem("file unchanged", item, s, d) - p.mu.Lock() - p.summary.Files.Unchanged++ - p.mu.Unlock() - default: p.printer.CompleteItem("file modified", item, s, d) - p.mu.Lock() - p.summary.Files.Changed++ - p.mu.Unlock() } } } @@ -213,8 +170,8 @@ func (p *Progress) ReportTotal(item string, s archiver.ScanStats) { } // Finish prints the finishing messages. -func (p *Progress) Finish(snapshotID restic.ID, dryrun bool) { +func (p *Progress) Finish(snapshotID restic.ID, summary *archiver.Summary, dryrun bool) { // wait for the status update goroutine to shut down p.Updater.Done() - p.printer.Finish(snapshotID, p.start, &p.summary, dryrun) + p.printer.Finish(snapshotID, p.start, summary, dryrun) } diff --git a/internal/ui/backup/progress_test.go b/internal/ui/backup/progress_test.go index 79a56c91e..6b242a0f3 100644 --- a/internal/ui/backup/progress_test.go +++ b/internal/ui/backup/progress_test.go @@ -33,11 +33,10 @@ func (p *mockPrinter) CompleteItem(messageType string, _ string, _ archiver.Item } func (p *mockPrinter) ReportTotal(_ time.Time, _ archiver.ScanStats) {} -func (p *mockPrinter) Finish(id restic.ID, _ time.Time, summary *Summary, _ bool) { +func (p *mockPrinter) Finish(id restic.ID, _ time.Time, _ *archiver.Summary, _ bool) { p.Lock() defer p.Unlock() - _ = *summary // Should not be nil. p.id = id } @@ -64,7 +63,7 @@ func TestProgress(t *testing.T) { time.Sleep(10 * time.Millisecond) id := restic.NewRandomID() - prog.Finish(id, false) + prog.Finish(id, nil, false) if !prnt.dirUnchanged { t.Error(`"dir unchanged" event not seen`) diff --git a/internal/ui/backup/text.go b/internal/ui/backup/text.go index 215982cd4..00d025e51 100644 --- a/internal/ui/backup/text.go +++ b/internal/ui/backup/text.go @@ -126,7 +126,7 @@ func (b *TextProgress) Reset() { } // Finish prints the finishing messages. -func (b *TextProgress) Finish(_ restic.ID, start time.Time, summary *Summary, dryRun bool) { +func (b *TextProgress) Finish(_ restic.ID, start time.Time, summary *archiver.Summary, dryRun bool) { b.P("\n") b.P("Files: %5d new, %5d changed, %5d unmodified\n", summary.Files.New, summary.Files.Changed, summary.Files.Unchanged) b.P("Dirs: %5d new, %5d changed, %5d unmodified\n", summary.Dirs.New, summary.Dirs.Changed, summary.Dirs.Unchanged)