Merge pull request #5095 from MichaelEischer/retry-load-config

Retry loading or creating repository config

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

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

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")
+}

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)
 			}

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)