You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2026/06/01 19:54:18 UTC
[PR] feat: Wait for gracefulShutdownTimeout in DartWorkerRunner#stop. (druid)
gianm opened a new pull request, #19538:
URL: https://github.com/apache/druid/pull/19538
This patch adjusts DartWorkerRunner to wait for gracefulShutdownTimeout before canceling workers.
The default gracefulShutdownTimeout is changed from 0s to 30s to match the documentation, and also, it is believed, to match actual behavior.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Wait for gracefulShutdownTimeout in DartWorkerRunner#stop. (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19538:
URL: https://github.com/apache/druid/pull/19538#discussion_r3341106768
##########
multi-stage-query/src/main/java/org/apache/druid/msq/dart/worker/DartWorkerRunner.java:
##########
@@ -250,15 +275,47 @@ public void start()
@LifecycleStop
public void stop()
{
+ final List<WorkerHolder> runningWorkers;
synchronized (this) {
- final Collection<WorkerHolder> holders = workerMap.values();
-
- for (final WorkerHolder holder : holders) {
- holder.runRef.cancel();
+ if (stopped) {
+ return;
}
+ stopped = true;
+ runningWorkers = new ArrayList<>(workerMap.values());
+ workerMap.clear();
+ }
- for (final WorkerHolder holder : holders) {
- holder.runRef.awaitStop();
+ if (runningWorkers.isEmpty()) {
+ return;
+ }
+
+ // Wait for workers to exit outside the lock.
+ final DateTime waitStart = DateTimes.utc(clock.millis());
+ final DateTime deadline = waitStart.plus(serverConfig.getGracefulShutdownTimeout());
+
+ log.info(
+ "Waiting until[%s] for queries[%s] to stop.",
+ deadline,
+ runningWorkers.stream().map(holder -> holder.workerContext.queryId()).collect(Collectors.joining(", "))
+ );
+
+ for (final WorkerHolder holder : runningWorkers) {
+ try {
+ final long timeout = deadline.getMillis() - clock.millis();
+ if (timeout <= 0 || !holder.runRef.awaitStop(timeout, TimeUnit.MILLISECONDS)) {
+ log.warn(
+ "Canceling work for query[%s] due to timeout during stop (waited [%,d] ms)",
+ holder.workerContext.queryId(),
+ clock.millis() - waitStart.getMillis()
+ );
+
+ holder.runRef.cancel();
+ holder.runRef.awaitStop();
+ }
+ }
+ catch (InterruptedException e) {
Review Comment:
[P2] Cancel remaining workers when shutdown wait is interrupted
If the lifecycle stop thread is interrupted while waiting for a worker, awaitStop now throws and this catch exits without canceling the current or remaining runningWorkers. Since stopped is already true and workerMap has been cleared, a later stop() call returns immediately, leaving background worker threads running instead of forcing cancellation. The interruption path should cancel the copied workers before returning or rethrowing.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Wait for gracefulShutdownTimeout in DartWorkerRunner#stop. (druid)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19538:
URL: https://github.com/apache/druid/pull/19538#issuecomment-5154237271
This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the dev@druid.apache.org list.
Thank you for your contributions.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org