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


##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -247,6 +247,12 @@ private BrokerResponse handleRequest(long requestId, 
String query, @Nullable Sql
         // find a way to split metrics in case of multiple table
         String rawTableName = 
TableNameBuilder.extractRawTableName(tableNames.iterator().next());
         entry.getValue().setStageLevelStats(rawTableName, brokerResponseStats, 
_brokerMetrics);
+
+        // Track number of queries with number of groups limit reached
+        if (brokerResponse.isNumGroupsLimitReached()) {

Review Comment:
   > do we need to track exactly which table triggers the group limit? cuz the 
flag is global. there's really not much a user can do with per-table insight, 
correct?
   
   Yeah this makes sense! We can just emit a global level metric in this case.
   
   > I don't have context about this piece of code but I see 
ExecutionStatsAggregation#setStats is already emitting a bunch of metrics. I 
think we can emit this groups limit metric there as well.
   The rawTableName is set to null right now which skips the metric emission 
there so we need to think about that.
   
   @ankitsultana I think we cannot reuse this method as at every stage, this is 
called and we would end up emitting the metric twice again.
   
   I think we can just maintain a flag that if in any one of the stages 
`isNumGroupsLimitReached` is true, we can emit a metric with `rawTableName` as 
null in this method itself outside the loop. 
   
   @ankitsultana @walterddr does this sound good?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to