nbalajee commented on code in PR #18279:
URL: https://github.com/apache/hudi/pull/18279#discussion_r3047960876
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/RollbackHelperV1.java:
##########
@@ -239,13 +253,16 @@ List<Pair<String, HoodieRollbackStat>>
maybeDeleteAndCollectStats(HoodieEngineCo
}
}
- // This step is intentionally done after writer is closed. Guarantees
that
- // getFileStatus would reflect correct stats and FileNotFoundException
is not thrown in
- // cloud-storage : HUDI-168
- Map<StoragePathInfo, Long> filesToNumBlocksRollback =
Collections.singletonMap(
-
metaClient.getStorage().getPathInfo(Objects.requireNonNull(filePath)),
- 1L
- );
+ Map<StoragePathInfo, Long> filesToNumBlocksRollback;
+ if (fileSizeCaptured) {
Review Comment:
Thanks for the context (#788 and ticket HUDI-168). When we are calling
getFileStatus(), before the close() operation is performed, then the metadata
may not reflect the correct file size. Hence the above block was needed was my
understanding.
Since I noticed (a) when writer.appendBlock() calls flush(), it calls
hsync(), the block will be flushed and there is no gap memory and on-disk
copies (data flushed to the buffers, but not made it to the disk). (b) calling
getCurrentSize() which calls ouput.getPos() should reflect the current size,
we should be able to depend reliably on the getCurrentSize().
Based on the history of #788 and HUDI-168, (the hsync() may be a no-op on
AWS storage S3), closing the file and performing a getFIleStatus() will be
safer and reliable. Restoring it.
--
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]
