Fix corruption during CONFLICT upload #3498
Merged
Conversation
This file contains 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
PBENCH-1219 Large uploads can time out, causing the client (e.g., the 0.69 passthrough server's dispatch) to retry. Eventually, this will result in an `OK` (200) response, which is good. However if we retry before the original operation finishes (it may be still running, despite the client timeout), we catch the already existing "temporary intake directory" as a `CONFLICT` error. Unfortunately, the cleanup logic doesn't recognize this distinction, and still deleted the intake directory on exit. Timed correctly, this could break the original upload: at best, it results in a noisy termination with complaints that the existed temporary directory no longer exists. Fix this problem by attempting to delete only when this API instance has successfully created the temporary directory. Modify the `CONFLICT` unit test case to reproduce the situation more accurately and additionally validate that the directory still exists after completion.
dbutenhof
added
bug
Server
API
|
NOTE: once I have approval here, I'll cherry-pick this to b0.72. |
ndokos
approved these changes
Jul 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PBENCH-1219
Large uploads can time out, causing the client (e.g., the 0.69 passthrough server's dispatch) to retry. Eventually, this will result in an
OK(200) response, which is good. However if we retry before the original operation finishes (it may be still running, despite the client timeout), we catch the already existing "temporary intake directory" as aCONFLICTerror.Unfortunately, the cleanup logic doesn't recognize this distinction, and still deleted the intake directory on exit. Timed correctly, this could break the original upload: at best, it results in a noisy termination with complaints that the previously-existing temporary directory no longer exists.
Fix this problem by attempting to delete only when this API instance has successfully created the temporary directory. Modify the
CONFLICTunit test case to reproduce the situation more accurately and additionally validate that the directory still exists after completion.