[GitHub] [hudi] guanziyue commented on a diff in pull request #9523: [DO NOT MEGRE] HUDI-1517 Adding support for Per log file marker to track all log files added by a commit and to assist with rollbacks

[via GitHub]

guanziyue commented on code in PR #9523:
URL: https://github.com/apache/hudi/pull/9523#discussion_r1305909523


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/rollback/MarkerBasedRollbackStrategy.java:
##########
@@ -78,22 +82,35 @@ public List<HoodieRollbackRequest> 
getRollbackRequests(HoodieInstant instantToRo
       return context.map(markerPaths, markerFilePath -> {
         String typeStr = 
markerFilePath.substring(markerFilePath.lastIndexOf(".") + 1);
         IOType type = IOType.valueOf(typeStr);
+        String partitionFilePath = 
WriteMarkers.stripMarkerSuffix(markerFilePath);
+        Path fullFilePath = new Path(basePath, partitionFilePath);
+        String partitionPath = FSUtils.getRelativePartitionPath(new 
Path(basePath), fullFilePath.getParent());
         switch (type) {
           case MERGE:
           case CREATE:
-            String fileToDelete = 
WriteMarkers.stripMarkerSuffix(markerFilePath);
-            Path fullDeletePath = new Path(basePath, fileToDelete);
-            String partitionPath = FSUtils.getRelativePartitionPath(new 
Path(basePath), fullDeletePath.getParent());
-            return new HoodieRollbackRequest(partitionPath, EMPTY_STRING, 
EMPTY_STRING,
-                Collections.singletonList(fullDeletePath.toString()),
+            String fileId = null;
+            String baseInstantTime = null;
+            if (FSUtils.isBaseFile(fullFilePath)) {
+              HoodieBaseFile baseFileToDelete = new 
HoodieBaseFile(fullFilePath.toString());
+              fileId = baseFileToDelete.getFileId();
+              baseInstantTime = baseFileToDelete.getCommitTime();
+            } else if (FSUtils.isLogFile(fullFilePath)) {
+              // TODO: HUDI-1517 may distinguish log file created from log 
file being appended in the future @guanziyue
+              // Now it should not have create type
+              checkArgument(type != IOType.CREATE, "Log file should not 
support create io type now");
+              checkArgument(type != IOType.MERGE, "Log file should not support 
merge io type");

Review Comment:
   may be we have opportunity to remove log files created in future. That's why 
I reserve partial code. Anyway, I'm also glad to remove all of them if I have 
enough time. Feel free to leave a comment about that



-- 
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-unsubscr...@hudi.apache.org

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