Skip to content

Commit

Permalink
Call compact for operators that don't compact native histograms
Browse files Browse the repository at this point in the history 
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
  • Loading branch information
@zenador
zenador committed May 11, 2023
1 parent 57b62b6 commit e5ebdb8
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 72 deletions.
14 changes: 8 additions & 6 deletions promql/engine.go
View file
Expand Up @@ -2415,20 +2415,19 @@ func vectorElemBinop(op parser.ItemType, lhs, rhs float64, hlhs, hrhs *histogram
// The histogram being added must have the larger schema
// code (i.e. the higher resolution).
if hrhs.Schema >= hlhs.Schema {
return 0, hlhs.Copy().Add(hrhs), true
return 0, hlhs.Copy().Add(hrhs).Compact(0), true
}
return 0, hrhs.Copy().Add(hlhs), true
return 0, hrhs.Copy().Add(hlhs).Compact(0), true
}
return lhs + rhs, nil, true
case parser.SUB:
if hlhs != nil && hrhs != nil {
// The histogram being subtracted must have the larger schema
// code (i.e. the higher resolution).
if hrhs.Schema >= hlhs.Schema {
return 0, hlhs.Copy().Sub(hrhs), true
return 0, hlhs.Copy().Sub(hrhs).Compact(0), true
}
// return 0, hrhs.Copy().Sub(hlhs).Scale(-1), true
return 0, hrhs.Copy().Scale(-1).Add(hlhs), true
return 0, hrhs.Copy().Scale(-1).Add(hlhs).Compact(0), true
}
return lhs - rhs, nil, true
case parser.MUL:
Expand Down Expand Up @@ -2745,7 +2744,7 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without
continue
}
if aggr.hasHistogram {
aggr.histogramValue = aggr.histogramMean
aggr.histogramValue = aggr.histogramMean.Compact(0)
} else {
aggr.floatValue = aggr.floatMean
}
Expand Down Expand Up @@ -2794,6 +2793,9 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without
// TODO(zenador): Issue warning when plumbing is in place.
continue
}
if aggr.hasHistogram {
aggr.histogramValue.Compact(0)
}
default:
// For other aggregations, we already have the right value.
}
Expand Down
87 changes: 21 additions & 66 deletions promql/engine_test.go
View file
Expand Up @@ -3970,11 +3970,9 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {
// TODO(codesome): Integrate histograms into the PromQL testing framework
// and write more tests there.
cases := []struct {
histograms []histogram.Histogram
expected histogram.FloatHistogram
expectedAvg histogram.FloatHistogram
expected2 histogram.FloatHistogram
expectedAvg2 histogram.FloatHistogram
histograms []histogram.Histogram
expected histogram.FloatHistogram
expectedAvg histogram.FloatHistogram
}{
{
histograms: []histogram.Histogram{
Expand Down Expand Up @@ -4042,44 +4040,6 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {
},
},
expected: histogram.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 0,
ZeroThreshold: 0.001,
ZeroCount: 14,
Count: 93,
Sum: 4691.2,
PositiveSpans: []histogram.Span{
{Offset: 0, Length: 3},
{Offset: 0, Length: 4},
},
PositiveBuckets: []float64{3, 8, 2, 5, 3, 2, 2},
NegativeSpans: []histogram.Span{
{Offset: 0, Length: 4},
{Offset: 0, Length: 2},
{Offset: 3, Length: 3},
},
NegativeBuckets: []float64{2, 6, 8, 4, 15, 9, 10, 10, 4},
},
expectedAvg: histogram.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 0,
ZeroThreshold: 0.001,
ZeroCount: 3.5,
Count: 23.25,
Sum: 1172.8,
PositiveSpans: []histogram.Span{
{Offset: 0, Length: 3},
{Offset: 0, Length: 4},
},
PositiveBuckets: []float64{0.75, 2, 0.5, 1.25, 0.75, 0.5, 0.5},
NegativeSpans: []histogram.Span{
{Offset: 0, Length: 4},
{Offset: 0, Length: 2},
{Offset: 3, Length: 3},
},
NegativeBuckets: []float64{0.5, 1.5, 2, 1, 3.75, 2.25, 2.5, 2.5, 1},
},
expected2: histogram.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 0,
ZeroThreshold: 0.001,
Expand All @@ -4096,7 +4056,7 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {
},
NegativeBuckets: []float64{2, 6, 8, 4, 15, 9, 10, 10, 4},
},
expectedAvg2: histogram.FloatHistogram{
expectedAvg: histogram.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 0,
ZeroThreshold: 0.001,
Expand Down Expand Up @@ -4190,11 +4150,11 @@ func TestNativeHistogram_Sum_Count_Add_AvgOperator(t *testing.T) {

// sum_over_time().
queryString = fmt.Sprintf("sum_over_time(%s[%dm:1m])", seriesNameOverTime, offset)
queryAndCheck(queryString, newTs, []Sample{{T: newTs, H: &c.expected2, Metric: labels.EmptyLabels()}})
queryAndCheck(queryString, newTs, []Sample{{T: newTs, H: &c.expected, Metric: labels.EmptyLabels()}})

// avg_over_time().
queryString = fmt.Sprintf("avg_over_time(%s[%dm:1m])", seriesNameOverTime, offset)
queryAndCheck(queryString, newTs, []Sample{{T: newTs, H: &c.expectedAvg2, Metric: labels.EmptyLabels()}})
queryAndCheck(queryString, newTs, []Sample{{T: newTs, H: &c.expectedAvg, Metric: labels.EmptyLabels()}})
})
idx0++
}
Expand Down Expand Up @@ -4252,17 +4212,16 @@ func TestNativeHistogram_SubOperator(t *testing.T) {
ZeroThreshold: 0.001,
ZeroCount: 2,
PositiveSpans: []histogram.Span{
{Offset: 0, Length: 4},
{Offset: 0, Length: 0},
{Offset: 0, Length: 3},
{Offset: 0, Length: 2},
{Offset: 1, Length: 4},
},
PositiveBuckets: []float64{1, 1, 0, 2, 1, 1, 1},
PositiveBuckets: []float64{1, 1, 2, 1, 1, 1},
NegativeSpans: []histogram.Span{
{Offset: 1, Length: 4},
{Offset: 2, Length: 0},
{Offset: 2, Length: 3},
{Offset: 1, Length: 2},
{Offset: 1, Length: 1},
{Offset: 4, Length: 3},
},
NegativeBuckets: []float64{1, 1, 0, 7, 5, 5, 2},
NegativeBuckets: []float64{1, 1, 7, 5, 5, 2},
},
},
{
Expand Down Expand Up @@ -4309,15 +4268,13 @@ func TestNativeHistogram_SubOperator(t *testing.T) {
ZeroThreshold: 0.001,
ZeroCount: 2,
PositiveSpans: []histogram.Span{
{Offset: 0, Length: 4},
{Offset: 0, Length: 0},
{Offset: 0, Length: 3},
{Offset: 0, Length: 1},
{Offset: 1, Length: 5},
},
PositiveBuckets: []float64{1, 0, 1, 2, 1, 1, 1},
PositiveBuckets: []float64{1, 1, 2, 1, 1, 1},
NegativeSpans: []histogram.Span{
{Offset: 1, Length: 4},
{Offset: 2, Length: 0},
{Offset: 2, Length: 3},
{Offset: 4, Length: 3},
},
NegativeBuckets: []float64{-2, 2, 2, 7, 5, 5, 2},
},
Expand Down Expand Up @@ -4366,15 +4323,13 @@ func TestNativeHistogram_SubOperator(t *testing.T) {
ZeroThreshold: 0.001,
ZeroCount: -2,
PositiveSpans: []histogram.Span{
{Offset: 0, Length: 4},
{Offset: 0, Length: 0},
{Offset: 0, Length: 3},
{Offset: 0, Length: 1},
{Offset: 1, Length: 5},
},
PositiveBuckets: []float64{-1, 0, -1, -2, -1, -1, -1},
PositiveBuckets: []float64{-1, -1, -2, -1, -1, -1},
NegativeSpans: []histogram.Span{
{Offset: 1, Length: 4},
{Offset: 2, Length: 0},
{Offset: 2, Length: 3},
{Offset: 4, Length: 3},
},
NegativeBuckets: []float64{2, -2, -2, -7, -5, -5, -2},
},
Expand Down

0 comments on commit e5ebdb8

Please sign in to comment.