[GitHub] [hudi] xicm commented on a diff in pull request #8057: [HUDI-5831] fix bug of early commit conflict detection

["xicm (via GitHub)" <gi...@apache.org>]

xicm commented on code in PR #8057:
URL: https://github.com/apache/hudi/pull/8057#discussion_r1118485563


##########
hudi-common/src/main/java/org/apache/hudi/common/util/MarkerUtils.java:
##########
@@ -269,7 +269,7 @@ public static boolean hasCommitConflict(HoodieActiveTimeline activeTimeline, Set
     Set<String> missingFileIDs = currentInstants.stream().flatMap(instant -> {
       try {
         return HoodieCommitMetadata.fromBytes(activeTimeline.getInstantDetails(instant).get(), HoodieCommitMetadata.class)
-            .getFileIdAndRelativePaths().keySet().stream();
+            .getFileIdAndRelativePaths().values().stream().map(MarkerUtils::makerToPartitionAndFileID);
       } catch (Exception e) {

Review Comment:
   I expected this test to be overridden by testHoodieClientBasicMultiWriterWithEarlyConflictDetection, but it wasn't.
   
   And I find we use partition+fileId in AsyncTimelineServerBasedDetectionStrategy when we detect conflict, but fileId in SimpleDirectMarkerBasedDetectionStrategy. we should use partition+fileId both, all right?
   
   I will go into this. 



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