tibrewalpratik17 commented on code in PR #12982:
URL: https://github.com/apache/pinot/pull/12982#discussion_r1574511131


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -309,7 +309,7 @@ protected BrokerResponse handleRequest(long requestId, 
String query, @Nullable S
       JsonNode request, @Nullable RequesterIdentity requesterIdentity, 
RequestContext requestContext,
       @Nullable HttpHeaders httpHeaders)
       throws Exception {
-    LOGGER.debug("SQL query for request {}: {}", requestId, query);
+    LOGGER.info("SQL query for request {}: {}", requestId, query);

Review Comment:
   This will add a lot of log lines for high-qps use-cases.



##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -260,10 +276,18 @@ protected BrokerResponse handleRequest(long requestId, 
String query, @Nullable S
     brokerResponse.setPartialResult(isPartialResult(brokerResponse));
 
     // Set total query processing time
-    // TODO: Currently we don't emit metric for QUERY_TOTAL_TIME_MS
     long totalTimeMs = TimeUnit.NANOSECONDS.toMillis(
         sqlNodeAndOptions.getParseTimeNs() + (executionEndTimeNs - 
compilationStartTimeNs));
     brokerResponse.setTimeUsedMs(totalTimeMs);
+    if (brokerResponse.isNumGroupsLimitReached()) {
+      for (String tableName : tableNames) {
+        _brokerMetrics.addMeteredTableValue(tableName, 
BrokerMeter.BROKER_RESPONSES_WITH_NUM_GROUPS_LIMIT_REACHED, 1);
+      }

Review Comment:
   We were planning to do something similar before and had some reservations 
around this. More info on this thread -- 
https://github.com/apache/pinot/pull/11023#discussion_r1248823295
   cc @ankitsultana 



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