tibrewalpratik17 commented on code in PR #11023:
URL: https://github.com/apache/pinot/pull/11023#discussion_r1260193410
##########
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:
> oh. sorry i completely misunderstood your intent for this PR. so the goal
is to emit a metrics not response metadata from broker returned to user?
Yes we want a metric to be emitted based on `isNumGroupsLimitReached`
field. We do it for v1 right now.
> if that's the case you will have to attach some meaningful rawTableName
here. otherwise simply a null is not differentiating for stats purpose.
As Ankit mentioned that we might emit metric twice if doing it inside the
for loop. Should we save the first table we are getting this metric for and
just emit based on that?
--
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
