diff --git a/changelog/unreleased/issue-5081 b/changelog/unreleased/issue-5081 new file mode 100644 index 000000000..6cf1bf592 --- /dev/null +++ b/changelog/unreleased/issue-5081 @@ -0,0 +1,7 @@ +Enhancement: Retry loading repository config + +Restic now retries loading the repository config file when opening a repository. +In addition, the `init` command now also retries backend operations. + +https://github.com/restic/restic/issues/5081 +https://github.com/restic/restic/pull/5095 diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 36f6e9b0c..fc52882d7 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -439,26 +439,6 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi return nil, err } - report := func(msg string, err error, d time.Duration) { - if d >= 0 { - Warnf("%v returned error, retrying after %v: %v\n", msg, d, err) - } else { - Warnf("%v failed: %v\n", msg, err) - } - } - success := func(msg string, retries int) { - Warnf("%v operation successful after %d retries\n", msg, retries) - } - be = retry.New(be, 15*time.Minute, report, success) - - // wrap backend if a test specified a hook - if opts.backendTestHook != nil { - be, err = opts.backendTestHook(be) - if err != nil { - return nil, err - } - } - s, err := repository.New(be, repository.Options{ Compression: opts.Compression, PackSize: opts.PackSize * 1024 * 1024, @@ -629,12 +609,31 @@ func innerOpen(ctx context.Context, s string, gopts GlobalOptions, opts options. } } + report := func(msg string, err error, d time.Duration) { + if d >= 0 { + Warnf("%v returned error, retrying after %v: %v\n", msg, d, err) + } else { + Warnf("%v failed: %v\n", msg, err) + } + } + success := func(msg string, retries int) { + Warnf("%v operation successful after %d retries\n", msg, retries) + } + be = retry.New(be, 15*time.Minute, report, success) + + // wrap backend if a test specified a hook + if gopts.backendTestHook != nil { + be, err = gopts.backendTestHook(be) + if err != nil { + return nil, err + } + } + return be, nil } // Open the backend specified by a location config. func open(ctx context.Context, s string, gopts GlobalOptions, opts options.Options) (backend.Backend, error) { - be, err := innerOpen(ctx, s, gopts, opts, false) if err != nil { return nil, err diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index df95031dc..cb4ccba41 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -177,3 +177,47 @@ func TestFindListOnce(t *testing.T) { // the snapshots can only be listed once, if both lists match then the there has been only a single List() call rtest.Equals(t, thirdSnapshot, snapshotIDs) } + +type failConfigOnceBackend struct { + backend.Backend + failedOnce bool +} + +func (be *failConfigOnceBackend) Load(ctx context.Context, h backend.Handle, + length int, offset int64, fn func(rd io.Reader) error) error { + + if !be.failedOnce && h.Type == restic.ConfigFile { + be.failedOnce = true + return fmt.Errorf("oops") + } + return be.Backend.Load(ctx, h, length, offset, fn) +} + +func (be *failConfigOnceBackend) Stat(ctx context.Context, h backend.Handle) (backend.FileInfo, error) { + if !be.failedOnce && h.Type == restic.ConfigFile { + be.failedOnce = true + return backend.FileInfo{}, fmt.Errorf("oops") + } + return be.Backend.Stat(ctx, h) +} + +func TestBackendRetryConfig(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + var wrappedBackend *failConfigOnceBackend + // cause config loading to fail once + env.gopts.backendInnerTestHook = func(r backend.Backend) (backend.Backend, error) { + wrappedBackend = &failConfigOnceBackend{Backend: r} + return wrappedBackend, nil + } + + testSetupBackupData(t, env) + rtest.Assert(t, wrappedBackend != nil, "backend not wrapped on init") + rtest.Assert(t, wrappedBackend != nil && wrappedBackend.failedOnce, "config loading was not retried on init") + wrappedBackend = nil + + testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9")}, BackupOptions{}, env.gopts) + rtest.Assert(t, wrappedBackend != nil, "backend not wrapped on backup") + rtest.Assert(t, wrappedBackend != nil && wrappedBackend.failedOnce, "config loading was not retried on init") +} diff --git a/internal/backend/retry/backend_retry.go b/internal/backend/retry/backend_retry.go index 92c285c4b..de8a520ec 100644 --- a/internal/backend/retry/backend_retry.go +++ b/internal/backend/retry/backend_retry.go @@ -221,12 +221,19 @@ func (be *Backend) Load(ctx context.Context, h backend.Handle, length int, offse // Stat returns information about the File identified by h. func (be *Backend) Stat(ctx context.Context, h backend.Handle) (fi backend.FileInfo, err error) { - err = be.retry(ctx, fmt.Sprintf("Stat(%v)", h), + // see the call to `cancel()` below for why this context exists + statCtx, cancel := context.WithCancel(ctx) + defer cancel() + + err = be.retry(statCtx, fmt.Sprintf("Stat(%v)", h), func() error { var innerError error fi, innerError = be.Backend.Stat(ctx, h) if be.Backend.IsNotExist(innerError) { + // stat is only used to check the existence of the config file. + // cancel the context to suppress the final error message if the file is not found. + cancel() // do not retry if file is not found, as stat is usually used to check whether a file exists return backoff.Permanent(innerError) } diff --git a/internal/backend/retry/backend_retry_test.go b/internal/backend/retry/backend_retry_test.go index ffb8ae186..9259144d4 100644 --- a/internal/backend/retry/backend_retry_test.go +++ b/internal/backend/retry/backend_retry_test.go @@ -400,7 +400,11 @@ func TestBackendStatNotExists(t *testing.T) { } TestFastRetries(t) - retryBackend := New(be, 10, nil, nil) + retryBackend := New(be, 10, func(s string, err error, d time.Duration) { + t.Fatalf("unexpected error output %v", s) + }, func(s string, i int) { + t.Fatalf("unexpected log output %v", s) + }) _, err := retryBackend.Stat(context.TODO(), backend.Handle{}) test.Assert(t, be.IsNotExistFn(err), "unexpected error %v", err)