kfaraz commented on a change in pull request #12073:
URL: https://github.com/apache/druid/pull/12073#discussion_r796304649
##########
File path:
processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java
##########
@@ -69,6 +69,24 @@ public VectorAggregator
factorizeVector(VectorColumnSelectorFactory selectorFact
throw new UOE("Aggregator[%s] cannot vectorize", getClass().getName());
}
+ /**
+ * Creates an {@link Aggregator} based on the provided column selector
factory.
+ * The returned value is a holder object which contains both the aggregator
+ * and its initial size in bytes. The callers can then invoke
+ * {@link Aggregator#aggregateWithSize()} to perform aggregation and get back
+ * the incremental memory required in each aggregate call. Combined with the
+ * initial size, this gives the total on-heap memory required by the
aggregator.
+ *
+ * This flow does not require invoking {@link
#guessAggregatorHeapFootprint(long)}
+ * which tends to over-estimate the required memory.
+ *
+ * @return AggregatorAndSize which contains the actual aggregator and its
initial size.
+ */
+ public AggregatorAndSize factorizeWithSize(ColumnSelectorFactory
metricFactory)
Review comment:
Updated the javadoc to advise on the required estimation.
> Also, I wonder if there is anything we could do to make sure this method
is overridden if aggregateWithSize is implemented
I guess it is okay even if it isn't overridden because we would only be
overestimating which would not cause failures, only somewhat poorer estimates.
--
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]
