From f7784bddb388c8dcf84a5db21783c44930c7f555 Mon Sep 17 00:00:00 2001
From: greatroar <@>
Date: Wed, 16 Dec 2020 13:58:02 +0100
Subject: [PATCH 1/4] Don't retry when "no space left on device" in local
backend
Also adds relevant documentation to the restic.Backend interface.
---
internal/backend/local/local.go | 18 ++++++--
internal/backend/local/local_internal_test.go | 42 +++++++++++++++++++
internal/errors/errors.go | 28 ++++++-------
internal/restic/backend.go | 6 +++
4 files changed, 77 insertions(+), 17 deletions(-)
create mode 100644 internal/backend/local/local_internal_test.go
diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go
index 2a48230fc..4930264fd 100644
--- a/internal/backend/local/local.go
+++ b/internal/backend/local/local.go
@@ -13,6 +13,8 @@ import (
"github.com/restic/restic/internal/backend"
"github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/fs"
+
+ "github.com/cenkalti/backoff/v4"
)
// Local is a backend in a local directory.
@@ -80,7 +82,7 @@ func (b *Local) IsNotExist(err error) bool {
}
// Save stores data in the backend at the handle.
-func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
+func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) (err error) {
debug.Log("Save %v", h)
if err := h.Valid(); err != nil {
return err
@@ -88,8 +90,16 @@ func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReade
filename := b.Filename(h)
+ defer func() {
+ // Mark non-retriable errors as such (currently only
+ // "no space left on device").
+ if errors.Is(err, syscall.ENOSPC) {
+ err = backoff.Permanent(err)
+ }
+ }()
+
// create new file
- f, err := fs.OpenFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY, backend.Modes.File)
+ f, err := openFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY, backend.Modes.File)
if b.IsNotExist(err) {
debug.Log("error %v: creating dir", err)
@@ -100,7 +110,7 @@ func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReade
debug.Log("error creating dir %v: %v", filepath.Dir(filename), mkdirErr)
} else {
// try again
- f, err = fs.OpenFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY, backend.Modes.File)
+ f, err = openFile(filename, os.O_CREATE|os.O_EXCL|os.O_WRONLY, backend.Modes.File)
}
}
@@ -141,6 +151,8 @@ func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReade
return nil
}
+var openFile = fs.OpenFile // Overridden by test.
+
// Load runs fn with a reader that yields the contents of the file at h at the
// given offset.
func (b *Local) Load(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error {
diff --git a/internal/backend/local/local_internal_test.go b/internal/backend/local/local_internal_test.go
new file mode 100644
index 000000000..a67233903
--- /dev/null
+++ b/internal/backend/local/local_internal_test.go
@@ -0,0 +1,42 @@
+package local
+
+import (
+ "context"
+ "errors"
+ "os"
+ "syscall"
+ "testing"
+
+ "github.com/restic/restic/internal/restic"
+ rtest "github.com/restic/restic/internal/test"
+
+ "github.com/cenkalti/backoff/v4"
+)
+
+func TestNoSpacePermanent(t *testing.T) {
+ oldOpenFile := openFile
+ defer func() {
+ openFile = oldOpenFile
+ }()
+
+ openFile = func(name string, flags int, mode os.FileMode) (*os.File, error) {
+ // The actual error from os.OpenFile is *os.PathError.
+ // Other functions called inside Save may return *os.SyscallError.
+ return nil, os.NewSyscallError("open", syscall.ENOSPC)
+ }
+
+ dir, cleanup := rtest.TempDir(t)
+ defer cleanup()
+
+ be, err := Open(context.Background(), Config{Path: dir})
+ rtest.OK(t, err)
+ defer be.Close()
+
+ h := restic.Handle{Type: restic.ConfigFile}
+ err = be.Save(context.Background(), h, nil)
+ _, ok := err.(*backoff.PermanentError)
+ rtest.Assert(t, ok,
+ "error type should be backoff.PermanentError, got %T", err)
+ rtest.Assert(t, errors.Is(err, syscall.ENOSPC),
+ "could not recover original ENOSPC error")
+}
diff --git a/internal/errors/errors.go b/internal/errors/errors.go
index ffd3d615e..51a425299 100644
--- a/internal/errors/errors.go
+++ b/internal/errors/errors.go
@@ -3,6 +3,7 @@ package errors
import (
"net/url"
+ "github.com/cenkalti/backoff/v4"
"github.com/pkg/errors"
)
@@ -34,20 +35,19 @@ func Cause(err error) error {
}
for {
- // unwrap *url.Error
- if urlErr, ok := err.(*url.Error); ok {
- err = urlErr.Err
- continue
+ switch e := err.(type) {
+ case Causer: // github.com/pkg/errors
+ err = e.Cause()
+ case *backoff.PermanentError:
+ err = e.Err
+ case *url.Error:
+ err = e.Err
+ default:
+ return err
}
-
- // if err is a Causer, return the cause for this error.
- if c, ok := err.(Causer); ok {
- err = c.Cause()
- continue
- }
-
- break
}
-
- return err
}
+
+// Go 1.13-style error handling.
+
+func Is(x, y error) bool { return errors.Is(x, y) }
diff --git a/internal/restic/backend.go b/internal/restic/backend.go
index b2fd46b97..cda5c30c7 100644
--- a/internal/restic/backend.go
+++ b/internal/restic/backend.go
@@ -6,6 +6,12 @@ import (
)
// Backend is used to store and access data.
+//
+// Backend operations that return an error will be retried when a Backend is
+// wrapped in a RetryBackend. To prevent that from happening, the operations
+// should return a github.com/cenkalti/backoff/v4.PermanentError. Errors from
+// the context package need not be wrapped, as context cancellation is checked
+// separately by the retrying logic.
type Backend interface {
// Location returns a string that describes the type and location of the
// repository.
From 746dbda4130485957afa452988f58b718795f4da Mon Sep 17 00:00:00 2001
From: greatroar <@>
Date: Wed, 16 Dec 2020 16:00:21 +0100
Subject: [PATCH 2/4] Mark "ssh exited" errors in SFTP as permanent
---
internal/backend/sftp/sftp.go | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go
index 0a2c15e75..4a0c1695b 100644
--- a/internal/backend/sftp/sftp.go
+++ b/internal/backend/sftp/sftp.go
@@ -16,6 +16,7 @@ import (
"github.com/restic/restic/internal/backend"
"github.com/restic/restic/internal/debug"
+ "github.com/cenkalti/backoff/v4"
"github.com/pkg/sftp"
)
@@ -99,7 +100,7 @@ func (r *SFTP) clientError() error {
select {
case err := <-r.result:
debug.Log("client has exited with err %v", err)
- return err
+ return backoff.Permanent(err)
default:
}
From 66d904c90541795d59d415eb2dc008c8e396551e Mon Sep 17 00:00:00 2001
From: greatroar <@>
Date: Thu, 17 Dec 2020 12:47:53 +0100
Subject: [PATCH 3/4] Make invalid handles permanent errors
---
internal/backend/azure/azure.go | 8 +++++---
internal/backend/b2/b2.go | 5 +++--
internal/backend/local/local.go | 6 +++---
internal/backend/mem/mem_backend.go | 9 +++++----
internal/backend/rest/rest.go | 11 ++++++-----
internal/backend/s3/s3.go | 8 ++++----
internal/backend/sftp/sftp.go | 6 +++---
internal/backend/swift/swift.go | 5 +++--
8 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/internal/backend/azure/azure.go b/internal/backend/azure/azure.go
index 682534cd2..33162c227 100644
--- a/internal/backend/azure/azure.go
+++ b/internal/backend/azure/azure.go
@@ -10,11 +10,13 @@ import (
"path"
"strings"
- "github.com/Azure/azure-sdk-for-go/storage"
"github.com/restic/restic/internal/backend"
"github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
+
+ "github.com/Azure/azure-sdk-for-go/storage"
+ "github.com/cenkalti/backoff/v4"
)
// Backend stores data on an azure endpoint.
@@ -119,7 +121,7 @@ func (be *Backend) Path() string {
// Save stores data in the backend at the handle.
func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
objName := be.Filename(h)
@@ -219,7 +221,7 @@ func (be *Backend) Load(ctx context.Context, h restic.Handle, length int, offset
func (be *Backend) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v from %v", h, length, offset, be.Filename(h))
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go
index be1adc160..f6d4331c8 100644
--- a/internal/backend/b2/b2.go
+++ b/internal/backend/b2/b2.go
@@ -11,6 +11,7 @@ import (
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
+ "github.com/cenkalti/backoff/v4"
"github.com/kurin/blazer/b2"
)
@@ -150,7 +151,7 @@ func (be *b2Backend) Load(ctx context.Context, h restic.Handle, length int, offs
func (be *b2Backend) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v from %v", h, length, offset, be.Filename(h))
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
@@ -189,7 +190,7 @@ func (be *b2Backend) Save(ctx context.Context, h restic.Handle, rd restic.Rewind
defer cancel()
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
be.sem.GetToken()
diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go
index 4930264fd..acb4751fc 100644
--- a/internal/backend/local/local.go
+++ b/internal/backend/local/local.go
@@ -85,7 +85,7 @@ func (b *Local) IsNotExist(err error) bool {
func (b *Local) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) (err error) {
debug.Log("Save %v", h)
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
filename := b.Filename(h)
@@ -162,7 +162,7 @@ func (b *Local) Load(ctx context.Context, h restic.Handle, length int, offset in
func (b *Local) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v", h, length, offset)
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
@@ -193,7 +193,7 @@ func (b *Local) openReader(ctx context.Context, h restic.Handle, length int, off
func (b *Local) Stat(ctx context.Context, h restic.Handle) (restic.FileInfo, error) {
debug.Log("Stat %v", h)
if err := h.Valid(); err != nil {
- return restic.FileInfo{}, err
+ return restic.FileInfo{}, backoff.Permanent(err)
}
fi, err := fs.Stat(b.Filename(h))
diff --git a/internal/backend/mem/mem_backend.go b/internal/backend/mem/mem_backend.go
index fc7196f0b..8378630fa 100644
--- a/internal/backend/mem/mem_backend.go
+++ b/internal/backend/mem/mem_backend.go
@@ -8,10 +8,11 @@ import (
"sync"
"github.com/restic/restic/internal/backend"
+ "github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
- "github.com/restic/restic/internal/debug"
+ "github.com/cenkalti/backoff/v4"
)
type memMap map[restic.Handle][]byte
@@ -61,7 +62,7 @@ func (be *MemoryBackend) IsNotExist(err error) bool {
// Save adds new Data to the backend.
func (be *MemoryBackend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
be.m.Lock()
@@ -94,7 +95,7 @@ func (be *MemoryBackend) Load(ctx context.Context, h restic.Handle, length int,
func (be *MemoryBackend) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
be.m.Lock()
@@ -133,7 +134,7 @@ func (be *MemoryBackend) Stat(ctx context.Context, h restic.Handle) (restic.File
defer be.m.Unlock()
if err := h.Valid(); err != nil {
- return restic.FileInfo{}, err
+ return restic.FileInfo{}, backoff.Permanent(err)
}
if h.Type == restic.ConfigFile {
diff --git a/internal/backend/rest/rest.go b/internal/backend/rest/rest.go
index a294cb35f..76feb3a11 100644
--- a/internal/backend/rest/rest.go
+++ b/internal/backend/rest/rest.go
@@ -13,11 +13,12 @@ import (
"golang.org/x/net/context/ctxhttp"
+ "github.com/restic/restic/internal/backend"
"github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
- "github.com/restic/restic/internal/backend"
+ "github.com/cenkalti/backoff/v4"
)
// make sure the rest backend implements restic.Backend
@@ -109,7 +110,7 @@ func (b *Backend) Location() string {
// Save stores data in the backend at the handle.
func (b *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
ctx, cancel := context.WithCancel(ctx)
@@ -204,7 +205,7 @@ func (b *Backend) Load(ctx context.Context, h restic.Handle, length int, offset
func (b *Backend) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v", h, length, offset)
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
@@ -256,7 +257,7 @@ func (b *Backend) openReader(ctx context.Context, h restic.Handle, length int, o
// Stat returns information about a blob.
func (b *Backend) Stat(ctx context.Context, h restic.Handle) (restic.FileInfo, error) {
if err := h.Valid(); err != nil {
- return restic.FileInfo{}, err
+ return restic.FileInfo{}, backoff.Permanent(err)
}
req, err := http.NewRequest(http.MethodHead, b.Filename(h), nil)
@@ -311,7 +312,7 @@ func (b *Backend) Test(ctx context.Context, h restic.Handle) (bool, error) {
// Remove removes the blob with the given name and type.
func (b *Backend) Remove(ctx context.Context, h restic.Handle) error {
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
req, err := http.NewRequest("DELETE", b.Filename(h), nil)
diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go
index 08d3b7aa7..83d784813 100644
--- a/internal/backend/s3/s3.go
+++ b/internal/backend/s3/s3.go
@@ -12,13 +12,13 @@ import (
"time"
"github.com/restic/restic/internal/backend"
+ "github.com/restic/restic/internal/debug"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
+ "github.com/cenkalti/backoff/v4"
"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
-
- "github.com/restic/restic/internal/debug"
)
// Backend stores data on an S3 endpoint.
@@ -260,7 +260,7 @@ func (be *Backend) Save(ctx context.Context, h restic.Handle, rd restic.RewindRe
debug.Log("Save %v", h)
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
objName := be.Filename(h)
@@ -300,7 +300,7 @@ func (be *Backend) Load(ctx context.Context, h restic.Handle, length int, offset
func (be *Backend) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v from %v", h, length, offset, be.Filename(h))
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go
index 4a0c1695b..e395de60d 100644
--- a/internal/backend/sftp/sftp.go
+++ b/internal/backend/sftp/sftp.go
@@ -264,7 +264,7 @@ func (r *SFTP) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader
}
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
filename := r.Filename(h)
@@ -311,7 +311,7 @@ func (r *SFTP) Load(ctx context.Context, h restic.Handle, length int, offset int
func (r *SFTP) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v", h, length, offset)
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
@@ -346,7 +346,7 @@ func (r *SFTP) Stat(ctx context.Context, h restic.Handle) (restic.FileInfo, erro
}
if err := h.Valid(); err != nil {
- return restic.FileInfo{}, err
+ return restic.FileInfo{}, backoff.Permanent(err)
}
fi, err := r.c.Lstat(r.Filename(h))
diff --git a/internal/backend/swift/swift.go b/internal/backend/swift/swift.go
index fcbbec5e5..488ccbd14 100644
--- a/internal/backend/swift/swift.go
+++ b/internal/backend/swift/swift.go
@@ -14,6 +14,7 @@ import (
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
+ "github.com/cenkalti/backoff/v4"
"github.com/ncw/swift"
)
@@ -122,7 +123,7 @@ func (be *beSwift) Load(ctx context.Context, h restic.Handle, length int, offset
func (be *beSwift) openReader(ctx context.Context, h restic.Handle, length int, offset int64) (io.ReadCloser, error) {
debug.Log("Load %v, length %v, offset %v", h, length, offset)
if err := h.Valid(); err != nil {
- return nil, err
+ return nil, backoff.Permanent(err)
}
if offset < 0 {
@@ -162,7 +163,7 @@ func (be *beSwift) openReader(ctx context.Context, h restic.Handle, length int,
// Save stores data in the backend at the handle.
func (be *beSwift) Save(ctx context.Context, h restic.Handle, rd restic.RewindReader) error {
if err := h.Valid(); err != nil {
- return err
+ return backoff.Permanent(err)
}
objName := be.Filename(h)
From 9341a83b0598e7284bbf0eeb4e007399caaba85a Mon Sep 17 00:00:00 2001
From: greatroar <@>
Date: Thu, 17 Dec 2020 21:47:28 +0100
Subject: [PATCH 4/4] Changelog entry for #2453 and #3170
---
changelog/unreleased/issue-2453 | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 changelog/unreleased/issue-2453
diff --git a/changelog/unreleased/issue-2453 b/changelog/unreleased/issue-2453
new file mode 100644
index 000000000..0e91b8c05
--- /dev/null
+++ b/changelog/unreleased/issue-2453
@@ -0,0 +1,12 @@
+Enhancement: Improve handling of permanent backend errors
+
+When encountering errors in reading from or writing to storage backends,
+restic retries the failing operation up to nine times (for a total of ten
+attempts). It used to retry all backend operations, but now detects some
+permanent error conditions so it can report fatal errors earlier.
+
+Permanent failures include local disks being full and SSH connections
+dropping.
+
+https://github.com/restic/restic/issues/2453
+https://github.com/restic/restic/pull/3170