kfaraz commented on code in PR #17535:
URL: https://github.com/apache/druid/pull/17535#discussion_r1883595718
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorManager.java:
##########
@@ -56,13 +64,17 @@ public class SupervisorManager
// SupervisorTaskAutoScaler could be null
private final ConcurrentHashMap<String, SupervisorTaskAutoScaler>
autoscalers = new ConcurrentHashMap<>();
private final Object lock = new Object();
+ private final ListeningExecutorService shutdownExec;
private volatile boolean started = false;
@Inject
public SupervisorManager(MetadataSupervisorManager metadataSupervisorManager)
{
this.metadataSupervisorManager = metadataSupervisorManager;
+ this.shutdownExec = MoreExecutors.listeningDecorator(
+ Execs.multiThreaded(25, "supervisor-manager-shutdown-%d")
Review Comment:
You could perhaps work around that problem by doing something like this:
- `stopAsync` sets the supervisor state to STOPPING
- `stopAsync` then submits a `stop()` runnable to the `scheduledExec`
- `buildRunTask` method should check and submit the `RunNotice` only if the
state of the supervisor is not STOPPING
- `stop()` can call `scheduledExec.shutdown()` instead of
`scheduledExec.shutdownNow()`
Another alternative is to simply create a `shutdownExec` inside `stopAsync`.
Difference from the current approach would be that the `SupervisorManager`
doesn't need to handle the lifecycle of the shutdown executor.
Let me know what you think.
--
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]
