gianm commented on code in PR #19772:
URL: https://github.com/apache/druid/pull/19772#discussion_r3683724196
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java:
##########
@@ -75,25 +84,41 @@ public class UnusedSegmentsKiller implements OverlordDuty
private static final String TASK_ID_PREFIX = "overlord-issued";
+ /**
+ * Use concurrent locks by default.
Review Comment:
In `delete.md` there is this text added in #19737:
> When using concurrent locks to run a `kill` task, ensure to keep a large
enough buffer period before killing segments after they have been marked as
unused. Otherwise, there may be a potential data loss if a concurrent append
job upgrades one of the segments that are being killed.
There is a similar note in the javadoc for `KillUnusedSegmentsTask`. Why
don't we need to worry about that here? Can the rationale be added here, and
possibly referenced in the other places?
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java:
##########
@@ -361,7 +421,27 @@ private void runKillTask(KillCandidate candidate, String
taskId)
}
finally {
cleanupLocksSilently(killTask);
- emitMetric(Metric.PROCESSED_KILL_JOBS, 1L,
Map.of(DruidMetrics.DATASOURCE, candidate.dataSource));
+ emitMetric(Metric.PROCESSED_KILL_JOBS, 1L,
Map.of(DruidMetrics.DATASOURCE, candidate.dataSource()));
+ }
+ }
+
+ private boolean shouldUseConcurrentLocks(Map<String, Object> context)
+ {
+ if (context == null) {
+ return DEFAULT_USE_CONCURRENT_LOCKS;
+ }
+
+ final Boolean useConcurrentLocksOverride = (Boolean)
context.get(Tasks.USE_CONCURRENT_LOCKS);
Review Comment:
+1, this should coerce. `SupervisorManager#specHasConcurrentLocks` has a
suitable implementation. As written, this may blow up if someone actually sets
the runtime property, if it comes in as a `String`.
--
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]
