siddharthteotia commented on a change in pull request #6886:
URL: https://github.com/apache/incubator-pinot/pull/6886#discussion_r630590682



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -127,15 +128,23 @@ public void runJob() {
       // Deregister the main thread and wait for all threads done
       phaser.awaitAdvance(phaser.arriveAndDeregister());
     }
-    CombineOperatorUtils.setExecutionStatistics(mergedBlock, _operators, 
totalWorkerThreadCpuTimeNs.get());
+    /*
+     * _numTasks are number of async tasks submitted to the _executorService, 
but it does not mean Pinot server
+     * use those number of threads to concurrently process segments. Instead, 
if _executorService thread pool has
+     * less number of threads than _numTasks, the number of threads that used 
to concurrently process segments equals
+     * to the pool size.
+     */
+    int numServerThreads = Math.min(_numTasks, 
ResourceManager.DEFAULT_QUERY_WORKER_THREADS);

Review comment:
       Default should not be used. From the executor service, can we get the 
number of threads it was configured with ?




-- 
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to