From 40611fc4fc26a0af13950a34eafd9b084213f3ec Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 12 Jun 2020 17:01:23 +0100 Subject: [PATCH] check: retry downloads if they fail when using the --download flag See: https://forum.rclone.org/t/tons-of-data-corruption-after-rclone-copy-to-mega-can-rclone-correct-it/17017/7 --- fs/operations/operations.go | 35 ++++++++++++++++++++++++++++---- fs/operations/operations_test.go | 25 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 8050e823e..1829de12c 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -966,11 +966,38 @@ func CheckEqualReaders(in1, in2 io.Reader) (differ bool, err error) { return false, nil } -// CheckIdentical checks to see if dst and src are identical by -// reading all their bytes if necessary. +// Retry runs fn up to maxTries times if it returns a retriable error +func Retry(o interface{}, maxTries int, fn func() error) (err error) { + for tries := 1; tries <= maxTries; tries++ { + // Call the function which might error + err = fn() + if err == nil { + break + } + // Retry if err returned a retry error + if fserrors.IsRetryError(err) || fserrors.ShouldRetry(err) { + fs.Debugf(o, "Received error: %v - low level retry %d/%d", err, tries, maxTries) + continue + } + break + } + return err +} + +// CheckIdenticalDownload checks to see if dst and src are identical +// by reading all their bytes if necessary. // // it returns true if differences were found -func CheckIdentical(ctx context.Context, dst, src fs.Object) (differ bool, err error) { +func CheckIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ bool, err error) { + err = Retry(src, fs.Config.LowLevelRetries, func() error { + differ, err = checkIdenticalDownload(ctx, dst, src) + return err + }) + return differ, err +} + +// Does the work for CheckIdenticalDownload +func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ bool, err error) { in1, err := dst.Open(ctx) if err != nil { return true, errors.Wrapf(err, "failed to open %q", dst) @@ -1000,7 +1027,7 @@ func CheckIdentical(ctx context.Context, dst, src fs.Object) (differ bool, err e // and the actual contents of the files. func CheckDownload(ctx context.Context, fdst, fsrc fs.Fs, oneway bool) error { check := func(ctx context.Context, a, b fs.Object) (differ bool, noHash bool) { - differ, err := CheckIdentical(ctx, a, b) + differ, err := CheckIdenticalDownload(ctx, a, b) if err != nil { err = fs.CountError(err) fs.Errorf(a, "Failed to download: %v", err) diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index 51713a829..05496e04a 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -329,6 +329,31 @@ func TestDelete(t *testing.T) { fstest.CheckItems(t, r.Fremote, file3) } +func TestRetry(t *testing.T) { + var i int + var err error + fn := func() error { + i-- + if i <= 0 { + return nil + } + return err + } + + i, err = 3, io.EOF + assert.Equal(t, nil, operations.Retry(nil, 5, fn)) + assert.Equal(t, 0, i) + + i, err = 10, io.EOF + assert.Equal(t, io.EOF, operations.Retry(nil, 5, fn)) + assert.Equal(t, 5, i) + + i, err = 10, fs.ErrorObjectNotFound + assert.Equal(t, fs.ErrorObjectNotFound, operations.Retry(nil, 5, fn)) + assert.Equal(t, 9, i) + +} + func testCheck(t *testing.T, checkFunction func(ctx context.Context, fdst, fsrc fs.Fs, oneway bool) error) { r := fstest.NewRun(t) defer r.Finalise()