danny0405 commented on code in PR #19226:
URL: https://github.com/apache/hudi/pull/19226#discussion_r3556694781


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/ListingBasedRollbackStrategy.java:
##########
@@ -151,20 +155,28 @@ public List<HoodieRollbackRequest> 
getRollbackRequests(HoodieInstant instantToRo
               // as part of regular rollback(on re-attempting a failed 
compaction), we might have to delete/rollback only the base file that could have
               // potentially been created. Even if there are log files added 
to the file slice of interest, we should not touch them.
               // but if its part of a restore operation, rolling back a 
compaction should rollback entire file slice, i.e base file and all log files.
-              if (!isRestore) {
-                // Rollback of a compaction action if not for restore means 
that the compaction is scheduled
-                // and has not yet finished. In this scenario we should delete 
only the newly created base files
-                // and not corresponding base commit log files created with 
this as baseCommit since updates would
-                // have been written to the log files.
-                
hoodieRollbackRequests.addAll(getHoodieRollbackRequests(partitionPath,
-                    
listBaseFilesToBeDeleted(instantToRollback.requestedTime(), baseFileExtension, 
partitionPath, metaClient.getStorage())));
-              } else {
+              if (isRestore) {
                 // if this is part of a restore operation, we should 
rollback/delete entire file slice.
                 // For table version 6, the files can be directly fetched from 
the instant to rollback
                 // For table version 8, the log files are not directly 
associated with the base file.
                 // The rollback will iterate in reverse order based on 
completion time so the log files completed
                 // after the compaction will already be queued for removal and 
therefore, only the files from the compaction commit must be deleted.
                 
hoodieRollbackRequests.addAll(getHoodieRollbackRequests(partitionPath, 
filesToDelete.get()));
+              } else {
+                HoodieActiveTimeline activeTimeline = 
metaClient.getActiveTimeline();
+                boolean higherDeltaCommits = 
!activeTimeline.getDeltaCommitTimeline()
+                    .findInstantsAfter(instantToRollback.requestedTime(), 
1).empty();
+                if (higherDeltaCommits) {
+                  // Later delta commits may have written log files on top of 
this compaction base commit, so do not
+                  // delete log files with the same base commit while rolling 
back the compaction itself.
+                  
hoodieRollbackRequests.addAll(getHoodieRollbackRequests(partitionPath,
+                      listBaseFilesToBeDeleted(
+                          instantToRollback.requestedTime(), 
baseFileExtension, partitionPath, metaClient.getStorage())));
+                } else {
+                  // No delta commit is present after this compaction, so 
files created with this instant as the base
+                  // commit can be deleted together.

Review Comment:
   is this an optimization?



-- 
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]

Reply via email to