mirror of
https://github.com/prometheus/prometheus.git
synced 2025-08-06 06:07:11 +02:00
fix(promql): histogram_count inconsistent
The problem is in the counter reset detection. The code that loads the samples is matrixIterSlice which uses the typed Buffer iterator, which will preload the integer histogram samples, however the last sample is always(!) loaded as a float histogram sample in matrixIterSlice and the optimized iterator fails to detect counter resets in that case. Also the iterator does not reset lastH, lastFH properly. Ref: https://github.com/prometheus/prometheus/issues/16681 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
This commit is contained in:
parent
e6b838391a
commit
df94ecebab
@ -105,6 +105,7 @@ func (f *histogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram)
|
||||
}
|
||||
|
||||
func (f *histogramStatsIterator) setLastH(h *histogram.Histogram) {
|
||||
f.lastFH = nil
|
||||
if f.lastH == nil {
|
||||
f.lastH = h.Copy()
|
||||
} else {
|
||||
@ -113,6 +114,7 @@ func (f *histogramStatsIterator) setLastH(h *histogram.Histogram) {
|
||||
}
|
||||
|
||||
func (f *histogramStatsIterator) setLastFH(fh *histogram.FloatHistogram) {
|
||||
f.lastH = nil
|
||||
if f.lastFH == nil {
|
||||
f.lastFH = fh.Copy()
|
||||
} else {
|
||||
@ -125,7 +127,13 @@ func (f *histogramStatsIterator) getFloatResetHint(hint histogram.CounterResetHi
|
||||
return hint
|
||||
}
|
||||
if f.lastFH == nil {
|
||||
return histogram.NotCounterReset
|
||||
// If there was no previous histogram, this will not be used,
|
||||
// and if there was a previous integer histogram, this will
|
||||
// force a reset detection. The later can happen if we used the
|
||||
// iterator to read integer histograms before, but switched to
|
||||
// float histograms for whatever reason.
|
||||
// https://github.com/prometheus/prometheus/issues/16681
|
||||
return histogram.UnknownCounterReset
|
||||
}
|
||||
|
||||
if f.currentFH.DetectReset(f.lastFH) {
|
||||
@ -139,7 +147,10 @@ func (f *histogramStatsIterator) getResetHint(h *histogram.Histogram) histogram.
|
||||
return h.CounterResetHint
|
||||
}
|
||||
if f.lastH == nil {
|
||||
return histogram.NotCounterReset
|
||||
// If there was no previous histogram, this will not be used,
|
||||
// and if there was a previous float histogram, this will
|
||||
// force a reset detection.
|
||||
return histogram.UnknownCounterReset
|
||||
}
|
||||
|
||||
fh, prevFH := h.ToFloat(nil), f.lastH.ToFloat(nil)
|
||||
|
@ -68,7 +68,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
|
||||
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
|
||||
},
|
||||
expectedHints: []histogram.CounterResetHint{
|
||||
histogram.NotCounterReset,
|
||||
histogram.UnknownCounterReset,
|
||||
},
|
||||
},
|
||||
{
|
||||
@ -78,7 +78,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
|
||||
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
|
||||
},
|
||||
expectedHints: []histogram.CounterResetHint{
|
||||
histogram.NotCounterReset,
|
||||
histogram.UnknownCounterReset,
|
||||
histogram.CounterReset,
|
||||
},
|
||||
},
|
||||
@ -90,7 +90,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
|
||||
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
|
||||
},
|
||||
expectedHints: []histogram.CounterResetHint{
|
||||
histogram.NotCounterReset,
|
||||
histogram.UnknownCounterReset,
|
||||
histogram.UnknownCounterReset,
|
||||
histogram.CounterReset,
|
||||
},
|
||||
@ -141,6 +141,41 @@ func TestHistogramStatsDecoding(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHistogramStatsMixedUse(t *testing.T) {
|
||||
histograms := []*histogram.Histogram{
|
||||
tsdbutil.GenerateTestHistogramWithHint(2, histogram.UnknownCounterReset),
|
||||
tsdbutil.GenerateTestHistogramWithHint(4, histogram.UnknownCounterReset),
|
||||
tsdbutil.GenerateTestHistogramWithHint(0, histogram.UnknownCounterReset),
|
||||
}
|
||||
|
||||
series := newHistogramSeries(histograms)
|
||||
it := series.Iterator(nil)
|
||||
|
||||
statsIterator := NewHistogramStatsIterator(it)
|
||||
|
||||
expectedHints := []histogram.CounterResetHint{
|
||||
histogram.UnknownCounterReset,
|
||||
histogram.NotCounterReset,
|
||||
histogram.UnknownCounterReset,
|
||||
}
|
||||
actualHints := make([]histogram.CounterResetHint, 3)
|
||||
typ := statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
_, h := statsIterator.AtHistogram(nil)
|
||||
actualHints[0] = h.CounterResetHint
|
||||
typ = statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
_, h = statsIterator.AtHistogram(nil)
|
||||
actualHints[1] = h.CounterResetHint
|
||||
typ = statsIterator.Next()
|
||||
require.Equal(t, chunkenc.ValHistogram, typ)
|
||||
_, fh := statsIterator.AtFloatHistogram(nil)
|
||||
actualHints[2] = fh.CounterResetHint
|
||||
|
||||
require.Equal(t, chunkenc.ValNone, statsIterator.Next())
|
||||
require.Equal(t, expectedHints, actualHints)
|
||||
}
|
||||
|
||||
type histogramSeries struct {
|
||||
histograms []*histogram.Histogram
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user