mirror of
https://github.com/prometheus/prometheus.git
synced 2026-05-28 04:02:21 -04:00
promql: Optimize HistogramStatsIterator by disallowing integer histograms
The `HistogramStatsIterator` is only meant to be used within PromQL. PromQL only ever uses float histograms. If `HistogramStatsIterator` is capable of handling integer histograms, it will still be used, for example by the `BufferedSeriesIterator`, which buffers samples and will use an integer `Histogram` for it, if the underlying chunk is an integer histogram chunk (which is common). However, we can simply intercept the `Next` and `Seek` calls and pretend to only ever be able te return float histograms. This has the welcome side effect that we do not have to handle a mix of float and integer histograms in the `HistogramStatsIterator` anymore. With this commit, the `AtHistogram` call has been changed to panic so that we ensure it is never called. Benchmark differences between this and the previous commit: name old time/op new time/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 837ms ± 3% 616ms ± 2% -26.36% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 1.11s ± 1% 0.91s ± 3% -17.75% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 751ms ± 6% 581ms ± 1% -22.63% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 1.13s ±11% 0.85s ± 2% -24.59% (p=0.008 n=5+5) name old alloc/op new alloc/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 531MB ± 0% 148MB ± 0% -72.08% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 528MB ± 0% 145MB ± 0% -72.60% (p=0.016 n=5+4) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 452MB ± 0% 145MB ± 0% -67.97% (p=0.016 n=5+4) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 452MB ± 0% 141MB ± 0% -68.70% (p=0.016 n=5+4) name old allocs/op new allocs/op delta NativeHistograms/histogram_count_with_short_rate_interval-16 8.95M ± 0% 1.60M ± 0% -82.15% (p=0.008 n=5+5) NativeHistograms/histogram_count_with_long_rate_interval-16 8.84M ± 0% 1.49M ± 0% -83.16% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_short_rate_interval-16 5.96M ± 0% 1.57M ± 0% -73.68% (p=0.008 n=5+5) NativeHistogramsCustomBuckets/histogram_count_with_long_rate_interval-16 5.86M ± 0% 1.46M ± 0% -75.05% (p=0.016 n=5+4) Signed-off-by: beorn7 <beorn@grafana.com>
This commit is contained in:
beorn7
parent
87d7c12563
commit
5010bd4bb1
2 changed files with 72 additions and 151 deletions
|
|
@ -19,27 +19,24 @@ import (
|
|||
"github.com/prometheus/prometheus/tsdb/chunkenc"
|
||||
)
|
||||
|
||||
// HistogramStatsIterator is an iterator that returns histogram objects
|
||||
// which have only their sum and count values populated. The iterator handles
|
||||
// counter reset detection internally and sets the counter reset hint accordingly
|
||||
// in each returned histogram object.
|
||||
// HistogramStatsIterator is an iterator that returns histogram objects that
|
||||
// have only their sum and count values populated. The iterator handles counter
|
||||
// reset detection internally and sets the counter reset hint accordingly in
|
||||
// each returned histogram object. The Next and Seek methods of the iterator
|
||||
// will never return ValHistogram, but ValFloatHistogram instead. Effectively,
|
||||
// the iterator enforces conversion of (integer) Histogram to FloatHistogram.
|
||||
// The AtHistogram method must not be called (and will panic).
|
||||
type HistogramStatsIterator struct {
|
||||
chunkenc.Iterator
|
||||
|
||||
currentH *histogram.Histogram
|
||||
lastH *histogram.Histogram
|
||||
|
||||
currentFH *histogram.FloatHistogram
|
||||
lastFH *histogram.FloatHistogram
|
||||
|
||||
currentSeriesRead bool
|
||||
}
|
||||
|
||||
// NewHistogramStatsIterator creates a new HistogramStatsIterator.
|
||||
func NewHistogramStatsIterator(it chunkenc.Iterator) *HistogramStatsIterator {
|
||||
return &HistogramStatsIterator{
|
||||
Iterator: it,
|
||||
currentH: &histogram.Histogram{},
|
||||
currentFH: &histogram.FloatHistogram{},
|
||||
}
|
||||
}
|
||||
|
|
@ -48,44 +45,38 @@ func NewHistogramStatsIterator(it chunkenc.Iterator) *HistogramStatsIterator {
|
|||
// objects already allocated where possible.
|
||||
func (hsi *HistogramStatsIterator) Reset(it chunkenc.Iterator) {
|
||||
hsi.Iterator = it
|
||||
hsi.currentSeriesRead = false
|
||||
hsi.lastFH = nil
|
||||
}
|
||||
|
||||
// AtHistogram returns the next timestamp/histogram pair. The counter reset
|
||||
// detection is guaranteed to be correct only when the caller does not switch
|
||||
// between AtHistogram and AtFloatHistogram calls.
|
||||
func (hsi *HistogramStatsIterator) AtHistogram(h *histogram.Histogram) (int64, *histogram.Histogram) {
|
||||
var t int64
|
||||
t, hsi.currentH = hsi.Iterator.AtHistogram(hsi.currentH)
|
||||
if value.IsStaleNaN(hsi.currentH.Sum) {
|
||||
h = &histogram.Histogram{Sum: hsi.currentH.Sum}
|
||||
return t, h
|
||||
// Next mostly relays to the underlying iterator, but changes a ValHistogram
|
||||
// return into a ValFloatHistogram return.
|
||||
func (hsi *HistogramStatsIterator) Next() chunkenc.ValueType {
|
||||
vt := hsi.Iterator.Next()
|
||||
if vt == chunkenc.ValHistogram {
|
||||
return chunkenc.ValFloatHistogram
|
||||
}
|
||||
|
||||
if h == nil {
|
||||
h = &histogram.Histogram{
|
||||
CounterResetHint: hsi.getResetHint(hsi.currentH),
|
||||
Count: hsi.currentH.Count,
|
||||
Sum: hsi.currentH.Sum,
|
||||
}
|
||||
hsi.setLastH(hsi.currentH)
|
||||
return t, h
|
||||
}
|
||||
|
||||
returnValue := histogram.Histogram{
|
||||
CounterResetHint: hsi.getResetHint(hsi.currentH),
|
||||
Count: hsi.currentH.Count,
|
||||
Sum: hsi.currentH.Sum,
|
||||
}
|
||||
returnValue.CopyTo(h)
|
||||
|
||||
hsi.setLastH(hsi.currentH)
|
||||
return t, h
|
||||
return vt
|
||||
}
|
||||
|
||||
// AtFloatHistogram returns the next timestamp/float histogram pair. The counter
|
||||
// reset detection is guaranteed to be correct only when the caller does not
|
||||
// switch between AtHistogram and AtFloatHistogram calls.
|
||||
// Seek mostly relays to the underlying iterator, but changes a ValHistogram
|
||||
// return into a ValFloatHistogram return.
|
||||
func (hsi *HistogramStatsIterator) Seek(t int64) chunkenc.ValueType {
|
||||
vt := hsi.Iterator.Seek(t)
|
||||
if vt == chunkenc.ValHistogram {
|
||||
return chunkenc.ValFloatHistogram
|
||||
}
|
||||
return vt
|
||||
}
|
||||
|
||||
// AtHistogram must never be called.
|
||||
func (*HistogramStatsIterator) AtHistogram(*histogram.Histogram) (int64, *histogram.Histogram) {
|
||||
panic("HistogramStatsIterator.AtHistogram must never be called")
|
||||
}
|
||||
|
||||
// AtFloatHistogram returns the next timestamp/float histogram pair. The method
|
||||
// performs a counter reset detection on the fly. It will return an explicit
|
||||
// hint (not UnknownCounterReset) if the previous sample has been accessed with
|
||||
// the same iterator.
|
||||
func (hsi *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) {
|
||||
var t int64
|
||||
t, hsi.currentFH = hsi.Iterator.AtFloatHistogram(hsi.currentFH)
|
||||
|
|
@ -114,26 +105,12 @@ func (hsi *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram
|
|||
return t, fh
|
||||
}
|
||||
|
||||
func (hsi *HistogramStatsIterator) setLastH(h *histogram.Histogram) {
|
||||
hsi.lastFH = nil
|
||||
if hsi.lastH == nil {
|
||||
hsi.lastH = h.Copy()
|
||||
} else {
|
||||
h.CopyTo(hsi.lastH)
|
||||
}
|
||||
|
||||
hsi.currentSeriesRead = true
|
||||
}
|
||||
|
||||
func (hsi *HistogramStatsIterator) setLastFH(fh *histogram.FloatHistogram) {
|
||||
hsi.lastH = nil
|
||||
if hsi.lastFH == nil {
|
||||
hsi.lastFH = fh.Copy()
|
||||
} else {
|
||||
fh.CopyTo(hsi.lastFH)
|
||||
}
|
||||
|
||||
hsi.currentSeriesRead = true
|
||||
}
|
||||
|
||||
func (hsi *HistogramStatsIterator) getFloatResetHint(hint histogram.CounterResetHint) histogram.CounterResetHint {
|
||||
|
|
@ -141,44 +118,19 @@ func (hsi *HistogramStatsIterator) getFloatResetHint(hint histogram.CounterReset
|
|||
return hint
|
||||
}
|
||||
prevFH := hsi.lastFH
|
||||
if prevFH == nil || !hsi.currentSeriesRead {
|
||||
if hsi.lastH == nil || !hsi.currentSeriesRead {
|
||||
// We don't know if there's a counter reset.
|
||||
return histogram.UnknownCounterReset
|
||||
}
|
||||
prevFH = hsi.lastH.ToFloat(nil)
|
||||
if prevFH == nil {
|
||||
// We don't know if there's a counter reset. Note that this
|
||||
// generally will trigger an explicit counter reset detection by
|
||||
// the PromQL engine, which in turn isn't as reliable in this
|
||||
// case because the PromQL engine will not see the buckets.
|
||||
// However, we can assume that in cases where the counter reset
|
||||
// detection is relevant, an iteration through the series has
|
||||
// happened, and therefore we do not end up here in the first
|
||||
// place.
|
||||
return histogram.UnknownCounterReset
|
||||
}
|
||||
if hsi.currentFH.DetectReset(prevFH) {
|
||||
return histogram.CounterReset
|
||||
}
|
||||
return histogram.NotCounterReset
|
||||
}
|
||||
|
||||
func (hsi *HistogramStatsIterator) getResetHint(h *histogram.Histogram) histogram.CounterResetHint {
|
||||
if h.CounterResetHint != histogram.UnknownCounterReset {
|
||||
return h.CounterResetHint
|
||||
}
|
||||
var prevFH *histogram.FloatHistogram
|
||||
if hsi.lastH == nil || !hsi.currentSeriesRead {
|
||||
if hsi.lastFH == nil || !hsi.currentSeriesRead {
|
||||
// We don't know if there's a counter reset. Note that
|
||||
// this generally will trigger an explicit counter reset
|
||||
// detection by the PromQL engine, which in turn isn't
|
||||
// as reliable in this case because the PromQL engine
|
||||
// will not see the buckets. However, we can assume that
|
||||
// in cases where the counter reset detection is
|
||||
// relevant, an iteration through the series has
|
||||
// happened, and therefore we do not end up here in the
|
||||
// first place.
|
||||
return histogram.UnknownCounterReset
|
||||
}
|
||||
prevFH = hsi.lastFH
|
||||
} else {
|
||||
prevFH = hsi.lastH.ToFloat(nil)
|
||||
}
|
||||
fh := h.ToFloat(nil)
|
||||
if fh.DetectReset(prevFH) {
|
||||
return histogram.CounterReset
|
||||
}
|
||||
return histogram.NotCounterReset
|
||||
}
|
||||
|
|
|
|||
|
|
@ -114,64 +114,32 @@ func TestHistogramStatsDecoding(t *testing.T) {
|
|||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Run("histogram_stats", func(t *testing.T) {
|
||||
check := func(statsIterator *HistogramStatsIterator) {
|
||||
decodedStats := make([]*histogram.Histogram, 0)
|
||||
|
||||
for statsIterator.Next() != chunkenc.ValNone {
|
||||
_, h := statsIterator.AtHistogram(nil)
|
||||
decodedStats = append(decodedStats, h)
|
||||
}
|
||||
|
||||
for i := 0; i < len(tc.histograms); i++ {
|
||||
require.Equalf(t, tc.expectedHints[i], decodedStats[i].CounterResetHint, "mismatch in counter reset hint for histogram %d", i)
|
||||
h := tc.histograms[i]
|
||||
if value.IsStaleNaN(h.Sum) {
|
||||
require.True(t, value.IsStaleNaN(decodedStats[i].Sum))
|
||||
require.Equal(t, uint64(0), decodedStats[i].Count)
|
||||
} else {
|
||||
require.Equal(t, tc.histograms[i].Count, decodedStats[i].Count)
|
||||
require.Equal(t, tc.histograms[i].Sum, decodedStats[i].Sum)
|
||||
}
|
||||
check := func(statsIterator *HistogramStatsIterator) {
|
||||
decodedStats := make([]*histogram.FloatHistogram, 0)
|
||||
for statsIterator.Next() != chunkenc.ValNone {
|
||||
_, h := statsIterator.AtFloatHistogram(nil)
|
||||
decodedStats = append(decodedStats, h)
|
||||
}
|
||||
for i := 0; i < len(tc.histograms); i++ {
|
||||
require.Equal(t, tc.expectedHints[i], decodedStats[i].CounterResetHint)
|
||||
fh := tc.histograms[i].ToFloat(nil)
|
||||
if value.IsStaleNaN(fh.Sum) {
|
||||
require.True(t, value.IsStaleNaN(decodedStats[i].Sum))
|
||||
require.Equal(t, float64(0), decodedStats[i].Count)
|
||||
} else {
|
||||
require.Equal(t, fh.Count, decodedStats[i].Count)
|
||||
require.Equal(t, fh.Sum, decodedStats[i].Sum)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check that we get the expected results with a fresh iterator.
|
||||
statsIterator := NewHistogramStatsIterator(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
// Check that we get the expected results with a fresh iterator.
|
||||
statsIterator := NewHistogramStatsIterator(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
|
||||
// Check that we get the same results if we reset and reuse that iterator.
|
||||
statsIterator.Reset(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
})
|
||||
t.Run("float_histogram_stats", func(t *testing.T) {
|
||||
check := func(statsIterator *HistogramStatsIterator) {
|
||||
decodedStats := make([]*histogram.FloatHistogram, 0)
|
||||
for statsIterator.Next() != chunkenc.ValNone {
|
||||
_, h := statsIterator.AtFloatHistogram(nil)
|
||||
decodedStats = append(decodedStats, h)
|
||||
}
|
||||
for i := 0; i < len(tc.histograms); i++ {
|
||||
require.Equal(t, tc.expectedHints[i], decodedStats[i].CounterResetHint)
|
||||
fh := tc.histograms[i].ToFloat(nil)
|
||||
if value.IsStaleNaN(fh.Sum) {
|
||||
require.True(t, value.IsStaleNaN(decodedStats[i].Sum))
|
||||
require.Equal(t, float64(0), decodedStats[i].Count)
|
||||
} else {
|
||||
require.Equal(t, fh.Count, decodedStats[i].Count)
|
||||
require.Equal(t, fh.Sum, decodedStats[i].Sum)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check that we get the expected results with a fresh iterator.
|
||||
statsIterator := NewHistogramStatsIterator(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
|
||||
// Check that we get the same results if we reset and reuse that iterator.
|
||||
statsIterator.Reset(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
})
|
||||
// Check that we get the same results if we reset and reuse that iterator.
|
||||
statsIterator.Reset(newHistogramSeries(tc.histograms).Iterator(nil))
|
||||
check(statsIterator)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -193,17 +161,18 @@ func TestHistogramStatsMixedUse(t *testing.T) {
|
|||
histogram.NotCounterReset,
|
||||
histogram.CounterReset,
|
||||
}
|
||||
// Note that statsIterator always returns float histograms.
|
||||
actualHints := make([]histogram.CounterResetHint, 3)
|
||||
typ := statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
_, h := statsIterator.AtHistogram(nil)
|
||||
require.Equal(t, chunkenc.ValFloatHistogram, typ)
|
||||
_, h := statsIterator.AtFloatHistogram(nil)
|
||||
actualHints[0] = h.CounterResetHint
|
||||
typ = statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
_, h = statsIterator.AtHistogram(nil)
|
||||
require.Equal(t, chunkenc.ValFloatHistogram, typ)
|
||||
_, h = statsIterator.AtFloatHistogram(nil)
|
||||
actualHints[1] = h.CounterResetHint
|
||||
typ = statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
require.Equal(t, chunkenc.ValFloatHistogram, typ)
|
||||
_, fh := statsIterator.AtFloatHistogram(nil)
|
||||
actualHints[2] = fh.CounterResetHint
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue