check: fix error reporting on download retry

This commit is contained in:
Michael Eischer
2025-09-07 17:13:19 +02:00
parent c55df65bb6
commit de29d74707
3 changed files with 49 additions and 6 deletions

View File

@@ -0,0 +1,11 @@
Bugfix: Improve handling of download retries in `check` command
In very rare cases, the `check` command could unnecessarily report a
repository damage if the backend returns incomplete, corrupted data on
the first download try which is resolved by a download retry.
Now, only the check result of the last download attempt is reported as
intended.
https://github.com/restic/restic/issues/5467
https://github.com/restic/restic/pull/5495

View File

@@ -293,7 +293,7 @@ type errorBackend struct {
func (b errorBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error { func (b errorBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error {
return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error {
if b.ProduceErrors { if b.ProduceErrors {
return consumer(errorReadCloser{rd}) return consumer(errorReadCloser{Reader: rd})
} }
return consumer(rd) return consumer(rd)
}) })
@@ -301,12 +301,21 @@ func (b errorBackend) Load(ctx context.Context, h backend.Handle, length int, of
type errorReadCloser struct { type errorReadCloser struct {
io.Reader io.Reader
shortenBy int
maxErrorOffset int // if 0, the error can be injected at any offset
} }
func (erd errorReadCloser) Read(p []byte) (int, error) { func (erd errorReadCloser) Read(p []byte) (int, error) {
n, err := erd.Reader.Read(p) n, err := erd.Reader.Read(p)
if n > 0 { if n > 0 {
induceError(p[:n]) maxOffset := n
if erd.maxErrorOffset > 0 {
maxOffset = min(erd.maxErrorOffset, maxOffset)
}
induceError(p[:maxOffset])
}
if n > erd.shortenBy {
n -= erd.shortenBy
} }
return n, err return n, err
} }
@@ -321,16 +330,25 @@ func induceError(data []byte) {
type errorOnceBackend struct { type errorOnceBackend struct {
backend.Backend backend.Backend
m sync.Map m sync.Map
shortenBy int
maxErrorOffset int
} }
func (b *errorOnceBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error { func (b *errorOnceBackend) Load(ctx context.Context, h backend.Handle, length int, offset int64, consumer func(rd io.Reader) error) error {
_, isRetry := b.m.LoadOrStore(h, struct{}{}) _, isRetry := b.m.LoadOrStore(h, struct{}{})
return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error { err := b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error {
if !isRetry && h.Type != restic.ConfigFile { if !isRetry && h.Type != restic.ConfigFile {
return consumer(errorReadCloser{rd}) return consumer(errorReadCloser{Reader: rd, shortenBy: b.shortenBy, maxErrorOffset: b.maxErrorOffset})
} }
return consumer(rd) return consumer(rd)
}) })
if err == nil {
return nil
}
// retry if the consumer returned an error
return b.Backend.Load(ctx, h, length, offset, func(rd io.Reader) error {
return consumer(rd)
})
} }
func TestCheckerModifiedData(t *testing.T) { func TestCheckerModifiedData(t *testing.T) {
@@ -368,6 +386,15 @@ func TestCheckerModifiedData(t *testing.T) {
} }
}, },
}, },
{
// ignore if a backend returns incomplete garbled data on the first try
"corruptPartialOnceBackend",
&errorOnceBackend{Backend: be, shortenBy: 10, maxErrorOffset: 100},
func() {},
func(t *testing.T, err error) {
test.Assert(t, err == nil, "unexpected error found, got %v", err)
},
},
} { } {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
checkRepo := repository.TestOpenBackend(t, test.be) checkRepo := repository.TestOpenBackend(t, test.be)

View File

@@ -88,10 +88,14 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re
// calculate hash on-the-fly while reading the pack and capture pack header // calculate hash on-the-fly while reading the pack and capture pack header
var hash restic.ID var hash restic.ID
var hdrBuf []byte var hdrBuf []byte
// must use a separate slice from `errs` here as we're only interested in the last retry
var blobErrors []error
h := backend.Handle{Type: backend.PackFile, Name: id.String()} h := backend.Handle{Type: backend.PackFile, Name: id.String()}
err := r.be.Load(ctx, h, int(size), 0, func(rd io.Reader) error { err := r.be.Load(ctx, h, int(size), 0, func(rd io.Reader) error {
hrd := hashing.NewReader(rd, sha256.New()) hrd := hashing.NewReader(rd, sha256.New())
bufRd.Reset(hrd) bufRd.Reset(hrd)
// reset blob errors for each retry
blobErrors = nil
it := newPackBlobIterator(id, newBufReader(bufRd), 0, blobs, r.Key(), dec) it := newPackBlobIterator(id, newBufReader(bufRd), 0, blobs, r.Key(), dec)
for { for {
@@ -108,7 +112,7 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re
debug.Log(" check blob %v: %v", val.Handle.ID, val.Handle) debug.Log(" check blob %v: %v", val.Handle.ID, val.Handle)
if val.Err != nil { if val.Err != nil {
debug.Log(" error verifying blob %v: %v", val.Handle.ID, val.Err) debug.Log(" error verifying blob %v: %v", val.Handle.ID, val.Err)
errs = append(errs, errors.Errorf("blob %v: %v", val.Handle.ID, val.Err)) blobErrors = append(blobErrors, errors.Errorf("blob %v: %v", val.Handle.ID, val.Err))
} }
} }
@@ -134,6 +138,7 @@ func checkPackInner(ctx context.Context, r *Repository, id restic.ID, blobs []re
hash = restic.IDFromHash(hrd.Sum(nil)) hash = restic.IDFromHash(hrd.Sum(nil))
return nil return nil
}) })
errs = append(errs, blobErrors...)
if err != nil { if err != nil {
var e *partialReadError var e *partialReadError
isPartialReadError := errors.As(err, &e) isPartialReadError := errors.As(err, &e)