From 74c783b850db9036f984a2fda82febe4669b9334 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 17 Oct 2024 20:21:54 +0200 Subject: [PATCH 1/4] retry load or creating repository config By now missing files are not endlessly retried by the retry backend such that it can be enabled right from the start. In addition, this change also enables the retry backend for the `init` command. --- cmd/restic/global.go | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) 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 From 58dc4a6892e80c2768c82961af1cd6656fcd01d0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 17 Oct 2024 20:35:26 +0200 Subject: [PATCH 2/4] backend/retry: hide final log for `stat()` method stat is only used to check the config file's existence. We don't want log output in this case. --- internal/backend/retry/backend_retry.go | 9 ++++++++- internal/backend/retry/backend_retry_test.go | 6 +++++- 2 files changed, 13 insertions(+), 2 deletions(-) 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) From b5bc76cdc77931d616909ad6531f07192682e200 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 1 Nov 2024 15:16:24 +0100 Subject: [PATCH 3/4] test retry on repo opening --- cmd/restic/integration_test.go | 44 ++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) 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") +} From 3be2b8a54b48ba855a49d99286c546d8486c0502 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 1 Nov 2024 15:22:55 +0100 Subject: [PATCH 4/4] add config retry changelog --- changelog/unreleased/issue-5081 | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/issue-5081 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