diff --git a/model/histogram/generic.go b/model/histogram/generic.go index cb7281962a..bc4c69d40c 100644 --- a/model/histogram/generic.go +++ b/model/histogram/generic.go @@ -45,12 +45,17 @@ var ( ErrHistogramCustomBucketsNegBuckets = errors.New("custom buckets: must not have negative buckets") ErrHistogramExpSchemaCustomBounds = errors.New("histogram with exponential schema must not have custom bounds") ErrHistogramsInvalidSchema = fmt.Errorf("histogram has an invalid schema, which must be between %d and %d for exponential buckets, or %d for custom buckets", ExponentialSchemaMin, ExponentialSchemaMax, CustomBucketsSchema) + ErrHistogramsUnknownSchema = fmt.Errorf("histogram has an unknown schema, which must be between %d and %d for exponential buckets, or %d for custom buckets", ExponentialSchemaMinReserved, ExponentialSchemaMaxReserved, CustomBucketsSchema) ) func InvalidSchemaError(s int32) error { return fmt.Errorf("%w, got schema %d", ErrHistogramsInvalidSchema, s) } +func UnknownSchemaError(s int32) error { + return fmt.Errorf("%w, got schema %d", ErrHistogramsUnknownSchema, s) +} + func IsCustomBucketsSchema(s int32) bool { return s == CustomBucketsSchema } @@ -67,6 +72,12 @@ func IsValidSchema(s int32) bool { return IsCustomBucketsSchema(s) || IsExponentialSchema(s) } +// IsKnownSchema returns bool if we known and accept the schema, but need to +// reduce resolution to the nearest supported schema. +func IsKnownSchema(s int32) bool { + return IsCustomBucketsSchema(s) || IsExponentialSchemaReserved(s) +} + // BucketCount is a type constraint for the count in a bucket, which can be // float64 (for type FloatHistogram) or uint64 (for type Histogram). type BucketCount interface { diff --git a/storage/remote/codec.go b/storage/remote/codec.go index 25e5ba04b6..7e21909354 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -472,10 +472,10 @@ func (c *concreteSeriesIterator) Seek(t int64) chunkenc.ValueType { } func validateHistogramSchema(h *prompb.Histogram) error { - if histogram.IsValidSchema(h.Schema) { + if histogram.IsKnownSchema(h.Schema) { return nil } - return histogram.InvalidSchemaError(h.Schema) + return histogram.UnknownSchemaError(h.Schema) } func getHistogramValType(h *prompb.Histogram) chunkenc.ValueType { @@ -500,14 +500,28 @@ func (c *concreteSeriesIterator) AtHistogram(*histogram.Histogram) (int64, *hist panic("iterator is not on an integer histogram sample") } h := c.series.histograms[c.histogramsCur] - return h.Timestamp, h.ToIntHistogram() + mh := h.ToIntHistogram() + if mh.Schema > histogram.ExponentialSchemaMax && mh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // sample is from a newer Prometheus version that supports higher + // resolution. + mh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return h.Timestamp, mh } // AtFloatHistogram implements chunkenc.Iterator. func (c *concreteSeriesIterator) AtFloatHistogram(*histogram.FloatHistogram) (int64, *histogram.FloatHistogram) { if c.curValType == chunkenc.ValHistogram || c.curValType == chunkenc.ValFloatHistogram { fh := c.series.histograms[c.histogramsCur] - return fh.Timestamp, fh.ToFloatHistogram() // integer will be auto-converted. + mfh := fh.ToFloatHistogram() // integer will be auto-converted. + if mfh.Schema > histogram.ExponentialSchemaMax && mfh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // sample is from a newer Prometheus version that supports higher + // resolution. + mfh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return fh.Timestamp, mfh } panic("iterator is not on a histogram sample") } diff --git a/storage/remote/codec_test.go b/storage/remote/codec_test.go index 396acd0a5f..ddf8f76cf6 100644 --- a/storage/remote/codec_test.go +++ b/storage/remote/codec_test.go @@ -570,24 +570,53 @@ func TestConcreteSeriesIterator_InvalidHistogramSamples(t *testing.T) { require.Equal(t, chunkenc.ValFloat, it.Next()) require.Equal(t, chunkenc.ValNone, it.Next()) require.Error(t, it.Err()) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) it = series.Iterator(it) require.Equal(t, chunkenc.ValFloat, it.Next()) require.Equal(t, chunkenc.ValNone, it.Next()) - require.Error(t, it.Err()) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) it = series.Iterator(it) require.Equal(t, chunkenc.ValNone, it.Seek(1)) - require.Error(t, it.Err()) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) it = series.Iterator(it) require.Equal(t, chunkenc.ValFloat, it.Seek(3)) require.Equal(t, chunkenc.ValNone, it.Next()) - require.Error(t, it.Err()) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) it = series.Iterator(it) require.Equal(t, chunkenc.ValNone, it.Seek(4)) - require.Error(t, it.Err()) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) + }) + } +} + +func TestConcreteSeriesIterator_ReducesHighResolutionHistograms(t *testing.T) { + for _, schema := range []int32{9, 52} { + t.Run(fmt.Sprintf("schema=%d", schema), func(t *testing.T) { + h := testHistogram.Copy() + h.Schema = schema + fh := h.ToFloat(nil) + series := &concreteSeries{ + labels: labels.FromStrings("foo", "bar"), + histograms: []prompb.Histogram{ + prompb.FromIntHistogram(1, h), + prompb.FromFloatHistogram(2, fh), + }, + } + it := series.Iterator(nil) + require.Equal(t, chunkenc.ValHistogram, it.Next()) + _, gotH := it.AtHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, gotH.Schema) + _, gotFH := it.AtFloatHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, gotFH.Schema) + require.Equal(t, chunkenc.ValFloatHistogram, it.Next()) + _, gotFH = it.AtFloatHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, gotFH.Schema) + require.Equal(t, chunkenc.ValNone, it.Next()) + require.NoError(t, it.Err()) }) } } diff --git a/tsdb/chunkenc/float_histogram.go b/tsdb/chunkenc/float_histogram.go index 13faf9961a..d80b1d9bcc 100644 --- a/tsdb/chunkenc/float_histogram.go +++ b/tsdb/chunkenc/float_histogram.go @@ -866,7 +866,7 @@ func (it *floatHistogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) } if fh == nil { it.atFloatHistogramCalled = true - return it.t, &histogram.FloatHistogram{ + fh = &histogram.FloatHistogram{ CounterResetHint: counterResetHint(it.counterResetHeader, it.numRead), Count: it.cnt.value, ZeroCount: it.zCnt.value, @@ -879,6 +879,14 @@ func (it *floatHistogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) NegativeBuckets: it.nBuckets, CustomValues: it.customValues, } + if fh.Schema > histogram.ExponentialSchemaMax && fh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + fh = fh.Copy() + fh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, fh } fh.CounterResetHint = counterResetHint(it.counterResetHeader, it.numRead) @@ -903,6 +911,13 @@ func (it *floatHistogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) // Custom values are interned. The single copy is in this iterator. fh.CustomValues = it.customValues + if fh.Schema > histogram.ExponentialSchemaMax && fh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + fh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, fh } @@ -955,8 +970,8 @@ func (it *floatHistogramIterator) Next() ValueType { return ValNone } - if !histogram.IsValidSchema(schema) { - it.err = histogram.InvalidSchemaError(schema) + if !histogram.IsKnownSchema(schema) { + it.err = histogram.UnknownSchemaError(schema) return ValNone } diff --git a/tsdb/chunkenc/float_histogram_test.go b/tsdb/chunkenc/float_histogram_test.go index a9813d2c64..d112c81f1c 100644 --- a/tsdb/chunkenc/float_histogram_test.go +++ b/tsdb/chunkenc/float_histogram_test.go @@ -1488,7 +1488,37 @@ func TestFloatHistogramIteratorFailIfSchemaInValid(t *testing.T) { it := c.Iterator(nil) require.Equal(t, ValNone, it.Next()) - require.ErrorIs(t, it.Err(), histogram.ErrHistogramsInvalidSchema) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) + }) + } +} + +func TestFloatHistogramIteratorReduceSchema(t *testing.T) { + for _, schema := range []int32{9, 52} { + t.Run(fmt.Sprintf("schema %d", schema), func(t *testing.T) { + h := &histogram.FloatHistogram{ + Schema: schema, + Count: 10, + Sum: 15.0, + ZeroThreshold: 1e-100, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 2}, + {Offset: 1, Length: 2}, + }, + PositiveBuckets: []float64{1, 2, 3, 4}, + } + + c := NewFloatHistogramChunk() + app, err := c.Appender() + require.NoError(t, err) + + _, _, _, err = app.AppendFloatHistogram(nil, 1, h, false) + require.NoError(t, err) + + it := c.Iterator(nil) + require.Equal(t, ValFloatHistogram, it.Next()) + _, rh := it.AtFloatHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, rh.Schema) }) } } diff --git a/tsdb/chunkenc/histogram.go b/tsdb/chunkenc/histogram.go index 194b67962f..9c433fc5e5 100644 --- a/tsdb/chunkenc/histogram.go +++ b/tsdb/chunkenc/histogram.go @@ -921,7 +921,7 @@ func (it *histogramIterator) AtHistogram(h *histogram.Histogram) (int64, *histog } if h == nil { it.atHistogramCalled = true - return it.t, &histogram.Histogram{ + h = &histogram.Histogram{ CounterResetHint: counterResetHint(it.counterResetHeader, it.numRead), Count: it.cnt, ZeroCount: it.zCnt, @@ -934,6 +934,14 @@ func (it *histogramIterator) AtHistogram(h *histogram.Histogram) (int64, *histog NegativeBuckets: it.nBuckets, CustomValues: it.customValues, } + if h.Schema > histogram.ExponentialSchemaMax && h.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + h = h.Copy() + h.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, h } h.CounterResetHint = counterResetHint(it.counterResetHeader, it.numRead) @@ -958,6 +966,13 @@ func (it *histogramIterator) AtHistogram(h *histogram.Histogram) (int64, *histog // Custom values are interned. The single copy is here in the iterator. h.CustomValues = it.customValues + if h.Schema > histogram.ExponentialSchemaMax && h.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + h.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, h } @@ -967,7 +982,7 @@ func (it *histogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int } if fh == nil { it.atFloatHistogramCalled = true - return it.t, &histogram.FloatHistogram{ + fh = &histogram.FloatHistogram{ CounterResetHint: counterResetHint(it.counterResetHeader, it.numRead), Count: float64(it.cnt), ZeroCount: float64(it.zCnt), @@ -980,6 +995,14 @@ func (it *histogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int NegativeBuckets: it.nFloatBuckets, CustomValues: it.customValues, } + if fh.Schema > histogram.ExponentialSchemaMax && fh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + fh = fh.Copy() + fh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, fh } fh.CounterResetHint = counterResetHint(it.counterResetHeader, it.numRead) @@ -1012,6 +1035,13 @@ func (it *histogramIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int // Custom values are interned. The single copy is here in the iterator. fh.CustomValues = it.customValues + if fh.Schema > histogram.ExponentialSchemaMax && fh.Schema <= histogram.ExponentialSchemaMaxReserved { + // This is a very slow path, but it should only happen if the + // chunk is from a newer Prometheus version that supports higher + // resolution. + fh.ReduceResolution(histogram.ExponentialSchemaMax) + } + return it.t, fh } @@ -1078,8 +1108,8 @@ func (it *histogramIterator) Next() ValueType { return ValNone } - if !histogram.IsValidSchema(schema) { - it.err = histogram.InvalidSchemaError(schema) + if !histogram.IsKnownSchema(schema) { + it.err = histogram.UnknownSchemaError(schema) return ValNone } diff --git a/tsdb/chunkenc/histogram_test.go b/tsdb/chunkenc/histogram_test.go index b191960d89..4213313acc 100644 --- a/tsdb/chunkenc/histogram_test.go +++ b/tsdb/chunkenc/histogram_test.go @@ -1844,7 +1844,40 @@ func TestHistogramIteratorFailIfSchemaInValid(t *testing.T) { it := c.Iterator(nil) require.Equal(t, ValNone, it.Next()) - require.ErrorIs(t, it.Err(), histogram.ErrHistogramsInvalidSchema) + require.ErrorIs(t, it.Err(), histogram.ErrHistogramsUnknownSchema) + }) + } +} + +func TestHistogramIteratorReduceSchema(t *testing.T) { + for _, schema := range []int32{9, 52} { + t.Run(fmt.Sprintf("schema %d", schema), func(t *testing.T) { + h := &histogram.Histogram{ + Schema: schema, + Count: 10, + Sum: 15.0, + ZeroThreshold: 1e-100, + PositiveSpans: []histogram.Span{ + {Offset: 0, Length: 2}, + {Offset: 1, Length: 2}, + }, + PositiveBuckets: []int64{1, 2, 3, 4}, + } + + c := NewHistogramChunk() + app, err := c.Appender() + require.NoError(t, err) + + _, _, _, err = app.AppendHistogram(nil, 1, h, false) + require.NoError(t, err) + + it := c.Iterator(nil) + require.Equal(t, ValHistogram, it.Next()) + _, rh := it.AtHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, rh.Schema) + + _, rfh := it.AtFloatHistogram(nil) + require.Equal(t, histogram.ExponentialSchemaMax, rfh.Schema) }) } }