Add opt-in persist-retry feature for transient rename failures - #445
Open
ChrisJr404 wants to merge 1 commit into
Open
ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
Persisting a temporary file can fail transiently on Windows when antivirus or file-indexing software briefly locks the file, causing the rename to fail with a sharing violation or access-denied error. Add a `persist-retry` feature that retries `persist` and `persist_noclobber` with a short, bounded backoff on those errors. It is disabled by default and does not change behavior when off.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds the
persist-retryfeature we discussed in #316, so thatpersistandpersist_noclobberretry the underlying rename when it fails with a transient error instead of failing on the first attempt.The motivating case is Windows, where antivirus and file-indexing software briefly lock a file and make the rename fail with a sharing violation or access-denied error. When the feature is enabled, those errors are retried with an exponential backoff bounded to roughly a second (the same ballpark as graceful-fs, which was the prior art in the issue). Following your guidance on the issue, it's a feature flag that's off by default, it doesn't add a new method or any platform-specific API, and it adds no sleeps unless a caller opts in.
The retry loop is factored so the sleep is injectable, which keeps the behavior unit-testable without actually sleeping in the test suite. I ran the retry logic against the
x86_64-pc-windows-msvctarget as well to make sure the sharing-violation path type-checks. Retries are keyed offPermissionDenied(whichERROR_ACCESS_DENIEDmaps to) plus the rawERROR_SHARING_VIOLATIONcode on Windows.Closes #316.