vvivekiyer commented on code in PR #9697:
URL: https://github.com/apache/pinot/pull/9697#discussion_r1010909353
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManager.java:
##########
@@ -47,8 +48,13 @@ public class ServerRoutingStatsManager {
private final PinotConfiguration _config;
private volatile boolean _isEnabled;
private ConcurrentHashMap<String, ServerRoutingStatsEntry>
_serverQueryStatsMap;
+
+ // Main executor service for collecting and aggregating stats for all
servers.
private ExecutorService _executorService;
+ // ScheduledExecutorServer for processing periodic tasks like decay.
+ private ScheduledExecutorService _periodicTaskExecutor;
Review Comment:
The reasoning for not using the same executorService is as follows: The main
executor service is responsible for collecting stats about servers. The number
of periodic tasks is proportional to the number of servers. And we wouldn't
want it to interfere with stats collection - which is critical for making the
routing decision.
However, we can have a separate config to control the number of periodic
task threads. But I do not see us needing more than 1 thread for this purpose.
Let's say, autoDecay window is 30seconds. With 200 severs, we would be
generating 2*200 = 400 periodic tasks every 30 seconds. A single thread should
be sufficient to process these tasks.
I've also DM'ed you on the Apache Pinot slack channel.
Thoughts?
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
