cache: fix race condition in cache cleanup or similar.#5047
cache: fix race condition in cache cleanup or similar.#5047MichaelEischer merged 1 commit intorestic:masterfrom
Conversation
internal/backend/cache/file.go
Outdated
| err := filepath.Walk(dir, func(name string, fi os.FileInfo, err error) error { | ||
| if err != nil { | ||
| // ignore ErrNotExist to gracefully handle multiple processes clearing the cache | ||
| if(errors.Is(err, os.ErrNotExist) |
There was a problem hiding this comment.
the brackets are somewhat messed up. It should be
if errors.Is(err, os.ErrNotExist) {
return nil
}
There was a problem hiding this comment.
Oops, missed the closing brace. Sorry about that. Made the change directly on github as I don't have a dev environment for Go. Would have benefited from some IDE syntax highlighting there. :)
There was a problem hiding this comment.
I've fixed the syntax error.
Let me know if anything else is needed.
|
Thanks for the bugfix. Can you add a minimal changelog entry? (or should I take that over?) |
No problem. I had a go with the changelog entry. I think it is correct. D. |
Fix multiple restic processes executing concurrently and racing to remove obsolete snapshots. Co-authored-by: Michael Eischer <michael.eischer@fau.de>
b90765d to
4795143
Compare
MichaelEischer
left a comment
There was a problem hiding this comment.
LGTM. I've taken the liberty to fix the remaining syntax error and squashed all commits.
Fix for multiple restic processes executing concurrently and racing to remove obsolete snapshots from the local backend cache.
What does this PR change? What problem does it solve?
This change tests the error returned from the closure/anonymous function, and if the error is that the given file does not exist, then skip the missing file in the results by returning nil.
The purpose is to silence the error generated from a race condition where multiple restic processes are executing concurrently and one unlinks a snapshot file, after another retrieves the file's path from a readdir, but before it attempts an lstat. It is in response to the following error message:
Was the change previously discussed in an issue or on the forum?
I didn't raise an issue, as its quite trivial. It is related to #4760, that was resolved in an identical way.
I'm not a go developer, so please review this PR carefully. I modelled the patch on #4760 but may have made errors.
Checklist
changelog/unreleased/that describes the changes for our users (see template).gofmton the code in all commits.