More resilient blob handling in snapshot repositories #19706
Merged
Conversation
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
Added BlobContainer tests for Azure storage and caught a bug at the same time in which deleteBlob was not raising an IOException when the blobName did not exist.
Added BlobContainer tests for HDFS storage and caught a bug at the same time in which deleteBlob was not raising an IOException when the blobName did not exist.
Makes deleting snapshots more robust by first deleting the snapshot from the index generational file, then handling individual deletion file errors with log messages instead of failing the entire operation.
for reads and deletes if the blob does not exist.
to snapshot/restore) and the index to UUID mapping is stored in the repository index file.
Azure and Google cloud blob containers, as the APIs for both return a 404 in the case of a missing object, which we already handle through a NoSuchFileFoundException.
upgrades if it determines the read data is in the legacy format. It writes the upgraded version if it is not a read-only repository and caches the repository data if it is a read-only repository.
Snapshot UUIDs in blob names
abeyad
added
>enhancement
:Distributed Coordination/Snapshot/Restore
| logger.trace("deleteBlob({})", blobName); | ||
|
|
||
| if (!blobExists(blobName)) { | ||
| throw new IOException("Blob [" + blobName + "] does not exist"); |
Contributor
There was a problem hiding this comment.
Should it be a NoSuchFileException here?
Author
There was a problem hiding this comment.
@dadoonet Great catch, thanks! I will fix it before merging
instead of a vanilla IOException when the blob doesn't exist, in order to conform to the BlobContainer's interface contract.
Author
|
This PR has passed testing on CI twice as well as locally for me, so I'm merging and will monitor the builds over the next few days. |
Author
|
@gfyoung many thanks for your great work on getting this through! |
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.
This PR encompasses work done to enhance the resiliency of snapshot repository blob handling. In particular:
Repositoryinterface now throw an IOException if the blob does not exist.indicesfolder, in order to prevent invalid characters in blob names as described in Escaping index name when creating a snapshot #7540This PR is a combination of two other PRs:
#18815
#19421
Relates #18156