Skip to content

Add memory accounting to exponential histogram library. #132580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JonasKunz
Copy link
Contributor

@JonasKunz JonasKunz commented Aug 8, 2025

Implements memory accounting (Accountable) and circuit breaker support for the exponential histogram library added in #131220. Just like the T-Digest library, the circuit breaker is abstracted away through an interface, so that no direct dependency on it is required for the lib.

This PR now defines for the ExponentialHistogramMerger that if the result is queried, the merger is cleared and the caller takes ownership of the returned histogram. Therefore the caller is then responsible for correctly calling close() on the returned histogram.

I was hesitant of doing it this way, because when implementing the ES|QL support this might cause unnecessary allocations:

E.g. for a query like | STATS PERCENTILE(MERGE(responseTime), 0.9) by service.name, TBUCKET(1m) it might be more efficient if we can reuse the ExponentialHistogramMerger across the aggregation groups. In this case this would mean merging all histograms for the first service.name, query the percentile and then continue with the next group. It would be ideal if the PERCENTILE could operate on a histogram instance "lent" from the merger, so that it can be reused then for the next group instead of requiring a new allocation.
However, if this is actually useful we can add this behaviour later when we get to the ES|QL implementation.

@JonasKunz JonasKunz added >non-issue :StorageEngine/Mapping The storage related side of mappings labels Aug 8, 2025
@JonasKunz JonasKunz marked this pull request as ready for review August 8, 2025 14:24
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants