walterddr commented on code in PR #11023:
URL: https://github.com/apache/pinot/pull/11023#discussion_r1263767555
##########
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:
just a quick thought. do we need to handle the "group-limit" metrics
emission on broker? cant we do this on the servers and would that make more
sense?
- upon checking the group-by reducer code on broker it doesn't actually
apply the group limit,
- only place it is applied and trimmed result is on server group-by-operator
and combine
- v2 intermediate stage also will apply limit but at that point the "table"
info is lost.
does it make sense to
1. directly emit the metrics on server?
2. do not allow group-limit to be apply on intermediate stage?
--
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]
