yashmayya commented on code in PR #13628:
URL: https://github.com/apache/pinot/pull/13628#discussion_r1685961517


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -478,8 +481,17 @@ protected BrokerResponse handleRequest(long requestId, 
String query, @Nullable S
       }
 
       _brokerMetrics.addMeteredTableValue(rawTableName, BrokerMeter.QUERIES, 
1);
+      _brokerMetrics.addMeteredGlobalValue(BrokerMeter.QUERIES_GLOBAL, 1);
       _brokerMetrics.addValueToTableGauge(rawTableName, 
BrokerGauge.REQUEST_SIZE, query.length());
 
+      if (!pinotQuery.isExplain() && _enableMultistageMigrationMetric) {

Review Comment:
   Thanks for the suggestions! I do agree that we need to be careful about 
affecting query latency in the v1 engine due to this additional query 
compilation overhead. I liked your second option more because the first option 
can still introduce unpredictable latency in some queries which can increase 
the tail latency by a fair amount. I did some ad-hoc benchmarking and for some 
queries, the v2 engine query compilation time is actually comparable in order 
of magnitude to the overall v1 query execution time (and I'm sure in certain 
cases it could even be higher).
   
   Also, I'm not a big fan of adding more user configuration than is necessary. 
I've gone with the second option for now - background thread processing (v2 
query compiling) a queue of v1 query requests where the queue capacity is 
currently hardcoded / not configurable. We could make this queue capacity 
configurable in the future if we see a need for allowing this to be 
configurable by users - i.e., in order to tune the percentage of requests that 
are effectively sampled based on the query workload.



-- 
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

Reply via email to