[GitHub] [incubator-pinot] mqliang commented on a change in pull request #6680: Instrument combine operators with total thread time

[GitBox <gi...@apache.org>]

mqliang commented on a change in pull request #6680:
URL: https://github.com/apache/incubator-pinot/pull/6680#discussion_r594777195



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -89,12 +92,28 @@ protected IntermediateResultsBlock getNextBlock() {
       _futures[i] = _executorService.submit(new TraceRunnable() {
         @Override
         public void runJob() {
+          ThreadTimer processThreadTimer = new ThreadTimer();
+          processThreadTimer.start();
+
           processSegments(threadIndex);
+
+          processThreadTimer.stop();
+          totalWorkerTime.addAndGet(processThreadTimer.getThreadTime());
         }
       });
     }
 
+    ThreadTimer mergeThreadTimer = new ThreadTimer();

Review comment:
       If we instrument in that way (start the main thread timer in InstanceResponseOperator), then the total time will account an extra processing thread time interval:
   
   example:
   t0: InstanceResponseOperator call InstanceResponseBlock's constructor, which will call _operator.nextBlock();
   t2: process segments in parallel. let's say we have 5 thread processing segments in parallel. Let's assume all the 5 thread start and end at the same time.
   t3: processing end, start merge
   t4: merge end.
   
   NOTE: since time spending on constructor is negligible, let's assume t0=t2.
   
   Then, expected_totalTime = processing_time + merge_time =  (t3-t2)*5 + (t4-t3). However, if start the main thread timer in InstanceResponseOperator, actual_totalTime = t4-t0+(t3-t2)*5 = t4-t2+(t3-t2)*5
   
   actual_totalTime - expected_totalTime = (t4-t2)-(t4-t3) = t3-t2, in other word, there is an extra processing thread time get accounted.
   
   I will let @mcvsubbu chime in to make a double check.
     
   




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



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