Skip to content

Commit

Permalink
Updated how NH are counted to samples, update testing to check NaN's …
Browse files Browse the repository at this point in the history
…and NH's
  • Loading branch information
tinitiuset committed Dec 18, 2024
1 parent 344bfc7 commit 3eaa045
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 8 deletions.
20 changes: 20 additions & 0 deletions pkg/streamingpromql/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2824,6 +2824,8 @@ func TestQueryStats(t *testing.T) {
start_series 0 1 _ _ _ _ _ _ _ _ _
end_series _ _ _ _ _ 5 6 7 8 9 10
sparse_series 0 _ _ _ _ _ _ 7 _ _ _
nan_series NaN NaN NaN NaN NaN NaN NaN NaN NaN NaN NaN
native_histogram_series {{schema:0 sum:2 count:4 buckets:[1 2 1]}} {{sum:2 count:4 buckets:[1 2 1]}}
`)

runQueryAndGetTotalSamples := func(t *testing.T, engine promql.QueryEngine, expr string, isInstantQuery bool) int64 {
Expand Down Expand Up @@ -2904,6 +2906,24 @@ func TestQueryStats(t *testing.T) {
expr: `dense_series{} + end_series{}`,
expectedTotalSamples: 11 + 6,
},
"instant vector selector with NaNs": {
expr: `nan_series{}`,
isInstantQuery: true,
expectedTotalSamples: 1,
},
"range vector selector with NaNs": {
expr: `sum_over_time(nan_series{}[1m])`,
expectedTotalSamples: 11,
},
"instant vector selector with native histograms": {
expr: `native_histogram_series{}`,
isInstantQuery: false,
expectedTotalSamples: 78,
},
"range vector selector with native histograms": {
expr: `sum_over_time(native_histogram_series{}[1m])`,
expectedTotalSamples: 26,
},
}

for name, testCase := range testCases {
Expand Down
4 changes: 2 additions & 2 deletions pkg/streamingpromql/operators/functions/quantile.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func histogramQuantile(q float64, h *histogram.FloatHistogram) float64 {
}
}
// Due to numerical inaccuracies, we could end up with a higher count
// than h.Count. Thus, make sure count is never higher than h.Count.
// than h.CountPoints. Thus, make sure count is never higher than h.CountPoints.
if count > h.Count {
count = h.Count
}
Expand All @@ -233,7 +233,7 @@ func histogramQuantile(q float64, h *histogram.FloatHistogram) float64 {
return bucket.Upper
}

// NaN observations increase h.Count but not the total number of
// NaN observations increase h.CountPoints but not the total number of
// observations in the buckets. Therefore, we have to use the forward
// iterator to find percentiles. We recognize histograms containing NaN
// observations by checking if their h.Sum is NaN.
Expand Down
2 changes: 1 addition & 1 deletion pkg/streamingpromql/operators/functions/range_vectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var CountOverTime = FunctionOverRangeVectorDefinition{

func countOverTime(step *types.RangeVectorStepData, _ float64, _ types.EmitAnnotationFunc) (float64, bool, *histogram.FloatHistogram, error) {
fPointCount := step.Floats.Count()
hPointCount := step.Histograms.Count()
hPointCount := step.Histograms.CountPoints()

if fPointCount == 0 && hPointCount == 0 {
return 0, false, nil, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe
continue
}

v.Stats.TotalSamples++

// if (f, h) have been set by PeekPrev, we do not know if f is 0 because that's the actual value, or because
// the previous value had a histogram.
// PeekPrev will set the histogram to nil, or the value to 0 if the other type exists.
Expand All @@ -146,6 +144,12 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe
data.Histograms = append(data.Histograms, promql.HPoint{T: stepT, H: h})
lastHistogramT = t
lastHistogram = h

// For consistency with PromQL engine:
// h.Size() returns the size of the histogram in bytes,
// add 8 bytes to account for the timestamp,
// and divide by 16 to get the number of samples.
v.Stats.TotalSamples += int64((h.Size() + 8) / 16)
} else {
// Only create the slice once we know the series is a histogram or not.
if len(data.Floats) == 0 {
Expand All @@ -157,6 +161,7 @@ func (v *InstantVectorSelector) NextSeries(ctx context.Context) (types.InstantVe
}
}
data.Floats = append(data.Floats, promql.FPoint{T: stepT, F: f})
v.Stats.TotalSamples++
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (m *RangeVectorSelector) NextStepSamples() (*types.RangeVectorStepData, err
m.stepData.RangeStart = rangeStart
m.stepData.RangeEnd = rangeEnd

m.Stats.TotalSamples += int64(m.stepData.Floats.Count() + m.stepData.Histograms.Count())
m.Stats.TotalSamples += int64(m.stepData.Floats.Count() + m.stepData.Histograms.CountSamples())

return m.stepData, nil
}
Expand Down
19 changes: 17 additions & 2 deletions pkg/streamingpromql/types/hpoint_ring_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,26 @@ func (v HPointRingBufferView) Last() (promql.HPoint, bool) {
return v.buffer.pointAt(v.size - 1), true
}

// Count returns the number of points in this ring buffer view.
func (v HPointRingBufferView) Count() int {
// CountPoints returns the number of points in this ring buffer view.
func (v HPointRingBufferView) CountPoints() int {
return v.size
}

// CountSamples returns the size of the HPointRingBufferView compared to the size of an FPointRingBufferView.
// For consistency with PromQL engine:
// H.Size() returns the size of the histogram in bytes,
// add 8 bytes to account for the timestamp,
// and divide by 16 to get the number of samples.
func (v HPointRingBufferView) CountSamples() int {
var totalSize int
for i := 0; i < v.size; i++ {
totalSize += v.buffer.pointAt(i).H.Size() + 8
}
totalSize = totalSize / 16

return totalSize
}

// Any returns true if this ring buffer view contains any points.
func (v HPointRingBufferView) Any() bool {
return v.size != 0
Expand Down

0 comments on commit 3eaa045

Please sign in to comment.