diff --git a/changelog/unreleased/pull-4152 b/changelog/unreleased/pull-4152 new file mode 100644 index 000000000..4eb004f20 --- /dev/null +++ b/changelog/unreleased/pull-4152 @@ -0,0 +1,16 @@ +Enhancement: Ignore empty lock files + +With restic 0.15.0 the checks for stale locks became much stricter than before. +In particular, empty or unreadable locks were no longer ignored. This caused +restic to complain about `Load(, 0, 0) returned error, +retrying after 552.330144ms: load(): invalid data returned` +and fail in the end. + +We have clarified the error message and changed the implementation to ignore +empty lock files which are sometimes created as the result of a failed upload +on some backends. Unreadable lock files still have to cleaned up manually. To +do so, you can run `restic unlock --remove-all` which removes all existing lock +files. But first make sure that no other restic process is currently running. + +https://github.com/restic/restic/issues/4143 +https://github.com/restic/restic/pull/4152 diff --git a/cmd/restic/lock.go b/cmd/restic/lock.go index e15d63b8d..f39a08db6 100644 --- a/cmd/restic/lock.go +++ b/cmd/restic/lock.go @@ -2,11 +2,11 @@ package main import ( "context" - "fmt" "sync" "time" "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/restic" ) @@ -44,8 +44,11 @@ func lockRepository(ctx context.Context, repo restic.Repository, exclusive bool) } lock, err := lockFn(ctx, repo) + if restic.IsInvalidLock(err) { + return nil, ctx, errors.Fatalf("%v\n\nthe `unlock --remove-all` command can be used to remove invalid locks. Make sure that no other restic process is accessing the repository when running the command", err) + } if err != nil { - return nil, ctx, fmt.Errorf("unable to create lock in backend: %w", err) + return nil, ctx, errors.Fatalf("unable to create lock in backend: %v", err) } debug.Log("create lock %p (exclusive %v)", lock, exclusive) diff --git a/internal/repository/repository.go b/internal/repository/repository.go index a8d885a01..df8a6fb68 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -188,6 +188,7 @@ func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id res h := restic.Handle{Type: t, Name: id.String()} retriedInvalidData := false + var dataErr error err := r.be.Load(ctx, h, 0, 0, func(rd io.Reader) error { // make sure this call is idempotent, in case an error occurs wr := bytes.NewBuffer(buf[:0]) @@ -202,13 +203,20 @@ func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id res if !retriedInvalidData { retriedInvalidData = true } else { + // with a canceled context there is not guarantee which error will + // be returned by `be.Load`. + dataErr = fmt.Errorf("load(%v): %w", h, restic.ErrInvalidData) cancel() } - return errors.Errorf("load(%v): invalid data returned", h) + return restic.ErrInvalidData + } return nil }) + if dataErr != nil { + return nil, dataErr + } if err != nil { return nil, err } diff --git a/internal/repository/testing.go b/internal/repository/testing.go index 5b2e17366..879650336 100644 --- a/internal/repository/testing.go +++ b/internal/repository/testing.go @@ -8,6 +8,7 @@ import ( "github.com/restic/restic/internal/backend/local" "github.com/restic/restic/internal/backend/mem" + "github.com/restic/restic/internal/backend/retry" "github.com/restic/restic/internal/crypto" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/test" @@ -97,11 +98,14 @@ func TestRepositoryWithVersion(t testing.TB, version uint) restic.Repository { // TestOpenLocal opens a local repository. func TestOpenLocal(t testing.TB, dir string) (r restic.Repository) { + var be restic.Backend be, err := local.Open(context.TODO(), local.Config{Path: dir, Connections: 2}) if err != nil { t.Fatal(err) } + be = retry.New(be, 3, nil, nil) + repo, err := New(be, Options{}) if err != nil { t.Fatal(err) diff --git a/internal/restic/lock.go b/internal/restic/lock.go index 8f49aee49..2da3667a3 100644 --- a/internal/restic/lock.go +++ b/internal/restic/lock.go @@ -60,6 +60,27 @@ func IsAlreadyLocked(err error) bool { return errors.As(err, &e) } +// invalidLockError is returned when NewLock or NewExclusiveLock fail due +// to an invalid lock. +type invalidLockError struct { + err error +} + +func (e *invalidLockError) Error() string { + return fmt.Sprintf("invalid lock file: %v", e.err) +} + +func (e *invalidLockError) Unwrap() error { + return e.err +} + +// IsInvalidLock returns true iff err indicates that locking failed due to +// an invalid lock. +func IsInvalidLock(err error) bool { + var e *invalidLockError + return errors.As(err, &e) +} + // NewLock returns a new, non-exclusive lock for the repository. If an // exclusive lock is already held by another process, it returns an error // that satisfies IsAlreadyLocked. @@ -146,7 +167,7 @@ func (l *Lock) checkForOtherLocks(ctx context.Context) error { // if we cannot load a lock then it is unclear whether it can be ignored // it could either be invalid or just unreadable due to network/permission problems debug.Log("ignore lock %v: %v", id, err) - return errors.Fatal(err.Error()) + return err } if l.Exclusive { @@ -168,6 +189,9 @@ func (l *Lock) checkForOtherLocks(ctx context.Context) error { return err } } + if errors.Is(err, ErrInvalidData) { + return &invalidLockError{err} + } return err } @@ -336,6 +360,12 @@ func ForAllLocks(ctx context.Context, repo Repository, excludeID *ID, fn func(ID if excludeID != nil && id.Equal(*excludeID) { return nil } + if size == 0 { + // Ignore empty lock files as some backends do not guarantee atomic uploads. + // These may leave empty files behind if an upload was interrupted between + // creating the file and writing its data. + return nil + } lock, err := LoadLock(ctx, repo, id) m.Lock() diff --git a/internal/restic/repository.go b/internal/restic/repository.go index c559b5aa3..e01d204e6 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -4,10 +4,14 @@ import ( "context" "github.com/restic/restic/internal/crypto" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/ui/progress" "golang.org/x/sync/errgroup" ) +// ErrInvalidData is used to report that a file is corrupted +var ErrInvalidData = errors.New("invalid data returned") + // Repository stores data in a backend. It provides high-level functions and // transparently encrypts/decrypts data. type Repository interface {