From 1140950d7b3cddb6a73680f02fe915982edb193e Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 8 Sep 2018 18:35:49 +0200 Subject: [PATCH 1/2] scanner: Use context only for cancellation When the scanner is slower than the actual backup, the tomb cancels the context passed to Scan(), which then returns ctx.Err(). In the end, the main function prints an error message that is not helpful ("Context cancelled") and exits with an error code although no error occurred. The code now ignores the error in the context and just uses it for cancellation. The scanner is not supposed to return an error anyway. Closes #1978 --- internal/archiver/scanner.go | 14 ++------------ internal/archiver/scanner_test.go | 10 +++------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/internal/archiver/scanner.go b/internal/archiver/scanner.go index ba3d5c60b..bd789893c 100644 --- a/internal/archiver/scanner.go +++ b/internal/archiver/scanner.go @@ -52,20 +52,17 @@ func (s *Scanner) Scan(ctx context.Context, targets []string) error { } if ctx.Err() != nil { - return ctx.Err() + return nil } } - if ctx.Err() != nil { - return ctx.Err() - } s.Result("", stats) return nil } func (s *Scanner) scan(ctx context.Context, stats ScanStats, target string) (ScanStats, error) { if ctx.Err() != nil { - return stats, ctx.Err() + return stats, nil } // exclude files by path before running stat to reduce number of lstat calls @@ -89,10 +86,6 @@ func (s *Scanner) scan(ctx context.Context, stats ScanStats, target string) (Sca stats.Files++ stats.Bytes += uint64(fi.Size()) case fi.Mode().IsDir(): - if ctx.Err() != nil { - return stats, ctx.Err() - } - names, err := readdirnames(s.FS, target) if err != nil { return stats, s.Error(target, fi, err) @@ -109,9 +102,6 @@ func (s *Scanner) scan(ctx context.Context, stats ScanStats, target string) (Sca stats.Others++ } - if ctx.Err() != nil { - return stats, ctx.Err() - } s.Result(target, stats) return stats, nil } diff --git a/internal/archiver/scanner_test.go b/internal/archiver/scanner_test.go index f02546f3c..a171df5f6 100644 --- a/internal/archiver/scanner_test.go +++ b/internal/archiver/scanner_test.go @@ -289,7 +289,7 @@ func TestScannerCancel(t *testing.T) { "other": TestFile{Content: "other"}, } - result := ScanStats{Files: 2, Bytes: 6} + result := ScanStats{Files: 2, Dirs: 1, Bytes: 6} ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -319,12 +319,8 @@ func TestScannerCancel(t *testing.T) { } err = sc.Scan(ctx, []string{"."}) - if err == nil { - t.Errorf("did not find expected error") - } - - if err != context.Canceled { - t.Errorf("unexpected error found, want %v, got %v", context.Canceled, err) + if err != nil { + t.Errorf("unexpected error %v found", err) } if lastStats != result { From e2d9900d826fb865c3d0af2ba8b386e36a22f4cc Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 8 Sep 2018 18:41:43 +0200 Subject: [PATCH 2/2] Add entry to CHANGELOG --- changelog/unreleased/issue-1978 | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog/unreleased/issue-1978 diff --git a/changelog/unreleased/issue-1978 b/changelog/unreleased/issue-1978 new file mode 100644 index 000000000..b3f5101ba --- /dev/null +++ b/changelog/unreleased/issue-1978 @@ -0,0 +1,12 @@ +Bugfix: Do not return an error when the scanner is faster than backup + +When restic makes a backup, there's a background task called "scanner" which +collects information on how many files and directories are to be saved, in +order to display progress information to the user. When the backup finishes +faster than the scanner, it is aborted because the result is not needed any +more. This logic contained a bug, where quitting the scanner process was +treated as an error, and caused restic to print an unhelpful error message +("context canceled"). + +https://github.com/restic/restic/issues/1978 +https://github.com/restic/restic/pull/1991