Implement Atomic Blob Writes for HDFS Repository #37066
Conversation
* Implement atomic writes the same way we do for the FsBlobContainer via rename which is atomic * Relates elastic#37011
original-brownbear
added
>enhancement
:Distributed/Snapshot/Restore
|
Pinging @elastic/es-distributed |
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| stream.write(buffer, 0, bytesRead); | ||
| } | ||
| stream.hsync(); |
There was a problem hiding this comment.
I made this a little faster than the approach in writeBlob here and just hsync once when done writing. I think we can likely make the same change in writeBlob too because partial files don't really hold any value so no need to sync more at the end of a write than once for a blob.
There was a problem hiding this comment.
Created #37069 to speed up writes using the same strategy for non-atomic writes.
|
Fails tests for some reason, looking into it |
| @@ -116,6 +118,30 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize, b | |||
| }); | |||
| } | |||
|
|
|||
| @Override | |||
| public void writeBlobAtomic(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists) throws IOException { | |||
| final String tempBlob = FsBlobContainer.tempBlobName(blobName); | |||
There was a problem hiding this comment.
should we move this method up to the BlobContainer interface?
There was a problem hiding this comment.
I'd rather not I think. This method is kinda specific to the two file-system based implementations. Not sure it makes sense blowing up the interface for that?
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| stream.write(buffer, 0, bytesRead); | ||
| } | ||
| stream.hsync(); |
There was a problem hiding this comment.
I think this only syncs the current block. To have syncs across blocks, you will also need to specify CreateFlag.SYNC_BLOCK when creating the file. I think the easier solution (after tracing through DFSOutputStream) is to just use the CreateFlag.SYNC_BLOCK flag and never perform a manual hsync here. Upon closing the stream, ``CreateFlag.SYNC_BLOCK` will also make sure that the endBlock will be synced.
There was a problem hiding this comment.
Urgh, you're right ... should've read the second paragraph in the docs of that method :)
Your solution is better will adjust.
|
@ywelsch fixed the sync method :) |
|
Jenkins run gradle build tests 1 |
* Implement atomic writes the same way we do for the FsBlobContainer via rename which is atomic * Relates #37011
* With elastic#37066 introducing atomic writes to HDFS repository we can enforce atomic write capabilities on this interface * The overrides on the other three cloud implementations are ok because: * https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectPUT.html states that "Amazon S3 never adds partial objects; if you receive a success response, Amazon S3 added the entire object to the bucket." * https://cloud.google.com/storage/docs/consistency states that GCS has strong read-after-write consistency * https://docs.microsoft.com/en-us/rest/api/storageservices/put-block#remarks Azure has the concept of committing blobs, so there's no partial content here either * Relates elastic#37011
* With #37066 introducing atomic writes to HDFS repository we can enforce atomic write capabilities on this interface * The overrides on the other three cloud implementations are ok because: * https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectPUT.html states that "Amazon S3 never adds partial objects; if you receive a success response, Amazon S3 added the entire object to the bucket." * https://cloud.google.com/storage/docs/consistency states that GCS has strong read-after-write consistency * https://docs.microsoft.com/en-us/rest/api/storageservices/put-block#remarks Azure has the concept of committing blobs, so there's no partial content here either * Relates #37011
FsBlobContainervia rename which is atomic