[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8242: handle lineage with empty segmentsFrom

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

Jackie-Jiang commented on a change in pull request #8242:
URL: https://github.com/apache/pinot/pull/8242#discussion_r815206489



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2963,6 +2961,25 @@ public String startReplaceSegments(String tableNameWithType, List<String> segmen
     return segmentLineageEntryId;
   }
 
+  // TODO: Add more conflict checks over segmentsTo later. For example, for APPEND table,
+  //       if the new segments from 2 batch jobs are overlapping, we reject one of job.
+  private static boolean isConflicted(List<String> segmentsFrom, LineageEntry lineageEntry, TableConfig tableConfig) {
+    if (!segmentsFrom.isEmpty()) {
+      // It's conflicted if there is any overlap between segmentsFrom.
+      return !Collections.disjoint(segmentsFrom, lineageEntry.getSegmentsFrom());
+    }
+    // For REFRESH table, it's conflicted if both segmentsFrom are empty.
+    if (isRefreshTable(tableConfig)) {

Review comment:
       I don't follow the logic here. Say I have a refresh table, and I scheduled multiple segment ingestion tasks in parallel, why should one task kill another?
   IMO, we should only perform the disjoint check because having common source segments can result in duplicate data




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org