kfaraz commented on code in PR #19737:
URL: https://github.com/apache/druid/pull/19737#discussion_r3644580678


##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTask.java:
##########
@@ -352,7 +347,47 @@ protected List<DataSegment> 
fetchNextBatchOfUnusedSegments(TaskToolbox toolbox,
             nextBatchSize,
             maxUsedStatusLastUpdatedTime
         )
-    );
+    )
+                  .stream()
+                  .map(segment -> new DataSegmentPlus(segment, null, null, 
null, null, null, null, null))
+                  .collect(Collectors.toList());
+  }
+
+  /**
+   * Fetches the parent IDs (if any) for the given unused segments.
+   *
+   * @param unusedSegments Unused segments whose parent IDs need to be fetched
+   * @return Map from segment ID to the segment ID from which
+   * it was upgraded. If an input segment was not upgraded from any other 
segment,
+   * it does not have an entry in the map.
+   */
+  protected Map<String, String> fetchParentIdsForSegments(
+      TaskToolbox toolbox,
+      List<DataSegmentPlus> unusedSegments
+  )
+  {
+    try {
+      final Set<String> segmentIds = unusedSegments.stream().map(
+          s -> s.getDataSegment().getId().toString()
+      ).collect(Collectors.toSet());
+
+      return toolbox.getTaskActionClient().submit(
+          new RetrieveUpgradedFromSegmentIdsAction(getDataSource(), segmentIds)
+      ).getUpgradedFromSegmentIds();
+    }
+    catch (Exception e) {
+      // Do not proceed with killing these segments as we cannot be sure if 
their
+      // load spec is shared by any other segment or not. If load spec is 
shared,
+      // segment files cannot be deleted from deep store. If load spec is not
+      // shared, segments cannot be deleted from metadata store as that would
+      // leave deep store files orphaned, and they would never be cleaned up.
+      throw new ISE(
+          e,
+          "Could not retrieve parent segment ids using task 
action[retrieveUpgradedFromSegmentIds]."
+          + " Stopping kill task to avoid data loss in case the segment files"
+          + " are shared by other segments."

Review Comment:
   Thanks for the suggestion! Added some tests.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to