Merge pull request #17213 from prometheus/krajo/native-histogram-schema-reduce

Native histograms: reduce resolution as needed when reading from chunk or remote read

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 {

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")
 }

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())
 		})
 	}
 }

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) {
+		if !histogram.IsKnownSchema(schema) {
-			it.err = histogram.InvalidSchemaError(schema)
+			it.err = histogram.UnknownSchemaError(schema)
 			return ValNone
 		}
 

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)
 		})
 	}
 }

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) {
+		if !histogram.IsKnownSchema(schema) {
-			it.err = histogram.InvalidSchemaError(schema)
+			it.err = histogram.UnknownSchemaError(schema)
 			return ValNone
 		}
 

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)
 		})
 	}
 }