saxenapranav commented on code in PR #6010:
URL: https://github.com/apache/hadoop/pull/6010#discussion_r1313019437
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java:
##########
@@ -345,6 +345,7 @@ private void uploadBlockAsync(DataBlocks.DataBlock
blockToUpload,
return null;
} finally {
IOUtils.close(blockUploadData);
+ blockToUpload.close();
Review Comment:
Thanks a lot for the comment.
`can you review the code to make sure the sequence of L348 always does the
right thing and not fail due to some attempted double delete of the file...`:
1. disk:
1. BlockUploadData.close() -> deletes the file:
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L266
2. blockToUpload.close() -> innerClose() -> closeBlock() -> tries to
delete the file (java.io.File#delete returns true in case path in the obj
deleted succesfully, false otherwise, ref:
https://docs.oracle.com/javase/8/docs/api/java/io/File.html#delete--)
:https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L1118
2. bytebuffer:
1. What BlockUploadData contains: ByteBufferInputStream of `blockBuffer`
:
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L732
2. BlockUploadData.close() -> closes the ByteBufferInputSteam :
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L263
-> unset the `bytebuffer` variable in the inputStream :
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L806
3. BlockToUpload.close() -> releases the buffer :
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L764
-> adds back the buffer in pool:
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L667
3. ByteArrayBlock:
1. BlockUploadData.close() -> closes the byteArrayInputStream(its wrapper
around the buffer array): does nothing.
2. BlockToUpload.close() -> does nothing:
https://github.com/apache/hadoop/blob/f6fa5bd1aa085a4d22f3450b545bb70063da9f51/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/store/DataBlocks.java#L623-L626
For checking that this flow works correctly on all the types of dataBuffers,
have made change in the `testCloseOfDataBlockOnAppendComplete`. This will also
help keep check the sanity in future on any change in the databuffer code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
