[GitHub] [druid] gianm commented on issue #12022: [Proposal] Improve memory estimations in `OnHeapIncrementalIndex`

[GitBox <gi...@apache.org>]

gianm commented on issue #12022:
URL: https://github.com/apache/druid/issues/12022#issuecomment-985596977


   > Workaround (Rejected):
   > To retain compatibilty, we could add a new default long aggregateAndEstimateMemory() method and leave the existing aggregate() method as is. The default implementation would return a negative value (say -1) in which case the OnHeapIncrementalIndex or any other estimator would use the max estimated size for that invocation.
   
   This approach actually seems like a good idea to me for a few reasons:
   
   - Aggregation is hot code, and there will be some overhead to size estimation, and query-time callers don't necessarily need it. So having two methods allows the caller to decide if it actually needs size estimates or not.
   - We want to avoid incompatible changes to aggregation interfaces whenever possible. 


-- 
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-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org