ayushkul2910 commented on a change in pull request #10448:
URL: https://github.com/apache/druid/pull/10448#discussion_r553162349
##########
File path:
server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java
##########
@@ -108,10 +108,10 @@ public MonitorScheduler getMonitorScheduler(
return new MonitorScheduler(
config.get(),
-
CronScheduler.newBuilder(Duration.ofSeconds(1L)).setThreadName("MonitorScheduler-%s").build(),
+
CronScheduler.newBuilder(Duration.ofSeconds(1L)).setThreadName("MonitorSchedulerThread").build(),
emitter,
monitors,
- Executors.newCachedThreadPool()
+ Execs.multiThreaded(64, "MonitorThread-%d")
Review comment:
Hi @jihoonson , sorry for delay in response.
I think currently there are ~20 monitors, which can run concurrently with
the MonitorScheduler class. Suppose a case in which frequency of scheduling <
time taken by the executor thread to do `monitor.monitor(...)`(Although I am
not sure if this case is possible in practical, kind of edge case). This can
result in queuing of the tasks if threads are very less. I think we should
atleast have no. of threads equal to max number of monitors supported. I may be
missing something here. What do 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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
