[GitHub] [hudi] nsivabalan commented on a change in pull request #3678: [HUDI-2444] Fixing cleaning and rollback if retried after failed attempt

[GitBox <gi...@apache.org>]

nsivabalan commented on a change in pull request #3678:
URL: https://github.com/apache/hudi/pull/3678#discussion_r710436094



##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/BaseRollbackHelper.java
##########
@@ -126,6 +127,8 @@ public BaseRollbackHelper(HoodieTableMetaClient metaClient, HoodieWriteConfig co
         String latestBaseInstant = rollbackRequest.getLatestBaseInstant();
         FileSystem fs = metaClient.getFs();
         // collect all log files that is supposed to be deleted with this rollback
+        // what happens if file was deleted when invoking fs.getFileStatus(?) below.

Review comment:
       @vinothchandar : looking for some clarification here. 
   Is there any reason why we store FileStatus object as part of HoodieRollbackStat. I mean, 
   ```
       public Builder withWrittenLogFileSizeMap(Map<FileStatus, Long> writtenLogFileSizeMap) {
         this.writtenLogFileSizeMap = writtenLogFileSizeMap;
         return this;
       }
   ```
   From what I infer, we invoke FileStatus.getLenth when we are construct the rollback metadata, but is there any other reason.
   
   I understand we may not delete log files at all. But if incase the log file was deleted somehow, won't we run into issues when we do FileStatus.getLength or any other calls? As part of rollback plan we added recently, we can avoid this call completely as the rollback plan stores Map<String, length> and we don't need to invoke any such apis. 
   




-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org