amaechler commented on code in PR #20740:
URL: https://github.com/apache/druid/pull/20740#discussion_r4395037543
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java:
##########
@@ -1255,6 +1256,14 @@ public void shutdown(String taskId, String reason)
if (workerHolderRunningTask == null) {
log.info("Can't shutdown! No worker running task[%s]", taskId);
}
+ } else if (taskRunnerWorkItem.getState() ==
HttpRemoteTaskRunnerWorkItem.State.PENDING) {
+ // Removing it here stops the pending task executor from picking it
up before it is completed below.
+ pendingTaskIds.remove(taskId);
+ pendingWorkItem = taskRunnerWorkItem;
+ } else if (taskRunnerWorkItem.getState() ==
HttpRemoteTaskRunnerWorkItem.State.PENDING_WORKER_ASSIGN) {
+ // The thread assigning this task owns the work item. Once the
worker runs the task, or the item goes back
+ // to PENDING, a later shutdown request handles it.
+ pendingTaskIds.remove(taskId);
Review Comment:
Thanks for the review. I'd keep this out of this PR:
- `runTaskOnWorker()` returns false only when the chosen worker is removed,
lazy or blacklisted, or `assignTask()` finds it disabled.
`getWorkersEligibleToRunTasks()` filters all of these out under `statusLock`
when the worker is picked, so the revert needs the worker to drop out in the
short gap between selection and that check, with the shutdown landing in the
same gap. HTTP failures don't revert: `assignTask()` returns true when it is
unsure, and the assignment timeout completes the item.
- In that case the task is never launched. The item stays listed as pending
until `TaskQueue`'s next cleanup pass calls `shutdown()` again, about one
`druid.indexer.queue.storageSyncRate` (1 minute by default) later, and that
call completes it through the PENDING branch.
- An explicit marker would also need a way for the test worker to reject an
assignment, which the harness doesn't have. #20298 suggested handling in-flight
assignment separately, so I'm happy to do that as a follow-up if the delay
matters to you.
--
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]
