albertobastos commented on code in PR #15280: URL: https://github.com/apache/pinot/pull/15280#discussion_r2016337952
########## pinot-core/src/main/java/org/apache/pinot/core/operator/query/FilteredGroupByOperator.java: ########## @@ -168,12 +170,16 @@ protected GroupByResultsBlock getNextBlock() { // Check if the groups limit is reached boolean numGroupsLimitReached = groupKeyGenerator.getNumKeys() >= _queryContext.getNumGroupsLimit(); + if (numGroupsLimitReached) { + ServerMetrics.get().addMeteredGlobalValue(ServerMeter.AGGREGATE_TIMES_NUM_GROUPS_LIMIT_REACHED, 1); + } Review Comment: After some testing this is the observed behaviour (launching a cluster with a numGroups limit so low it is always exceeded): - For each query against the SSE, the metric gets incremented once per each segment the table has. That makes sense, as the engine creates a worker for each segment and it is the worker the one that checks if the number of groups for the current segment is above the limit. - For each query against the MSE, the metric gets incremented the same amount **plus one**. That's because the `MultiStageOperator` stays on top of several Leaf operators that end up incrementing the metric themself (same as while using the SSE) and, on the way back, the `MultiStageOperator' increments it by one more unit. IMHO the `MultiStageOperator` should not touch this two metrics, as they are mantained from each worker. I removed it but left a comment explaining why, and now the metric gets updated consistently between SSE and MSE. Edit: the per-worker nature of this two metrics is already documented on `ServerMeter.java`. -- 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