Anubhav-Roy commented on code in PR #20299:
URL: https://github.com/apache/druid/pull/20299#discussion_r4003909530
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java:
##########
@@ -1097,6 +1105,14 @@ private void pendingTasksExecutionLoop()
ImmutableWorkerInfo immutableWorker = null;
synchronized (statusLock) {
+ // Compute the eligible-worker snapshot ONCE per pass instead of
rebuilding it for every
+ // pending task. Within a single synchronized(statusLock) pass no
worker reservation is made
+ // until the break below, so getWorkersEligibleToRunTasks() is
invariant across the inner
+ // loop. Rebuilding it per pending task made this loop
O(pendingTasks x workers x
+ // tasksPerWorker) and held statusLock for long periods under a
large pending backlog,
+ // which stalled TaskQueue.add/manage and task submission
cluster-wide.
+ final ImmutableMap<String, ImmutableWorkerInfo> eligibleWorkers =
+ ImmutableMap.copyOf(getWorkersEligibleToRunTasks());
Review Comment:
You're right that reusing the pass-start snapshot could oversubscribe under
affinity strategies since WorkerHolder publishes announcements without
statusLock.
In the next commit, the reused snapshot is now only a pre-filter to skip
tasks with no eligible worker; the actual assignment re-selects against a fresh
snapshot right before reserving, so capacity is always current. The lock-hold
reduction is preserved because a saturated cluster pre-filters every pending
task to null against the single reused snapshot without rebuilding it per task.
--
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]
