codope commented on code in PR #9776:
URL: https://github.com/apache/hudi/pull/9776#discussion_r1350023322


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/compact/plan/generators/BaseHoodieCompactionPlanGenerator.java:
##########
@@ -118,8 +120,21 @@ public HoodieCompactionPlan generateCompactionPlan() 
throws IOException {
         .getLatestFileSlices(partitionPath)
         .filter(slice -> filterFileSlice(slice, lastCompletedInstantTime, 
fgIdsInPendingCompactionAndClustering, instantRange))
         .map(s -> {
-          List<HoodieLogFile> logFiles =
-              
s.getLogFiles().sorted(HoodieLogFile.getLogFileComparator()).collect(toList());
+          List<HoodieLogFile> logFiles = s.getLogFiles()
+              // ==============================================================
+              // IMPORTANT
+              // ==============================================================
+              // Currently, our filesystem view could return a file slice with 
pending log files there,
+              // these files should be excluded from the plan, let's say we 
have such a sequence of actions
+
+              // t10: a delta commit starts,
+              // t20: the compaction is scheduled and the t10 delta commit is 
still pending, and the "fg_10.log" is included in the plan
+              // t25: the delta commit 10 finishes,
+              // t30: the compaction execution starts, now the reader 
considers the log file "fg_10.log" as valid.
+
+              // for both OCC and NB-CC, this is in-correct.

Review Comment:
   thanks for adding this test.



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