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

backend/retry: return worker function error and abort

This is a bug fix: Before, when the worker function fn in List() of the
RetryBackend returned an error, the operation is retried with the next
file. This is not consistent with the documentation, the intention was
that when fn returns an error, this is passed on to the caller and the
List() operation is aborted. Only errors happening on the underlying
backend are retried.

The error leads to restic ignoring exclusive locks that are present in
the repo, so it may happen that a new backup is written which references
data that is going to be removed by a concurrently running `prune`
operation.

The bug was reported by a user here:
https://forum.restic.net/t/restic-backup-returns-0-exit-code-when-already-locked/484
This commit is contained in:
Alexander Neumann 2018-02-24 13:23:42 +01:00
parent dfd37afee2
commit 93210614f4
3 changed files with 130 additions and 5 deletions

View file

@ -125,16 +125,39 @@ func (be *RetryBackend) Test(ctx context.Context, h restic.Handle) (exists bool,
return exists, err
}
// List runs fn for each file in the backend which has the type t.
// List runs fn for each file in the backend which has the type t. When an
// error is returned by the underlying backend, the request is retried. When fn
// returns an error, the operation is aborted and the error is returned to the
// caller.
func (be *RetryBackend) List(ctx context.Context, t restic.FileType, fn func(restic.FileInfo) error) error {
listed := make(map[string]struct{})
return be.retry(ctx, fmt.Sprintf("List(%v)", t), func() error {
// create a new context that we can cancel when fn returns an error, so
// that listing is aborted
listCtx, cancel := context.WithCancel(ctx)
defer cancel()
listed := make(map[string]struct{}) // remember for which files we already ran fn
var innerErr error // remember when fn returned an error, so we can return that to the caller
err := be.retry(listCtx, fmt.Sprintf("List(%v)", t), func() error {
return be.Backend.List(ctx, t, func(fi restic.FileInfo) error {
if _, ok := listed[fi.Name]; ok {
return nil
}
listed[fi.Name] = struct{}{}
return fn(fi)
innerErr = fn(fi)
if innerErr != nil {
// if fn returned an error, listing is aborted, so we cancel the context
cancel()
}
return innerErr
})
})
// the error fn returned takes precedence
if innerErr != nil {
return innerErr
}
return err
}

View file

@ -124,6 +124,104 @@ func TestBackendListRetry(t *testing.T) {
test.Equals(t, []string{ID1, ID2}, listed) // assert no duplicate files
}
func TestBackendListRetryErrorFn(t *testing.T) {
var names = []string{"id1", "id2", "foo", "bar"}
be := &mock.Backend{
ListFn: func(ctx context.Context, tpe restic.FileType, fn func(restic.FileInfo) error) error {
t.Logf("List called for %v", tpe)
for _, name := range names {
err := fn(restic.FileInfo{Name: name})
if err != nil {
return err
}
}
return nil
},
}
retryBackend := RetryBackend{
Backend: be,
}
var ErrTest = errors.New("test error")
var listed []string
run := 0
err := retryBackend.List(context.TODO(), restic.DataFile, func(fi restic.FileInfo) error {
t.Logf("fn called for %v", fi.Name)
run++
// return an error for the third item in the list
if run == 3 {
t.Log("returning an error")
return ErrTest
}
listed = append(listed, fi.Name)
return nil
})
if err != ErrTest {
t.Fatalf("wrong error returned, want %v, got %v", ErrTest, err)
}
// processing should stop after the error was returned, so run should be 3
if run != 3 {
t.Fatalf("function was called %d times, wanted %v", run, 3)
}
test.Equals(t, []string{"id1", "id2"}, listed)
}
func TestBackendListRetryErrorBackend(t *testing.T) {
var names = []string{"id1", "id2", "foo", "bar"}
var ErrBackendTest = errors.New("test error")
retries := 0
be := &mock.Backend{
ListFn: func(ctx context.Context, tpe restic.FileType, fn func(restic.FileInfo) error) error {
t.Logf("List called for %v, retries %v", tpe, retries)
retries++
for i, name := range names {
if i == 2 {
return ErrBackendTest
}
err := fn(restic.FileInfo{Name: name})
if err != nil {
return err
}
}
return nil
},
}
const maxRetries = 2
retryBackend := RetryBackend{
MaxTries: maxRetries,
Backend: be,
}
var listed []string
err := retryBackend.List(context.TODO(), restic.DataFile, func(fi restic.FileInfo) error {
t.Logf("fn called for %v", fi.Name)
listed = append(listed, fi.Name)
return nil
})
if err != ErrBackendTest {
t.Fatalf("wrong error returned, want %v, got %v", ErrBackendTest, err)
}
if retries != maxRetries+1 {
t.Fatalf("List was called %d times, wanted %v", retries, maxRetries+1)
}
test.Equals(t, names[:2], listed)
}
// failingReader returns an error after reading limit number of bytes
type failingReader struct {
data []byte

View file

@ -44,7 +44,11 @@ type ErrAlreadyLocked struct {
}
func (e ErrAlreadyLocked) Error() string {
return fmt.Sprintf("repository is already locked by %v", e.otherLock)
s := ""
if e.otherLock.Exclusive {
s = "exclusively "
}
return fmt.Sprintf("repository is already locked %sby %v", s, e.otherLock)
}
// IsAlreadyLocked returns true iff err is an instance of ErrAlreadyLocked.