From ec9170b8bf6dd8595c42b57d211f3567aa3bd0c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Rabenstein?= Date: Wed, 20 Sep 2023 18:01:21 +0200 Subject: [PATCH 1/4] Merge pull request #12874 from krajorama/outof-order-chunks Fix duplicate sample detection at chunk size limit Signed-off-by: SuperQ --- tsdb/head_append.go | 9 ++++++--- tsdb/head_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/tsdb/head_append.go b/tsdb/head_append.go index 9016943756a4..d1f4d3035e11 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -1282,9 +1282,6 @@ func (s *memSeries) appendPreprocessor(t int64, e chunkenc.Encoding, o chunkOpts // There is no head chunk in this series yet, create the first chunk for the sample. c = s.cutNewHeadChunk(t, e, o.chunkRange) chunkCreated = true - } else if len(c.chunk.Bytes()) > maxBytesPerXORChunk { - c = s.cutNewHeadChunk(t, e, o.chunkRange) - chunkCreated = true } // Out of order sample. @@ -1292,6 +1289,12 @@ func (s *memSeries) appendPreprocessor(t int64, e chunkenc.Encoding, o chunkOpts return c, false, chunkCreated } + // Check the chunk size, unless we just created it and if the chunk is too large, cut a new one. + if !chunkCreated && len(c.chunk.Bytes()) > maxBytesPerXORChunk { + c = s.cutNewHeadChunk(t, e, o.chunkRange) + chunkCreated = true + } + if c.chunk.Encoding() != e { // The chunk encoding expected by this append is different than the head chunk's // encoding. So we cut a new chunk with the expected encoding. diff --git a/tsdb/head_test.go b/tsdb/head_test.go index b501f5f3f3be..cd9ab651088a 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -5399,3 +5399,49 @@ func TestCuttingNewHeadChunks(t *testing.T) { }) } } + +// TestHeadDetectsDuplcateSampleAtSizeLimit tests a regression where a duplicate sample +// is appended to the head, right when the head chunk is at the size limit. +// The test adds all samples as duplicate, thus expecting that the result has +// exactly half of the samples. +func TestHeadDetectsDuplicateSampleAtSizeLimit(t *testing.T) { + numSamples := 1000 + baseTS := int64(1695209650) + + h, _ := newTestHead(t, DefaultBlockDuration, wlog.CompressionNone, false) + defer func() { + require.NoError(t, h.Close()) + }() + + a := h.Appender(context.Background()) + var err error + vals := []float64{math.MaxFloat64, 0x00} // Use the worst case scenario for the XOR encoding. Otherwise we hit the sample limit before the size limit. + for i := 0; i < numSamples; i++ { + ts := baseTS + int64(i/2)*10000 + a.Append(0, labels.FromStrings("foo", "bar"), ts, vals[(i/2)%len(vals)]) + err = a.Commit() + require.NoError(t, err) + a = h.Appender(context.Background()) + } + + indexReader, err := h.Index() + require.NoError(t, err) + + var ( + chunks []chunks.Meta + builder labels.ScratchBuilder + ) + require.NoError(t, indexReader.Series(1, &builder, &chunks)) + + chunkReader, err := h.Chunks() + require.NoError(t, err) + + storedSampleCount := 0 + for _, chunkMeta := range chunks { + chunk, err := chunkReader.Chunk(chunkMeta) + require.NoError(t, err) + storedSampleCount += chunk.NumSamples() + } + + require.Equal(t, numSamples/2, storedSampleCount) +} From 64130d7909e2ce5a2e35f2fbb39eb5bc5e09fc92 Mon Sep 17 00:00:00 2001 From: SuperQ Date: Wed, 4 Oct 2023 09:46:57 +0200 Subject: [PATCH 2/4] Release 2.47.1 * [BUGFIX] Fix duplicate sample detection at chunk size limit #12874 Signed-off-by: SuperQ --- CHANGELOG.md | 4 ++++ VERSION | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a9d4521a171..7ad9c69dc968 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2.47.1 / 2023-10-04 + +* [BUGFIX] Fix duplicate sample detection at chunk size limit #12874 + ## 2.47.0 / 2023-09-06 This release adds an experimental OpenTelemetry (OTLP) Ingestion feature, diff --git a/VERSION b/VERSION index bb13a7e35459..24c6ede7ad05 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.47.0 +2.47.1 From 426f959bfe9cbf46e6171d44c2102885985ff823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Rabenstein?= Date: Tue, 19 Sep 2023 17:10:27 +0200 Subject: [PATCH 3/4] Merge pull request #12838 from krajorama/fix-disappearing-span-panic Fix counterResetInAnyBucket panic --- tsdb/chunkenc/float_histogram.go | 21 ++- tsdb/chunkenc/float_histogram_test.go | 231 ++++++++++++++++++++------ tsdb/chunkenc/histogram.go | 21 ++- tsdb/chunkenc/histogram_meta.go | 11 +- tsdb/chunkenc/histogram_test.go | 225 +++++++++++++++++++------ 5 files changed, 391 insertions(+), 118 deletions(-) diff --git a/tsdb/chunkenc/float_histogram.go b/tsdb/chunkenc/float_histogram.go index 2b9cc11a346b..31b3a09defc2 100644 --- a/tsdb/chunkenc/float_histogram.go +++ b/tsdb/chunkenc/float_histogram.go @@ -334,11 +334,16 @@ func counterResetInAnyFloatBucket(oldBuckets []xorValue, newBuckets []float64, o return false } - oldSpanSliceIdx, newSpanSliceIdx := 0, 0 // Index for the span slices. - oldInsideSpanIdx, newInsideSpanIdx := uint32(0), uint32(0) // Index inside a span. - oldIdx, newIdx := oldSpans[0].Offset, newSpans[0].Offset - - oldBucketSliceIdx, newBucketSliceIdx := 0, 0 // Index inside bucket slice. + var ( + oldSpanSliceIdx, newSpanSliceIdx int = -1, -1 // Index for the span slices. Starts at -1 to indicate that the first non empty span is not yet found. + oldInsideSpanIdx, newInsideSpanIdx uint32 // Index inside a span. + oldIdx, newIdx int32 // Index inside a bucket slice. + oldBucketSliceIdx, newBucketSliceIdx int // Index inside bucket slice. + ) + + // Find first non empty spans. + oldSpanSliceIdx, oldIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldIdx, oldSpans) + newSpanSliceIdx, newIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newIdx, newSpans) oldVal, newVal := oldBuckets[0].value, newBuckets[0] // Since we assume that new spans won't have missing buckets, there will never be a case @@ -354,13 +359,12 @@ func counterResetInAnyFloatBucket(oldBuckets []xorValue, newBuckets []float64, o // Moving ahead old bucket and span by 1 index. if oldInsideSpanIdx+1 >= oldSpans[oldSpanSliceIdx].Length { // Current span is over. - oldSpanSliceIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldSpans) + oldSpanSliceIdx, oldIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldIdx, oldSpans) oldInsideSpanIdx = 0 if oldSpanSliceIdx >= len(oldSpans) { // All old spans are over. break } - oldIdx += 1 + oldSpans[oldSpanSliceIdx].Offset } else { oldInsideSpanIdx++ oldIdx++ @@ -373,14 +377,13 @@ func counterResetInAnyFloatBucket(oldBuckets []xorValue, newBuckets []float64, o // Moving ahead new bucket and span by 1 index. if newInsideSpanIdx+1 >= newSpans[newSpanSliceIdx].Length { // Current span is over. - newSpanSliceIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newSpans) + newSpanSliceIdx, newIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newIdx, newSpans) newInsideSpanIdx = 0 if newSpanSliceIdx >= len(newSpans) { // All new spans are over. // This should not happen, old spans above should catch this first. panic("new spans over before old spans in counterReset") } - newIdx += 1 + newSpans[newSpanSliceIdx].Offset } else { newInsideSpanIdx++ newIdx++ diff --git a/tsdb/chunkenc/float_histogram_test.go b/tsdb/chunkenc/float_histogram_test.go index 8af381fd7ca8..7e200ce16cc1 100644 --- a/tsdb/chunkenc/float_histogram_test.go +++ b/tsdb/chunkenc/float_histogram_test.go @@ -443,62 +443,193 @@ func assertRecodedFloatHistogramChunkOnAppend(t *testing.T, prevChunk Chunk, hAp } func TestFloatHistogramChunkAppendableWithEmptySpan(t *testing.T) { - h1 := &histogram.FloatHistogram{ - Schema: 0, - Count: 21, - Sum: 1234.5, - ZeroThreshold: 0.001, - ZeroCount: 4, - PositiveSpans: []histogram.Span{ - {Offset: 0, Length: 4}, - {Offset: 0, Length: 0}, - {Offset: 0, Length: 3}, + tests := map[string]struct { + h1 *histogram.FloatHistogram + h2 *histogram.FloatHistogram + }{ + "empty span in old and new histogram": { + h1: &histogram.FloatHistogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 0, Length: 0}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []float64{1, 2, 1, 1, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 2, 1, 2, 2, 2, 2}, + }, + h2: &histogram.FloatHistogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 0, Length: 0}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []float64{1, 3, 1, 2, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 4, 2, 7, 5, 5, 2}, + }, }, - PositiveBuckets: []float64{1, 2, 1, 1, 1, 1, 1}, - NegativeSpans: []histogram.Span{ - {Offset: 1, Length: 4}, - {Offset: 2, Length: 0}, - {Offset: 2, Length: 3}, + "empty span in old histogram": { + h1: &histogram.FloatHistogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 1, Length: 0}, // This span will disappear. + {Offset: 2, Length: 4}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []float64{1, 2, 1, 1, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 2, 1, 2, 2, 2, 2}, + }, + h2: &histogram.FloatHistogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 3, Length: 4}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []float64{1, 3, 1, 2, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 4, 2, 7, 5, 5, 2}, + }, }, - NegativeBuckets: []float64{1, 2, 1, 2, 2, 2, 2}, - } - h2 := &histogram.FloatHistogram{ - Schema: 0, - Count: 37, - Sum: 2345.6, - ZeroThreshold: 0.001, - ZeroCount: 5, - PositiveSpans: []histogram.Span{ - {Offset: 0, Length: 4}, - {Offset: 0, Length: 0}, - {Offset: 0, Length: 3}, + "empty span in new histogram": { + h1: &histogram.FloatHistogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 3, Length: 3}, + }, + PositiveBuckets: []float64{1, 2, 1, 1, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 2, 1, 2, 2, 2, 2}, + }, + h2: &histogram.FloatHistogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 1, Length: 0}, // This span is new. + {Offset: 2, Length: 3}, + }, + PositiveBuckets: []float64{1, 3, 1, 2, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 4, 2, 7, 5, 5, 2}, + }, }, - PositiveBuckets: []float64{1, 3, 1, 2, 1, 1, 1}, - NegativeSpans: []histogram.Span{ - {Offset: 1, Length: 4}, - {Offset: 2, Length: 0}, - {Offset: 2, Length: 3}, + "two empty spans mixing offsets": { + h1: &histogram.FloatHistogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 1, Length: 0}, + {Offset: 3, Length: 0}, + {Offset: 4, Length: 3}, + }, + PositiveBuckets: []float64{1, 2, 1, 1, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 2, 1, 2, 2, 2, 2}, + }, + h2: &histogram.FloatHistogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 3, Length: 0}, + {Offset: 1, Length: 0}, + {Offset: 4, Length: 3}, + }, + PositiveBuckets: []float64{1, 3, 1, 2, 1, 1, 1}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []float64{1, 4, 2, 7, 5, 5, 2}, + }, }, - NegativeBuckets: []float64{1, 4, 2, 7, 5, 5, 2}, } - c := Chunk(NewFloatHistogramChunk()) - - // Create fresh appender and add the first histogram. - app, err := c.Appender() - require.NoError(t, err) - require.Equal(t, 0, c.NumSamples()) - - _, _, _, err = app.AppendFloatHistogram(nil, 1, h1, true) - require.NoError(t, err) - require.Equal(t, 1, c.NumSamples()) - hApp, _ := app.(*FloatHistogramAppender) - - pI, nI, okToAppend, counterReset := hApp.appendable(h2) - require.Empty(t, pI) - require.Empty(t, nI) - require.True(t, okToAppend) - require.False(t, counterReset) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + c := Chunk(NewFloatHistogramChunk()) + + // Create fresh appender and add the first histogram. + app, err := c.Appender() + require.NoError(t, err) + require.Equal(t, 0, c.NumSamples()) + + _, _, _, err = app.AppendFloatHistogram(nil, 1, tc.h1, true) + require.NoError(t, err) + require.Equal(t, 1, c.NumSamples()) + hApp, _ := app.(*FloatHistogramAppender) + + pI, nI, okToAppend, counterReset := hApp.appendable(tc.h2) + require.Empty(t, pI) + require.Empty(t, nI) + require.True(t, okToAppend) + require.False(t, counterReset) + }) + } } func TestFloatHistogramChunkAppendableGauge(t *testing.T) { diff --git a/tsdb/chunkenc/histogram.go b/tsdb/chunkenc/histogram.go index 4b021d4269af..e14e25df1e12 100644 --- a/tsdb/chunkenc/histogram.go +++ b/tsdb/chunkenc/histogram.go @@ -354,11 +354,16 @@ func counterResetInAnyBucket(oldBuckets, newBuckets []int64, oldSpans, newSpans return false } - oldSpanSliceIdx, newSpanSliceIdx := 0, 0 // Index for the span slices. - oldInsideSpanIdx, newInsideSpanIdx := uint32(0), uint32(0) // Index inside a span. - oldIdx, newIdx := oldSpans[0].Offset, newSpans[0].Offset - - oldBucketSliceIdx, newBucketSliceIdx := 0, 0 // Index inside bucket slice. + var ( + oldSpanSliceIdx, newSpanSliceIdx int = -1, -1 // Index for the span slices. Starts at -1 to indicate that the first non empty span is not yet found. + oldInsideSpanIdx, newInsideSpanIdx uint32 // Index inside a span. + oldIdx, newIdx int32 // Index inside a bucket slice. + oldBucketSliceIdx, newBucketSliceIdx int // Index inside bucket slice. + ) + + // Find first non empty spans. + oldSpanSliceIdx, oldIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldIdx, oldSpans) + newSpanSliceIdx, newIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newIdx, newSpans) oldVal, newVal := oldBuckets[0], newBuckets[0] // Since we assume that new spans won't have missing buckets, there will never be a case @@ -374,13 +379,12 @@ func counterResetInAnyBucket(oldBuckets, newBuckets []int64, oldSpans, newSpans // Moving ahead old bucket and span by 1 index. if oldInsideSpanIdx+1 >= oldSpans[oldSpanSliceIdx].Length { // Current span is over. - oldSpanSliceIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldSpans) + oldSpanSliceIdx, oldIdx = nextNonEmptySpanSliceIdx(oldSpanSliceIdx, oldIdx, oldSpans) oldInsideSpanIdx = 0 if oldSpanSliceIdx >= len(oldSpans) { // All old spans are over. break } - oldIdx += 1 + oldSpans[oldSpanSliceIdx].Offset } else { oldInsideSpanIdx++ oldIdx++ @@ -393,14 +397,13 @@ func counterResetInAnyBucket(oldBuckets, newBuckets []int64, oldSpans, newSpans // Moving ahead new bucket and span by 1 index. if newInsideSpanIdx+1 >= newSpans[newSpanSliceIdx].Length { // Current span is over. - newSpanSliceIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newSpans) + newSpanSliceIdx, newIdx = nextNonEmptySpanSliceIdx(newSpanSliceIdx, newIdx, newSpans) newInsideSpanIdx = 0 if newSpanSliceIdx >= len(newSpans) { // All new spans are over. // This should not happen, old spans above should catch this first. panic("new spans over before old spans in counterReset") } - newIdx += 1 + newSpans[newSpanSliceIdx].Offset } else { newInsideSpanIdx++ newIdx++ diff --git a/tsdb/chunkenc/histogram_meta.go b/tsdb/chunkenc/histogram_meta.go index 7a21bc20bdc8..cda1080a8b95 100644 --- a/tsdb/chunkenc/histogram_meta.go +++ b/tsdb/chunkenc/histogram_meta.go @@ -489,8 +489,13 @@ func counterResetHint(crh CounterResetHeader, numRead uint16) histogram.CounterR } // Handle pathological case of empty span when advancing span idx. -func nextNonEmptySpanSliceIdx(idx int, spans []histogram.Span) (newIdx int) { - for idx++; idx < len(spans) && spans[idx].Length == 0; idx++ { +// Call it with idx==-1 to find the first non empty span. +func nextNonEmptySpanSliceIdx(idx int, bucketIdx int32, spans []histogram.Span) (newIdx int, newBucketIdx int32) { + for idx++; idx < len(spans); idx++ { + if spans[idx].Length > 0 { + return idx, bucketIdx + spans[idx].Offset + 1 + } + bucketIdx += spans[idx].Offset } - return idx + return idx, 0 } diff --git a/tsdb/chunkenc/histogram_test.go b/tsdb/chunkenc/histogram_test.go index 449af5902394..c760d36c9985 100644 --- a/tsdb/chunkenc/histogram_test.go +++ b/tsdb/chunkenc/histogram_test.go @@ -463,62 +463,193 @@ func assertSampleCount(t *testing.T, c Chunk, exp int64, vtype ValueType) { } func TestHistogramChunkAppendableWithEmptySpan(t *testing.T) { - h1 := &histogram.Histogram{ - Schema: 0, - Count: 21, - Sum: 1234.5, - ZeroThreshold: 0.001, - ZeroCount: 4, - PositiveSpans: []histogram.Span{ - {Offset: 0, Length: 4}, - {Offset: 0, Length: 0}, - {Offset: 0, Length: 3}, + tests := map[string]struct { + h1 *histogram.Histogram + h2 *histogram.Histogram + }{ + "empty span in old and new histogram": { + h1: &histogram.Histogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 0, Length: 0}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []int64{1, 1, -1, 0, 0, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 1, -1, 1, 0, 0, 0}, + }, + h2: &histogram.Histogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 0, Length: 0}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []int64{1, 2, -2, 1, -1, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 3, -2, 5, -2, 0, -3}, + }, }, - PositiveBuckets: []int64{1, 1, -1, 0, 0, 0, 0}, - NegativeSpans: []histogram.Span{ - {Offset: 1, Length: 4}, - {Offset: 2, Length: 0}, - {Offset: 2, Length: 3}, + "empty span in old histogram": { + h1: &histogram.Histogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 1, Length: 0}, // This span will disappear. + {Offset: 2, Length: 4}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []int64{1, 1, -1, 0, 0, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 1, -1, 1, 0, 0, 0}, + }, + h2: &histogram.Histogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 3, Length: 4}, + {Offset: 0, Length: 3}, + }, + PositiveBuckets: []int64{1, 2, -2, 1, -1, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 3, -2, 5, -2, 0, -3}, + }, }, - NegativeBuckets: []int64{1, 1, -1, 1, 0, 0, 0}, - } - h2 := &histogram.Histogram{ - Schema: 0, - Count: 37, - Sum: 2345.6, - ZeroThreshold: 0.001, - ZeroCount: 5, - PositiveSpans: []histogram.Span{ - {Offset: 0, Length: 4}, - {Offset: 0, Length: 0}, - {Offset: 0, Length: 3}, + "empty span in new histogram": { + h1: &histogram.Histogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 3, Length: 3}, + }, + PositiveBuckets: []int64{1, 1, -1, 0, 0, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 1, -1, 1, 0, 0, 0}, + }, + h2: &histogram.Histogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 1, Length: 0}, // This span is new. + {Offset: 2, Length: 3}, + }, + PositiveBuckets: []int64{1, 2, -2, 1, -1, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 3, -2, 5, -2, 0, -3}, + }, }, - PositiveBuckets: []int64{1, 2, -2, 1, -1, 0, 0}, - NegativeSpans: []histogram.Span{ - {Offset: 1, Length: 4}, - {Offset: 2, Length: 0}, - {Offset: 2, Length: 3}, + "two empty spans mixing offsets": { + h1: &histogram.Histogram{ + Schema: 0, + Count: 21, + Sum: 1234.5, + ZeroThreshold: 0.001, + ZeroCount: 4, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 1, Length: 0}, + {Offset: 3, Length: 0}, + {Offset: 4, Length: 3}, + }, + PositiveBuckets: []int64{1, 1, -1, 0, 0, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 1, -1, 1, 0, 0, 0}, + }, + h2: &histogram.Histogram{ + Schema: 0, + Count: 37, + Sum: 2345.6, + ZeroThreshold: 0.001, + ZeroCount: 5, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 4}, + {Offset: 3, Length: 0}, + {Offset: 1, Length: 0}, + {Offset: 4, Length: 3}, + }, + PositiveBuckets: []int64{1, 2, -2, 1, -1, 0, 0}, + NegativeSpans: []histogram.Span{ + {Offset: 1, Length: 4}, + {Offset: 2, Length: 0}, + {Offset: 2, Length: 3}, + }, + NegativeBuckets: []int64{1, 3, -2, 5, -2, 0, -3}, + }, }, - NegativeBuckets: []int64{1, 3, -2, 5, -2, 0, -3}, } - c := Chunk(NewHistogramChunk()) + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + c := Chunk(NewHistogramChunk()) - // Create fresh appender and add the first histogram. - app, err := c.Appender() - require.NoError(t, err) - require.Equal(t, 0, c.NumSamples()) + // Create fresh appender and add the first histogram. + app, err := c.Appender() + require.NoError(t, err) + require.Equal(t, 0, c.NumSamples()) - _, _, _, err = app.AppendHistogram(nil, 1, h1, true) - require.NoError(t, err) - require.Equal(t, 1, c.NumSamples()) - hApp, _ := app.(*HistogramAppender) + _, _, _, err = app.AppendHistogram(nil, 1, tc.h1, true) + require.NoError(t, err) + require.Equal(t, 1, c.NumSamples()) + hApp, _ := app.(*HistogramAppender) - pI, nI, okToAppend, counterReset := hApp.appendable(h2) - require.Empty(t, pI) - require.Empty(t, nI) - require.True(t, okToAppend) - require.False(t, counterReset) + pI, nI, okToAppend, counterReset := hApp.appendable(tc.h2) + require.Empty(t, pI) + require.Empty(t, nI) + require.True(t, okToAppend) + require.False(t, counterReset) + }) + } } func TestAtFloatHistogram(t *testing.T) { From 01e1b7cfa625cf82852b4fce48918c63db763739 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 11 Oct 2023 14:49:47 +0100 Subject: [PATCH 4/4] Release 2.47.2 Signed-off-by: Bryan Boreham --- CHANGELOG.md | 4 ++++ VERSION | 2 +- web/ui/module/codemirror-promql/package.json | 4 ++-- web/ui/module/lezer-promql/package.json | 2 +- web/ui/package-lock.json | 14 +++++++------- web/ui/react-app/package.json | 4 ++-- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ad9c69dc968..9ae179a34b66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2.47.2 / 2023-10-11 + +* [BUGFIX] TSDB: Fix counter reset edgecases causing native histogram panics. #12838 + ## 2.47.1 / 2023-10-04 * [BUGFIX] Fix duplicate sample detection at chunk size limit #12874 diff --git a/VERSION b/VERSION index 24c6ede7ad05..6964972affeb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.47.1 +2.47.2 diff --git a/web/ui/module/codemirror-promql/package.json b/web/ui/module/codemirror-promql/package.json index 0b718b7fc3f5..23a20ee96154 100644 --- a/web/ui/module/codemirror-promql/package.json +++ b/web/ui/module/codemirror-promql/package.json @@ -1,6 +1,6 @@ { "name": "@prometheus-io/codemirror-promql", - "version": "0.47.0", + "version": "0.47.2", "description": "a CodeMirror mode for the PromQL language", "types": "dist/esm/index.d.ts", "module": "dist/esm/index.js", @@ -29,7 +29,7 @@ }, "homepage": "https://github.com/prometheus/prometheus/blob/main/web/ui/module/codemirror-promql/README.md", "dependencies": { - "@prometheus-io/lezer-promql": "0.47.0", + "@prometheus-io/lezer-promql": "0.47.2", "lru-cache": "^6.0.0" }, "devDependencies": { diff --git a/web/ui/module/lezer-promql/package.json b/web/ui/module/lezer-promql/package.json index 69109b20405e..0c775b6a18a8 100644 --- a/web/ui/module/lezer-promql/package.json +++ b/web/ui/module/lezer-promql/package.json @@ -1,6 +1,6 @@ { "name": "@prometheus-io/lezer-promql", - "version": "0.47.0", + "version": "0.47.2", "description": "lezer-based PromQL grammar", "main": "dist/index.cjs", "type": "module", diff --git a/web/ui/package-lock.json b/web/ui/package-lock.json index c434cb7ab602..9abc315cdc0c 100644 --- a/web/ui/package-lock.json +++ b/web/ui/package-lock.json @@ -28,10 +28,10 @@ }, "module/codemirror-promql": { "name": "@prometheus-io/codemirror-promql", - "version": "0.47.0", + "version": "0.47.2", "license": "Apache-2.0", "dependencies": { - "@prometheus-io/lezer-promql": "0.47.0", + "@prometheus-io/lezer-promql": "0.47.2", "lru-cache": "^6.0.0" }, "devDependencies": { @@ -61,7 +61,7 @@ }, "module/lezer-promql": { "name": "@prometheus-io/lezer-promql", - "version": "0.47.0", + "version": "0.47.2", "license": "Apache-2.0", "devDependencies": { "@lezer/generator": "^1.2.3", @@ -20765,7 +20765,7 @@ }, "react-app": { "name": "@prometheus-io/app", - "version": "0.47.0", + "version": "0.47.2", "dependencies": { "@codemirror/autocomplete": "^6.7.1", "@codemirror/commands": "^6.2.4", @@ -20783,7 +20783,7 @@ "@lezer/lr": "^1.3.6", "@nexucis/fuzzy": "^0.4.1", "@nexucis/kvsearch": "^0.8.1", - "@prometheus-io/codemirror-promql": "0.47.0", + "@prometheus-io/codemirror-promql": "0.47.2", "bootstrap": "^4.6.2", "css.escape": "^1.5.1", "downshift": "^7.6.0", @@ -23423,7 +23423,7 @@ "@lezer/lr": "^1.3.6", "@nexucis/fuzzy": "^0.4.1", "@nexucis/kvsearch": "^0.8.1", - "@prometheus-io/codemirror-promql": "0.47.0", + "@prometheus-io/codemirror-promql": "0.47.2", "@testing-library/react-hooks": "^7.0.2", "@types/enzyme": "^3.10.13", "@types/flot": "0.0.32", @@ -23487,7 +23487,7 @@ "@lezer/common": "^1.0.3", "@lezer/highlight": "^1.1.6", "@lezer/lr": "^1.3.6", - "@prometheus-io/lezer-promql": "0.47.0", + "@prometheus-io/lezer-promql": "0.47.2", "@types/lru-cache": "^5.1.1", "isomorphic-fetch": "^3.0.0", "lru-cache": "^6.0.0", diff --git a/web/ui/react-app/package.json b/web/ui/react-app/package.json index fa6eb8e3a5f1..8c5e2cada0fb 100644 --- a/web/ui/react-app/package.json +++ b/web/ui/react-app/package.json @@ -1,6 +1,6 @@ { "name": "@prometheus-io/app", - "version": "0.47.0", + "version": "0.47.2", "private": true, "dependencies": { "@codemirror/autocomplete": "^6.7.1", @@ -19,7 +19,7 @@ "@lezer/common": "^1.0.3", "@nexucis/fuzzy": "^0.4.1", "@nexucis/kvsearch": "^0.8.1", - "@prometheus-io/codemirror-promql": "0.47.0", + "@prometheus-io/codemirror-promql": "0.47.2", "bootstrap": "^4.6.2", "css.escape": "^1.5.1", "downshift": "^7.6.0",