FrankChen021 commented on code in PR #20299:
URL: https://github.com/apache/druid/pull/20299#discussion_r4004995846
##########
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:
## Follow-up assessment
The empty-queue snapshot issue is resolved in the current head, but the
stale-capacity issue remains. I rechecked 1 of 1 changed files.
`findWorkerToRunTask(ti.getTask())` obtains a fresh `tasksSnapshotRef` view at
line 1158, but `workersWithUnacknowledgedTask.putIfAbsent` runs afterward and
`runTaskOnWorker` later only checks that the worker still exists and is not
lazy/blacklisted before posting `/assignTask`; it does not revalidate capacity.
A `WorkerHolder` delta sync can therefore add another task between the fresh
snapshot and the reservation/request, leaving the selected
`ImmutableWorkerInfo` stale and still allowing an oversubscribing assignment.
Please make the capacity check and reservation atomic, or revalidate
immediately before/inside assignment, and add a regression test for this race.
<!-- mergelens:review -->
---
This is an automated review by Codex GPT-5.6-Luna(max)
--
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]
