[GitHub] [druid] imply-cheddar commented on a change in pull request #12137: Batch ingestion replace

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

imply-cheddar commented on a change in pull request #12137:
URL: https://github.com/apache/druid/pull/12137#discussion_r800304804



##########
File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/IndexTask.java
##########
@@ -941,9 +939,37 @@ private TaskStatus generateAndPublishSegments(
               ingestionSchema
           );
 
+      Set<DataSegment> tombStones = Collections.emptySet();
+      if (ingestionSchema.getIOConfig().isDropExisting()) {
+        TombstoneHelper tombstoneHelper = new TombstoneHelper(pushed.getSegments(),
+                                                              ingestionSchema.getDataSchema(),
+                                                              toolbox.getTaskActionClient());
+
+        List<Interval> tombstoneIntervals = tombstoneHelper.computeTombstoneIntervals();
+        // now find the versions for the tombstone intervals
+        Map<Interval, String> tombstonesAndVersions = new HashMap<>();
+        for (Interval interval : tombstoneIntervals) {
+          NonnullPair<Interval, String> intervalAndVersion =
+              findIntervalAndVersion(

Review comment:
       Generally speaking, at this point, the time interval should've already been locked, but the method that you are calling here has a bunch of logic that is trying to deal with a lock not yet existing.  Do you have a test case that exercises the code to somehow cause an interval to be hit here that isn't already locked?  I'm curious about the case of events that can lead to that, as there is likely a bug somewhere else in the stack.  This code should be able to just use the locks that already exist to get a version (that version should also generally be the same for all of the locks), it should most likely throw an error if it's gotten to this point and not locked the interval yet.
   




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

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



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