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

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



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

Reply via email to