mirror/restic
1
0
Fork 0
mirror of https://github.com/restic/restic.git synced 2024-12-23 08:16:36 +00:00
Code Issues Releases Wiki Activity

retry: do not retry permanent errors

Browse source 
This is currently gated behind a feature flag as some unexpected
interactions might show up in the wild.
This commit is contained in:
Michael Eischer 2024-05-11 20:25:04 +02:00
parent bf8cc59889
commit aeb7eb245c
2 changed files with 45 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
internal/backend/retry/backend_retry.go
View file

@ -2,6 +2,7 @@ package retry
import (
"context"
"errors"
"fmt"
"io"
"time"
@ -9,6 +10,7 @@ import (
"github.com/cenkalti/backoff/v4"
"github.com/restic/restic/internal/backend"
"github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/feature"
)
// Backend retries operations on the backend in case of an error with a
@ -74,7 +76,16 @@ func (be *Backend) retry(ctx context.Context, msg string, f func() error) error
bo.InitialInterval = 1 * time.Millisecond
}
err := retryNotifyErrorWithSuccess(f,
err := retryNotifyErrorWithSuccess(
func() error {
err := f()
// don't retry permanent errors as those very likely cannot be fixed by retrying
// TODO remove IsNotExist(err) special cases when removing the feature flag
if feature.Flag.Enabled(feature.BackendErrorRedesign) && !errors.Is(err, &backoff.PermanentError{}) && be.Backend.IsPermanentError(err) {
return backoff.Permanent(err)
}
return err
},
backoff.WithContext(backoff.WithMaxRetries(bo, uint64(be.MaxTries)), ctx),
func(err error, d time.Duration) {
if be.Report != nil {
@ -128,11 +139,7 @@ func (be *Backend) Save(ctx context.Context, h backend.Handle, rd backend.Rewind
func (be *Backend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) (err error) {
return be.retry(ctx, fmt.Sprintf("Load(%v, %v, %v)", h, length, offset),
func() error {
err := be.Backend.Load(ctx, h, length, offset, consumer)
if be.Backend.IsNotExist(err) {
return backoff.Permanent(err)
}
return err
return be.Backend.Load(ctx, h, length, offset, consumer)
})
}

34
internal/backend/retry/backend_retry_test.go
View file

@ -289,7 +289,7 @@ func TestBackendLoadNotExists(t *testing.T) {
}
return nil, notFound
}
be.IsNotExistFn = func(err error) bool {
be.IsPermanentErrorFn = func(err error) bool {
return errors.Is(err, notFound)
}
@ -299,7 +299,7 @@ func TestBackendLoadNotExists(t *testing.T) {
err := retryBackend.Load(context.TODO(), backend.Handle{}, 0, 0, func(rd io.Reader) (err error) {
return nil
})
test.Assert(t, be.IsNotExistFn(err), "unexpected error %v", err)
test.Assert(t, be.IsPermanentErrorFn(err), "unexpected error %v", err)
test.Equals(t, 1, attempt)
}
@ -329,6 +329,36 @@ func TestBackendStatNotExists(t *testing.T) {
test.Equals(t, 1, attempt)
}
func TestBackendRetryPermanent(t *testing.T) {
// retry should not retry if the error matches IsPermanentError
notFound := errors.New("not found")
attempt := 0
be := mock.NewBackend()
be.IsPermanentErrorFn = func(err error) bool {
return errors.Is(err, notFound)
}
TestFastRetries(t)
retryBackend := New(be, 2, nil, nil)
err := retryBackend.retry(context.TODO(), "test", func() error {
attempt++
return notFound
})
test.Assert(t, be.IsPermanentErrorFn(err), "unexpected error %v", err)
test.Equals(t, 1, attempt)
attempt = 0
err = retryBackend.retry(context.TODO(), "test", func() error {
attempt++
return errors.New("something")
})
test.Assert(t, !be.IsPermanentErrorFn(err), "error unexpectedly considered permanent %v", err)
test.Equals(t, 3, attempt)
}
func assertIsCanceled(t *testing.T, err error) {
test.Assert(t, err == context.Canceled, "got unexpected err %v", err)
}