shauryachats commented on issue #14685: URL: https://github.com/apache/pinot/issues/14685#issuecomment-2623264110
`numGroupsLimit` defines the maximum number of distinct groups that can be present in the GroupBy at a server level, whereas `maxInitialResultHolderCapacity` sets the initial capacity for the `mergeResultHolder` which stores the results after the groupBy has happened. It makes more sense for the new config to reserve `numGroupsLimit` rather than `maxInitialResultHolderCapacity`to eliminate redundantly resizes since we can expect the proposed config (`pinot.server.query.executor.group.generator.reserveMap`) would be enabled for cases where the group by key has high cardinality, and typically the `maxInitialResultHolderCapacity` would be less than `numGroupsLimit` which would result in reduced but still non-zero resizes which would significantly contribute to the latency. Ideally the `maxInitialResultHolderCapacity` should never be more than `numGroupsLimit` since the result holder cannot store more than the maximum number of groups specified by the config, hence we should also consider adding a check in Pinot for 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
