FrankChen021 commented on code in PR #19772:
URL: https://github.com/apache/druid/pull/19772#discussion_r3682717163
##########
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:
[P2] Parse untyped default context values instead of casting
Default task context is `Map<String, Object>`, and task lock code accepts
string forms such as `"true"` via `QueryContexts`. Directly casting
`useConcurrentLocks` to `Boolean` throws `ClassCastException` for that valid
representation; `TaskLockType.valueOf` also rejects lowercase values accepted
elsewhere. This can prevent the Overlord duty from being constructed. Use
`QueryContexts.getAsBoolean` and `getAsEnum`.
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/duty/UnusedSegmentsKiller.java:
##########
@@ -436,33 +509,27 @@ private EmbeddedKillTask(
{
super(
taskId,
- candidate.dataSource,
- candidate.interval,
- null,
- Map.of(Tasks.PRIORITY_KEY,
Tasks.DEFAULT_EMBEDDED_KILL_TASK_PRIORITY),
- null,
+ candidate.dataSource(),
+ candidate.interval(),
null,
+ Map.of(
+ Tasks.PRIORITY_KEY, Tasks.DEFAULT_EMBEDDED_KILL_TASK_PRIORITY,
+ Tasks.USE_CONCURRENT_LOCKS, useConcurrentLocks
Review Comment:
[P1] REPLACE locks do not resolve capped-scan starvation
REPLACE only coexists with APPEND. Default ingestion still uses EXCLUSIVE
locks, and SHARED or REPLACE jobs also remain incompatible. Those intervals are
skipped and selected again on every capped scan, indefinitely starving eligible
intervals beyond the cap. The prior head-of-line finding remains unresolved
unless queue rebuilding advances past skipped candidates.
--
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]
