[GitHub] [pinot] ankitsultana commented on a diff in pull request #11023: [multistage] Add numGroupsLimitReached metrics to broker v2 queries

["ankitsultana (via GitHub)" <gi...@apache.org>]

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


##########
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:
   Say 3 tables were used in a join: `table_1, table_2, table_3`. If `table_2` was the one that had groups limit reached, the current logic will emit a metric for `table_2` and `table_3` since the same `brokerResponse` is written to in the loop.
   
   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.
   
   Maybe we should emit global counters instead of table level counters there?
   
   For Multistage queries, I am not sure if table level metrics make a lot of sense at broker level. My thought is that we can start with global metrics and then later think about table-level metrics.
   
   @walterddr lmk your thoughts.



-- 
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-unsubscribe@pinot.apache.org

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