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



##########
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:
       @jackjlli @snleee In order to handle such case, we should also check if 
`segmentsTo` are disjoint. We don't want to override the just pushed segments. 
These checks are already there for non-force-cleanup branch, not sure why it is 
not done in the same way for force-cleanup branch.




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