From f1616df782d1991f2845e332bc1671731c455964 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 18:18:16 +0000 Subject: [PATCH] fix: correct prev-tracking in V2 histogram encoders for interleaved custom-bucket histograms Co-authored-by: ywwg <888940+ywwg@users.noreply.github.com> --- tsdb/record/record.go | 98 ++++++++++++++++++++++---------------- tsdb/record/record_test.go | 80 +++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 40 deletions(-) diff --git a/tsdb/record/record.go b/tsdb/record/record.go index afcc48fb8c..88ee03b14c 100644 --- a/tsdb/record/record.go +++ b/tsdb/record/record.go @@ -1119,27 +1119,39 @@ func (*Encoder) histogramSamplesV2(histograms []RefHistogramSample, b []byte) ([ var customBucketHistograms []RefHistogramSample - first := histograms[0] - - // First sample: full varint values, no deltas, no ST marker. - if first.H.UsesCustomBuckets() { - customBucketHistograms = append(customBucketHistograms, first) - } else { - buf.PutVarint64(int64(first.Ref)) - buf.PutVarint64(first.T) - buf.PutVarint64(first.ST) - EncodeHistogram(&buf, first.H) - } - - // Subsequent samples: ref delta to prev, T delta to first, ST marker. - for i := 1; i < len(histograms); i++ { - h := histograms[i] + // Find the first non-custom-bucket histogram to use as anchor for deltas. + firstIdx := -1 + var first RefHistogramSample + for i, h := range histograms { if h.H.UsesCustomBuckets() { customBucketHistograms = append(customBucketHistograms, h) continue } - prev := histograms[i-1] + firstIdx = i + first = h + break + } + if firstIdx == -1 { + // All histograms use custom buckets. + buf.Reset() + return buf.Get(), customBucketHistograms + } + + // First non-custom-bucket sample: full varint values, no deltas, no ST marker. + buf.PutVarint64(int64(first.Ref)) + buf.PutVarint64(first.T) + buf.PutVarint64(first.ST) + EncodeHistogram(&buf, first.H) + + // Remaining samples: ref delta to last encoded, T delta to first, ST marker. + // prev tracks the last encoded non-custom-bucket histogram. + prev := first + for _, h := range histograms[firstIdx+1:] { + if h.H.UsesCustomBuckets() { + customBucketHistograms = append(customBucketHistograms, h) + continue + } buf.PutVarint64(int64(h.Ref) - int64(prev.Ref)) buf.PutVarint64(h.T - first.T) @@ -1153,11 +1165,7 @@ func (*Encoder) histogramSamplesV2(histograms []RefHistogramSample, b []byte) ([ buf.PutVarint64(h.ST - first.ST) } EncodeHistogram(&buf, h.H) - } - - // Reset buffer if only custom bucket histograms existed in list of histogram samples. - if len(histograms) == len(customBucketHistograms) { - buf.Reset() + prev = h } return buf.Get(), customBucketHistograms @@ -1325,25 +1333,39 @@ func (*Encoder) floatHistogramSamplesV2(histograms []RefFloatHistogramSample, b var customBucketsFloatHistograms []RefFloatHistogramSample - first := histograms[0] - - if first.FH.UsesCustomBuckets() { - customBucketsFloatHistograms = append(customBucketsFloatHistograms, first) - } else { - buf.PutVarint64(int64(first.Ref)) - buf.PutVarint64(first.T) - buf.PutVarint64(first.ST) - EncodeFloatHistogram(&buf, first.FH) - } - - for i := 1; i < len(histograms); i++ { - h := histograms[i] + // Find the first non-custom-bucket histogram to use as anchor for deltas. + firstIdx := -1 + var first RefFloatHistogramSample + for i, h := range histograms { if h.FH.UsesCustomBuckets() { customBucketsFloatHistograms = append(customBucketsFloatHistograms, h) continue } - prev := histograms[i-1] + firstIdx = i + first = h + break + } + if firstIdx == -1 { + // All histograms use custom buckets. + buf.Reset() + return buf.Get(), customBucketsFloatHistograms + } + + // First non-custom-bucket sample: full varint values, no deltas, no ST marker. + buf.PutVarint64(int64(first.Ref)) + buf.PutVarint64(first.T) + buf.PutVarint64(first.ST) + EncodeFloatHistogram(&buf, first.FH) + + // Remaining samples: ref delta to last encoded, T delta to first, ST marker. + // prev tracks the last encoded non-custom-bucket histogram. + prev := first + for _, h := range histograms[firstIdx+1:] { + if h.FH.UsesCustomBuckets() { + customBucketsFloatHistograms = append(customBucketsFloatHistograms, h) + continue + } buf.PutVarint64(int64(h.Ref) - int64(prev.Ref)) buf.PutVarint64(h.T - first.T) @@ -1357,11 +1379,7 @@ func (*Encoder) floatHistogramSamplesV2(histograms []RefFloatHistogramSample, b buf.PutVarint64(h.ST - first.ST) } EncodeFloatHistogram(&buf, h.FH) - } - - // Reset buffer if only custom bucket histograms existed in list of histogram samples - if len(histograms) == len(customBucketsFloatHistograms) { - buf.Reset() + prev = h } return buf.Get(), customBucketsFloatHistograms diff --git a/tsdb/record/record_test.go b/tsdb/record/record_test.go index ab5be5e897..9a03ac60ea 100644 --- a/tsdb/record/record_test.go +++ b/tsdb/record/record_test.go @@ -600,6 +600,86 @@ func TestRecord_EncodeDecode(t *testing.T) { require.Equal(t, int64(0), h.ST, "V1 float histogram records must decode with ST=0") } }) + + enc = Encoder{EnableSTStorage: true} + + // V2 int-histogram with custom bucket as first element. + t.Run("V2 int-histogram leading custom bucket", func(t *testing.T) { + // histograms[2] uses custom buckets; placing it first exercises the + // code path where the anchor must be the first non-custom-bucket sample. + input := []RefHistogramSample{ + {Ref: 67, T: 1000, ST: 500, H: histograms[2].H}, + {Ref: 56, T: 2000, ST: 500, H: histograms[0].H}, + {Ref: 42, T: 3000, ST: 600, H: histograms[1].H}, + } + histBuf, customBuckets := enc.HistogramSamples(input, nil) + customBuf := enc.CustomBucketsHistogramSamples(customBuckets, nil) + decHists, err := dec.HistogramSamples(histBuf, nil) + require.NoError(t, err) + decCustom, err := dec.HistogramSamples(customBuf, nil) + require.NoError(t, err) + require.Equal(t, input[1:], decHists) + require.Equal(t, input[:1], decCustom) + }) + + // V2 int-histogram with custom bucket in the middle. + t.Run("V2 int-histogram interleaved custom bucket", func(t *testing.T) { + // histograms[2] uses custom buckets; placing it between regular histograms + // exercises the prev-tracking fix so ref deltas are computed correctly. + input := []RefHistogramSample{ + {Ref: 56, T: 1000, ST: 500, H: histograms[0].H}, + {Ref: 67, T: 2000, ST: 500, H: histograms[2].H}, + {Ref: 42, T: 3000, ST: 600, H: histograms[1].H}, + } + histBuf, customBuckets := enc.HistogramSamples(input, nil) + customBuf := enc.CustomBucketsHistogramSamples(customBuckets, nil) + decHists, err := dec.HistogramSamples(histBuf, nil) + require.NoError(t, err) + decCustom, err := dec.HistogramSamples(customBuf, nil) + require.NoError(t, err) + require.Equal(t, []RefHistogramSample{input[0], input[2]}, decHists) + require.Equal(t, input[1:2], decCustom) + }) + + // V2 float-histogram with custom bucket as first element. + t.Run("V2 float-histogram leading custom bucket", func(t *testing.T) { + customFH := histograms[2].H.ToFloat(nil) + regularFH0 := histograms[0].H.ToFloat(nil) + regularFH1 := histograms[1].H.ToFloat(nil) + input := []RefFloatHistogramSample{ + {Ref: 67, T: 1000, ST: 500, FH: customFH}, + {Ref: 56, T: 2000, ST: 500, FH: regularFH0}, + {Ref: 42, T: 3000, ST: 600, FH: regularFH1}, + } + floatBuf, customBuckets := enc.FloatHistogramSamples(input, nil) + customBuf := enc.CustomBucketsFloatHistogramSamples(customBuckets, nil) + decFloats, err := dec.FloatHistogramSamples(floatBuf, nil) + require.NoError(t, err) + decCustom, err := dec.FloatHistogramSamples(customBuf, nil) + require.NoError(t, err) + require.Equal(t, input[1:], decFloats) + require.Equal(t, input[:1], decCustom) + }) + + // V2 float-histogram with custom bucket in the middle. + t.Run("V2 float-histogram interleaved custom bucket", func(t *testing.T) { + customFH := histograms[2].H.ToFloat(nil) + regularFH0 := histograms[0].H.ToFloat(nil) + regularFH1 := histograms[1].H.ToFloat(nil) + input := []RefFloatHistogramSample{ + {Ref: 56, T: 1000, ST: 500, FH: regularFH0}, + {Ref: 67, T: 2000, ST: 500, FH: customFH}, + {Ref: 42, T: 3000, ST: 600, FH: regularFH1}, + } + floatBuf, customBuckets := enc.FloatHistogramSamples(input, nil) + customBuf := enc.CustomBucketsFloatHistogramSamples(customBuckets, nil) + decFloats, err := dec.FloatHistogramSamples(floatBuf, nil) + require.NoError(t, err) + decCustom, err := dec.FloatHistogramSamples(customBuf, nil) + require.NoError(t, err) + require.Equal(t, []RefFloatHistogramSample{input[0], input[2]}, decFloats) + require.Equal(t, input[1:2], decCustom) + }) } func TestRecord_DecodeInvalidHistogramSchema(t *testing.T) {