diff --git a/tsdb/chunkenc/float_histogram.go b/tsdb/chunkenc/float_histogram.go index a5f123bc93..f18eb77dad 100644 --- a/tsdb/chunkenc/float_histogram.go +++ b/tsdb/chunkenc/float_histogram.go @@ -974,6 +974,7 @@ func (it *floatHistogramIterator) Reset(b []byte) { if it.atFloatHistogramCalled { it.atFloatHistogramCalled = false it.pBuckets, it.nBuckets = nil, nil + it.pSpans, it.nSpans = nil, nil } else { it.pBuckets, it.nBuckets = it.pBuckets[:0], it.nBuckets[:0] } @@ -1069,7 +1070,7 @@ func (it *floatHistogramIterator) Next() ValueType { // The case for the 2nd sample with single deltas is implicitly handled correctly with the double delta code, // so we don't need a separate single delta logic for the 2nd sample. - // Recycle bucket slices that have not been returned yet. Otherwise, copy them. + // Recycle bucket and span slices that have not been returned yet. Otherwise, copy them. // We can always recycle the slices for leading and trailing bits as they are // never returned to the caller. if it.atFloatHistogramCalled { @@ -1088,6 +1089,20 @@ func (it *floatHistogramIterator) Next() ValueType { } else { it.nBuckets = nil } + if len(it.pSpans) > 0 { + newSpans := make([]histogram.Span, len(it.pSpans)) + copy(newSpans, it.pSpans) + it.pSpans = newSpans + } else { + it.pSpans = nil + } + if len(it.nSpans) > 0 { + newSpans := make([]histogram.Span, len(it.nSpans)) + copy(newSpans, it.nSpans) + it.nSpans = newSpans + } else { + it.nSpans = nil + } } tDod, err := readVarbitInt(&it.br) diff --git a/tsdb/chunkenc/float_histogram_test.go b/tsdb/chunkenc/float_histogram_test.go index 689696f5ae..6092c0f636 100644 --- a/tsdb/chunkenc/float_histogram_test.go +++ b/tsdb/chunkenc/float_histogram_test.go @@ -1306,3 +1306,54 @@ func TestFloatHistogramAppendOnlyErrors(t *testing.T) { require.EqualError(t, err, "float histogram counter reset") }) } + +func TestFloatHistogramUniqueSpansAfterNext(t *testing.T) { + // Create two histograms with the same schema and spans. + h1 := &histogram.FloatHistogram{ + Schema: 1, + ZeroThreshold: 1e-100, + Count: 10, + ZeroCount: 2, + Sum: 15.0, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 2}, + {Offset: 1, Length: 2}, + }, + PositiveBuckets: []float64{1, 2, 3, 4}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 1}, + }, + NegativeBuckets: []float64{2}, + } + + h2 := h1.Copy() + + // Create a chunk and append both histograms. + c := NewFloatHistogramChunk() + app, err := c.Appender() + require.NoError(t, err) + + _, _, _, err = app.AppendFloatHistogram(nil, 0, h1, false) + require.NoError(t, err) + + _, _, _, err = app.AppendFloatHistogram(nil, 1, h2, false) + require.NoError(t, err) + + // Create an iterator and advance to the first histogram. + it := c.Iterator(nil) + require.Equal(t, ValFloatHistogram, it.Next()) + _, rh1 := it.AtFloatHistogram(nil) + + // Advance to the second histogram and retrieve it. + require.Equal(t, ValFloatHistogram, it.Next()) + _, rh2 := it.AtFloatHistogram(nil) + + require.Equal(t, rh1.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh1.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + require.Equal(t, rh2.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh2.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + + // Check that the spans for h1 and h2 are unique slices. + require.NotSame(t, &rh1.PositiveSpans[0], &rh2.PositiveSpans[0], "PositiveSpans should be unique between histograms") + require.NotSame(t, &rh1.NegativeSpans[0], &rh2.NegativeSpans[0], "NegativeSpans should be unique between histograms") +} diff --git a/tsdb/chunkenc/histogram.go b/tsdb/chunkenc/histogram.go index fafae48d3c..f8796d64ec 100644 --- a/tsdb/chunkenc/histogram.go +++ b/tsdb/chunkenc/histogram.go @@ -1073,6 +1073,7 @@ func (it *histogramIterator) Reset(b []byte) { if it.atHistogramCalled { it.atHistogramCalled = false it.pBuckets, it.nBuckets = nil, nil + it.pSpans, it.nSpans = nil, nil } else { it.pBuckets = it.pBuckets[:0] it.nBuckets = it.nBuckets[:0] @@ -1185,8 +1186,25 @@ func (it *histogramIterator) Next() ValueType { // The case for the 2nd sample with single deltas is implicitly handled correctly with the double delta code, // so we don't need a separate single delta logic for the 2nd sample. - // Recycle bucket slices that have not been returned yet. Otherwise, + // Recycle bucket and span slices that have not been returned yet. Otherwise, copy them. // copy them. + if it.atFloatHistogramCalled || it.atHistogramCalled { + if len(it.pSpans) > 0 { + newSpans := make([]histogram.Span, len(it.pSpans)) + copy(newSpans, it.pSpans) + it.pSpans = newSpans + } else { + it.pSpans = nil + } + if len(it.nSpans) > 0 { + newSpans := make([]histogram.Span, len(it.nSpans)) + copy(newSpans, it.nSpans) + it.nSpans = newSpans + } else { + it.nSpans = nil + } + } + if it.atHistogramCalled { it.atHistogramCalled = false if len(it.pBuckets) > 0 { @@ -1204,6 +1222,7 @@ func (it *histogramIterator) Next() ValueType { it.nBuckets = nil } } + // FloatBuckets are set from scratch, so simply create empty ones. if it.atFloatHistogramCalled { it.atFloatHistogramCalled = false diff --git a/tsdb/chunkenc/histogram_test.go b/tsdb/chunkenc/histogram_test.go index 59187ed17e..29b77b1583 100644 --- a/tsdb/chunkenc/histogram_test.go +++ b/tsdb/chunkenc/histogram_test.go @@ -1487,6 +1487,108 @@ func TestHistogramAppendOnlyErrors(t *testing.T) { }) } +func TestHistogramUniqueSpansAfterNext(t *testing.T) { + // Create two histograms with the same schema and spans. + h1 := &histogram.Histogram{ + Schema: 1, + ZeroThreshold: 1e-100, + Count: 10, + ZeroCount: 2, + Sum: 15.0, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 2}, + {Offset: 1, Length: 2}, + }, + PositiveBuckets: []int64{1, 2, 3, 4}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 1}, + }, + NegativeBuckets: []int64{2}, + } + + h2 := h1.Copy() + + // Create a chunk and append both histograms. + c := NewHistogramChunk() + app, err := c.Appender() + require.NoError(t, err) + + _, _, _, err = app.AppendHistogram(nil, 0, h1, false) + require.NoError(t, err) + + _, _, _, err = app.AppendHistogram(nil, 1, h2, false) + require.NoError(t, err) + + // Create an iterator and advance to the first histogram. + it := c.Iterator(nil) + require.Equal(t, ValHistogram, it.Next()) + _, rh1 := it.AtHistogram(nil) + + // Advance to the second histogram and retrieve it. + require.Equal(t, ValHistogram, it.Next()) + _, rh2 := it.AtHistogram(nil) + + require.Equal(t, rh1.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh1.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + require.Equal(t, rh2.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh2.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + + // Check that the spans for h1 and h2 are unique slices. + require.NotSame(t, &rh1.PositiveSpans[0], &rh2.PositiveSpans[0], "PositiveSpans should be unique between histograms") + require.NotSame(t, &rh1.NegativeSpans[0], &rh2.NegativeSpans[0], "NegativeSpans should be unique between histograms") +} + +func TestHistogramUniqueSpansAfterNextWithAtFloatHistogram(t *testing.T) { + // Create two histograms with the same schema and spans. + h1 := &histogram.Histogram{ + Schema: 1, + ZeroThreshold: 1e-100, + Count: 10, + ZeroCount: 2, + Sum: 15.0, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 2}, + {Offset: 1, Length: 2}, + }, + PositiveBuckets: []int64{1, 2, 3, 4}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 1}, + }, + NegativeBuckets: []int64{2}, + } + + h2 := h1.Copy() + + // Create a chunk and append both histograms. + c := NewHistogramChunk() + app, err := c.Appender() + require.NoError(t, err) + + _, _, _, err = app.AppendHistogram(nil, 0, h1, false) + require.NoError(t, err) + + _, _, _, err = app.AppendHistogram(nil, 1, h2, false) + require.NoError(t, err) + + // Create an iterator and advance to the first histogram. + it := c.Iterator(nil) + require.Equal(t, ValHistogram, it.Next()) + _, rh1 := it.AtFloatHistogram(nil) + + // Advance to the second histogram and retrieve it. + require.Equal(t, ValHistogram, it.Next()) + _, rh2 := it.AtFloatHistogram(nil) + + require.Equal(t, rh1.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh1.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + require.Equal(t, rh2.PositiveSpans, h1.PositiveSpans, "Returned positive spans are as expected") + require.Equal(t, rh2.NegativeSpans, h1.NegativeSpans, "Returned negative spans are as expected") + + // Check that the spans for h1 and h2 are unique slices. + require.NotSame(t, &rh1.PositiveSpans[0], &rh2.PositiveSpans[0], "PositiveSpans should be unique between histograms") + require.NotSame(t, &rh1.NegativeSpans[0], &rh2.NegativeSpans[0], "NegativeSpans should be unique between histograms") +} + func BenchmarkAppendable(b *testing.B) { // Create a histogram with a bunch of spans and buckets. const (