maytasm commented on code in PR #14642:
URL: https://github.com/apache/druid/pull/14642#discussion_r1278255538
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/KillUnusedSegmentsTask.java:
##########
@@ -114,23 +119,37 @@ public TaskStatus runTask(TaskToolbox toolbox) throws
Exception
}
// List unused segments
- final List<DataSegment> unusedSegments = toolbox
+ final List<DataSegment> allUnusedSegments = toolbox
.getTaskActionClient()
.submit(new RetrieveUnusedSegmentsAction(getDataSource(),
getInterval()));
- if (!TaskLocks.isLockCoversSegments(taskLockMap, unusedSegments)) {
- throw new ISE(
- "Locks[%s] for task[%s] can't cover segments[%s]",
-
taskLockMap.values().stream().flatMap(List::stream).collect(Collectors.toList()),
- getId(),
- unusedSegments
- );
+ final List<List<DataSegment>> unusedSegmentBatches =
Lists.partition(allUnusedSegments, SEGMENT_NUKE_BATCH_SIZE);
+
+ // The individual activities here on the toolbox have possibility to run
for a longer period of time,
+ // since they involve calls to metadata storage and archival object
storage. And, the tasks take hold of the
+ // task lockbox to run. By splitting the segment list into smaller
batches, we have an opportunity to yield the
+ // lock to other activity that might need to happen using the overlord
tasklockbox.
+
+ for (final List<DataSegment> unusedSegments : unusedSegmentBatches) {
+ if (!TaskLocks.isLockCoversSegments(taskLockMap, unusedSegments)) {
+ throw new ISE(
+ "Locks[%s] for task[%s] can't cover segments[%s]",
+
taskLockMap.values().stream().flatMap(List::stream).collect(Collectors.toList()),
+ getId(),
+ unusedSegments
+ );
+ }
+
+ // Kill segments:
+ // Order is important here: we want the nuke action to clean up the
metadata records _before_ the
+ // segments are removed from storage, this helps maintain that we will
always have a storage segment if
Review Comment:
We decided not to do the batching inside SegmentNukeAction but in
KillUnusedSegmentsTask. This still ensures the current behavior of deleting
from sql and only delete from s3 if sql succeeded. This basically is like
turning one KillUnusedSegmentsTask into multiple smaller KillUnusedSegmentsTask.
--
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]
