Skip to content

MQE improve memory consumption estimate with Label information #11203

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

lamida
Copy link
Contributor

@lamida lamida commented Apr 13, 2025

What this PR does

Add memory consumption estimate with Label information. The estimation is based on the following:

  • The size of labels.Label struct (32 bytes).
  • Average length of string name of value of the label (8 bytes each).
  • Average number of labels per series (guesstimating to 10).

I will try to figure out to improve the guesstimate of the two averages above by looking at the ops data.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

lamida added 6 commits April 14, 2025 05:09
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
@lamida lamida changed the title MQE improve memory consumption estimate for label MQE improve memory consumption estimate with Label information Apr 13, 2025
lamida added 4 commits April 14, 2025 07:27
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
Signed-off-by: Jon Kartago Lamida <me@lamida.net>
@lamida lamida marked this pull request as ready for review April 14, 2025 00:17
@lamida lamida requested a review from a team as a code owner April 14, 2025 00:17
Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite what I had in mind: I think we need to accurately account for the size of labels given they can vary wildly, and we need to track this for anything that contains labels (SeriesMetadata slices being the main missing source not yet accounted for).

Signed-off-by: Jon Kartago Lamida <me@lamida.net>
@lamida
Copy link
Contributor Author

lamida commented Apr 14, 2025

@charleskorn

I think we need to accurately account for the size of labels given they can vary wildly

Yes I am thinking about this too, but I started only with this estimate, because to track the size labels accurately, it means we need to move the call to types.VectorPool.Get(len(series), q.memoryConsumptionTracker) from its current place in populateVectorFromInstantVectorOperator. Now we just get the pool based on the count of series. But to account the exact size of labels, we need to move this or having additional place to calculate it at the point we load the series. Is this inline with what you have in mind for handling labels size variation precisely? I have a small worry on the performance impact if we want to measure the labels counts and the exact string size for each labels and label names though.

SeriesMetadata slices being the main missing source not yet accounted for

To be honest, I didn't think about this. Will add this. Do you think there is additional place where labels are used apart of SeriesMetadata and the time when we load sample?

@lamida lamida marked this pull request as draft April 14, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants