amaechler opened a new pull request, #20740:
URL: https://github.com/apache/druid/pull/20740
Fixes #20298.
### Description
A task killed while waiting for a task slot could still be launched by
`HttpRemoteTaskRunner`. `TaskQueue.shutdown()` marks the task FAILED, releases
its locks and calls `TaskRunner.shutdown()`, but
`HttpRemoteTaskRunner.shutdown()` ignored PENDING and PENDING_WORKER_ASSIGN
work items. The pending task executor then assigned the task to the next free
worker, where it ran until the next storage sync dropped the queue entry and
the Overlord shut it down again. It also kept showing up in `getPendingTasks()`.
`shutdown()` now removes a PENDING work item from `pendingTaskIds` under
`statusLock`, then completes it with a FAILED status outside the lock, as
`taskComplete(...)` requires. The item stays in `tasks` as COMPLETE, like a
task a worker reported finished, and the next `shutdown()` call from
`TaskQueue` removes it. `TaskQueue`'s callback for the FAILED future returns
early because the entry is already complete.
A PENDING_WORKER_ASSIGN item is only removed from `pendingTaskIds`, so it
cannot be assigned again. Its state belongs to the thread that is assigning it,
which may revert it to PENDING outside `statusLock`; completing it from
`shutdown()` would race with that revert. If the worker starts the task, the
next `shutdown()` call from `TaskQueue` finds it RUNNING and kills it on the
worker. If the assignment fails, the item goes back to PENDING and the next
`shutdown()` call completes it.
This is a regression from #13558, which stopped `shutdown()` from removing
every work item from `tasks` so that running tasks keep their completion
callbacks.
#### Release note
Fixed an issue where a task killed while waiting for a task slot could still
be launched on a Middle Manager or Indexer when the Overlord uses the
`httpRemote` task runner (the default).
<hr>
##### Key changed/added classes in this PR
* `HttpRemoteTaskRunner`
<hr>
This PR has:
- [x] been self-reviewed.
- [x] using the [concurrency
checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md)
(Remove this item if the PR doesn't have any relation to concurrency.)
- [x] a release note entry in the PR description.
- [x] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
--
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]
