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.
--
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
