cache: Ensure failed downloads are retried

This fixes #1833, which consists of two different bugs:

 * The `defer` in `cacheFile()` may remove a channel from the
   `inProgress` map although it is not responsible for downloading the
   file

 * If the download fails, goroutines waiting for the file to be cached
   assumed that the file was there, there was no way to signal the
   error.
This commit is contained in:
Alexander Neumann 2018-06-09 17:22:33 +02:00
parent 48aab8bd65
commit fc5439a37a
2 changed files with 72 additions and 23 deletions

View file

@ -91,14 +91,6 @@ var autoCacheFiles = map[restic.FileType]bool{
func (b *Backend) cacheFile(ctx context.Context, h restic.Handle) error {
finish := make(chan struct{})
defer func() {
close(finish)
// remove the finish channel from the map
b.inProgressMutex.Lock()
delete(b.inProgress, h)
b.inProgressMutex.Unlock()
}()
b.inProgressMutex.Lock()
other, alreadyDownloading := b.inProgress[h]
@ -120,10 +112,17 @@ func (b *Backend) cacheFile(ctx context.Context, h restic.Handle) error {
if err != nil {
// try to remove from the cache, ignore errors
_ = b.Cache.Remove(h)
return err
}
return nil
// signal other waiting goroutines that the file may now be cached
close(finish)
// remove the finish channel from the map
b.inProgressMutex.Lock()
delete(b.inProgress, h)
b.inProgressMutex.Unlock()
return err
}
// loadFromCacheOrDelegate will try to load the file from the cache, and fall
@ -131,12 +130,13 @@ func (b *Backend) cacheFile(ctx context.Context, h restic.Handle) error {
func (b *Backend) loadFromCacheOrDelegate(ctx context.Context, h restic.Handle, length int, offset int64, consumer func(rd io.Reader) error) error {
rd, err := b.Cache.Load(h, length, offset)
if err != nil {
debug.Log("error caching %v: %v, falling back to backend", h, err)
return b.Backend.Load(ctx, h, length, offset, consumer)
}
err = consumer(rd)
if err != nil {
rd.Close() // ignore secondary errors
_ = rd.Close() // ignore secondary errors
return err
}
return rd.Close()
@ -193,19 +193,8 @@ func (b *Backend) Load(ctx context.Context, h restic.Handle, length int, offset
debug.Log("auto-store %v in the cache", h)
err := b.cacheFile(ctx, h)
if err == nil {
// load the cached version
rd, err := b.Cache.Load(h, 0, 0)
if err != nil {
return err
}
err = consumer(rd)
if err != nil {
rd.Close() // ignore secondary errors
return err
}
return rd.Close()
return b.loadFromCacheOrDelegate(ctx, h, length, offset, consumer)
}
debug.Log("error caching %v: %v, falling back to backend", h, err)