From 7bbf24b707966f6577e37beb95634bc58bae4f42 Mon Sep 17 00:00:00 2001 From: Justin Lei Date: Sun, 30 Apr 2023 13:13:25 -0700 Subject: [PATCH] Make MemoizedSeriesIterator not implement chunkenc.Iterator Signed-off-by: Justin Lei --- promql/engine.go | 2 +- storage/memoized_iterator.go | 35 ++++++++------------- storage/memoized_iterator_test.go | 52 ++++++++++++------------------- 3 files changed, 34 insertions(+), 55 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index 688048e7a..ae46f6005 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -1850,7 +1850,7 @@ func (ev *evaluator) vectorSelectorSingle(it *storage.MemoizedSeriesIterator, no } case chunkenc.ValFloat: t, v = it.At() - case chunkenc.ValHistogram, chunkenc.ValFloatHistogram: + case chunkenc.ValFloatHistogram: t, h = it.AtFloatHistogram() default: panic(fmt.Errorf("unknown value type %v", valueType)) diff --git a/storage/memoized_iterator.go b/storage/memoized_iterator.go index 465afa91c..cb9fdeef4 100644 --- a/storage/memoized_iterator.go +++ b/storage/memoized_iterator.go @@ -21,6 +21,9 @@ import ( ) // MemoizedSeriesIterator wraps an iterator with a buffer to look back the previous element. +// +// This iterator regards integer histograms as float histograms; calls to Seek() will never return chunkenc.Histogram. +// This iterator deliberately does not implement chunkenc.Iterator. type MemoizedSeriesIterator struct { it chunkenc.Iterator delta int64 @@ -78,8 +81,11 @@ func (b *MemoizedSeriesIterator) Seek(t int64) chunkenc.ValueType { b.prevTime = math.MinInt64 b.valueType = b.it.Seek(t0) - if b.valueType == chunkenc.ValNone { + switch b.valueType { + case chunkenc.ValNone: return chunkenc.ValNone + case chunkenc.ValHistogram: + b.valueType = chunkenc.ValFloatHistogram } b.lastTime = b.it.AtT() } @@ -95,7 +101,8 @@ func (b *MemoizedSeriesIterator) Seek(t int64) chunkenc.ValueType { return chunkenc.ValNone } -// Next advances the iterator to the next element. +// Next advances the iterator to the next element. Note that this does not check whether the element being buffered is +// within the time range of the current element and the duration of delta before. func (b *MemoizedSeriesIterator) Next() chunkenc.ValueType { // Keep track of the previous element. switch b.valueType { @@ -104,12 +111,7 @@ func (b *MemoizedSeriesIterator) Next() chunkenc.ValueType { case chunkenc.ValFloat: b.prevTime, b.prevValue = b.it.At() b.prevFloatHistogram = nil - case chunkenc.ValHistogram: - b.prevValue = 0 - ts, h := b.it.AtHistogram() - b.prevTime = ts - b.prevFloatHistogram = h.ToFloat() - case chunkenc.ValFloatHistogram: + case chunkenc.ValHistogram, chunkenc.ValFloatHistogram: b.prevValue = 0 b.prevTime, b.prevFloatHistogram = b.it.AtFloatHistogram() } @@ -118,6 +120,9 @@ func (b *MemoizedSeriesIterator) Next() chunkenc.ValueType { if b.valueType != chunkenc.ValNone { b.lastTime = b.it.AtT() } + if b.valueType == chunkenc.ValHistogram { + b.valueType = chunkenc.ValFloatHistogram + } return b.valueType } @@ -126,25 +131,11 @@ func (b *MemoizedSeriesIterator) At() (int64, float64) { return b.it.At() } -// AtHistogram is not supported by this iterator. -func (b *MemoizedSeriesIterator) AtHistogram() (int64, *histogram.Histogram) { - panic("AtHistogram is not supported by this iterator.") -} - // AtFloatHistogram returns the current float-histogram element of the iterator. func (b *MemoizedSeriesIterator) AtFloatHistogram() (int64, *histogram.FloatHistogram) { - if b.valueType == chunkenc.ValHistogram { - ts, h := b.it.AtHistogram() - return ts, h.ToFloat() - } return b.it.AtFloatHistogram() } -// AtT returns the current timestamp of the iterator. -func (b *MemoizedSeriesIterator) AtT() int64 { - return b.it.AtT() -} - // Err returns the last encountered error. func (b *MemoizedSeriesIterator) Err() error { return b.it.Err() diff --git a/storage/memoized_iterator_test.go b/storage/memoized_iterator_test.go index f922aaeec..1c8711928 100644 --- a/storage/memoized_iterator_test.go +++ b/storage/memoized_iterator_test.go @@ -16,11 +16,11 @@ package storage import ( "testing" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/tsdb/chunkenc" "github.com/prometheus/prometheus/tsdb/tsdbutil" - - "github.com/stretchr/testify/require" ) func TestMemoizedSeriesIterator(t *testing.T) { @@ -64,55 +64,43 @@ func TestMemoizedSeriesIterator(t *testing.T) { hSample{t: 200, h: tsdbutil.GenerateTestHistogram(4)}, fhSample{t: 299, fh: tsdbutil.GenerateTestFloatHistogram(5)}, fSample{t: 300, f: 11}, + hSample{t: 399, h: tsdbutil.GenerateTestHistogram(6)}, + fSample{t: 400, f: 12}, }), 2) require.Equal(t, it.Seek(-123), chunkenc.ValFloat, "seek failed") sampleEq(1, 2, nil) prevSampleEq(0, 0, nil, false) - require.Equal(t, it.Next(), chunkenc.ValFloat, "next failed") - sampleEq(2, 3, nil) - prevSampleEq(1, 2, nil, true) - - require.Equal(t, it.Next(), chunkenc.ValFloat, "next failed") - require.Equal(t, it.Next(), chunkenc.ValFloat, "next failed") - require.Equal(t, it.Next(), chunkenc.ValFloat, "next failed") - sampleEq(5, 6, nil) - prevSampleEq(4, 5, nil, true) - require.Equal(t, it.Seek(5), chunkenc.ValFloat, "seek failed") sampleEq(5, 6, nil) prevSampleEq(4, 5, nil, true) - require.Equal(t, it.Seek(101), chunkenc.ValFloat, "seek failed") - sampleEq(101, 10, nil) - prevSampleEq(100, 9, nil, true) - - require.Equal(t, chunkenc.ValHistogram, it.Next(), "next failed") - sampleEq(102, 0, tsdbutil.GenerateTestFloatHistogram(0)) + // Seek to a histogram sample with a previous float sample. + require.Equal(t, it.Seek(102), chunkenc.ValFloatHistogram, "seek failed") + sampleEq(102, 10, tsdbutil.GenerateTestFloatHistogram(0)) prevSampleEq(101, 10, nil, true) - require.Equal(t, chunkenc.ValHistogram, it.Next(), "next failed") - sampleEq(103, 0, tsdbutil.GenerateTestFloatHistogram(1)) - prevSampleEq(102, 0, tsdbutil.GenerateTestFloatHistogram(0), true) + // Attempt to seek backwards (no-op). + require.Equal(t, it.Seek(50), chunkenc.ValFloatHistogram, "seek failed") + sampleEq(102, 10, tsdbutil.GenerateTestFloatHistogram(0)) + prevSampleEq(101, 10, nil, true) - require.Equal(t, chunkenc.ValFloatHistogram, it.Seek(104), "seek failed") + // Seek to a float histogram sample with a previous histogram sample. + require.Equal(t, it.Seek(104), chunkenc.ValFloatHistogram, "seek failed") sampleEq(104, 0, tsdbutil.GenerateTestFloatHistogram(2)) prevSampleEq(103, 0, tsdbutil.GenerateTestFloatHistogram(1), true) - require.Equal(t, chunkenc.ValFloatHistogram, it.Next(), "next failed") - sampleEq(199, 0, tsdbutil.GenerateTestFloatHistogram(3)) - prevSampleEq(104, 0, tsdbutil.GenerateTestFloatHistogram(2), true) - - require.Equal(t, chunkenc.ValFloatHistogram, it.Seek(280), "seek failed") - sampleEq(299, 0, tsdbutil.GenerateTestFloatHistogram(5)) - prevSampleEq(0, 0, nil, false) - - require.Equal(t, chunkenc.ValFloat, it.Next(), "next failed") + // Seek to a float sample with a previous float histogram sample. + require.Equal(t, chunkenc.ValFloat, it.Seek(300), "seek failed") sampleEq(300, 11, nil) prevSampleEq(299, 0, tsdbutil.GenerateTestFloatHistogram(5), true) - require.Equal(t, it.Next(), chunkenc.ValNone, "next succeeded unexpectedly") + // Seek to a float sample with a previous histogram sample. + require.Equal(t, chunkenc.ValFloat, it.Seek(400), "seek failed") + sampleEq(400, 12, nil) + prevSampleEq(399, 0, tsdbutil.GenerateTestFloatHistogram(6), true) + require.Equal(t, it.Seek(1024), chunkenc.ValNone, "seek succeeded unexpectedly") }