Skip to content
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

Native histograms: optimize chunk iterator usage #7274

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

duricanikolic
Copy link
Contributor

What this PR does

Prometheus introduced a possibility to optionally reuse the memory area of the given histogram or float histogram to reduce allocation (prometheus/prometheus#13340). We highlight the functions from the chunkenc.Iterator interface, that are of particular importance for the above mentioned change:

type Iterator interface {
	...
	// AtHistogram returns the current timestamp/value pair if the value is a
	// histogram with integer counts. Before the iterator has advanced, the behaviour
	// is unspecified.
	// The method accepts an optional Histogram object which will be
	// reused when not nil. Otherwise, a new Histogram object will be allocated.
	AtHistogram(*histogram.Histogram) (int64, *histogram.Histogram)
	// AtFloatHistogram returns the current timestamp/value pair if the
	// value is a histogram with floating-point counts. It also works if the
	// value is a histogram with integer counts, in which case a
	// FloatHistogram copy of the histogram is returned. Before the iterator
	// has advanced, the behaviour is unspecified.
	// The method accepts an optional FloatHistogram object which will be
	// reused when not nil. Otherwise, a new FloatHistogram object will be allocated.
	AtFloatHistogram(*histogram.FloatHistogram) (int64, *histogram.FloatHistogram)
}

Before #7219, which allowed Mimir to use Prometheus' optimizations mentioned above, Mimir's querier/iterator.chunkIterator used to implement Prometheus' chunkenc.Iterator, but the signatures of querier/iterator.chunkIterator's AtHistogram() and AtFloatHistogram() were not enriched with the optional *histogram.Histogram and *histogram.FloatHistogram arguments in #7219. This PR fixes that problem, so that querier/iterator.chunkIterator is an implementation of chunkenc.Iterator again.

Moreover, this PR ensures that Mimir's querier/iterator.chunkIterator and querier/iterator.chunkMergeIterator, both implementations of the chunkenc.Iterator interace, reuse histograms and float histograms memory, analogously to what is done in Prometheus.

Which issue(s) this PR fixes or relates to

Part of #7235

Checklist

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

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
@duricanikolic duricanikolic self-assigned this Feb 2, 2024
@duricanikolic duricanikolic requested a review from a team as a code owner February 2, 2024 10:36
pkg/querier/iterators/chunk_iterator.go Outdated Show resolved Hide resolved
pkg/querier/iterators/chunk_iterator.go Outdated Show resolved Hide resolved
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

LGTM

@duricanikolic duricanikolic merged commit ae039a3 into main Feb 2, 2024
28 checks passed
@duricanikolic duricanikolic deleted the yuri/native-histograms-optimization branch February 2, 2024 13:35
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