From 80e977aae6656940f7b48ec7fa24f6cffd47b184 Mon Sep 17 00:00:00 2001 From: zenador Date: Wed, 25 Oct 2023 00:36:07 +0800 Subject: [PATCH] Remove `NewPossibleNonCounterInfo` and minimise creating empty annotations (#13012) * Remove NewPossibleNonCounterInfo until it can be made more efficient, and avoid creating empty annotations as much as possible Signed-off-by: Jeanette Tan --- CHANGELOG.md | 4 ++++ promql/engine.go | 2 +- promql/functions.go | 14 +++----------- tsdb/db_test.go | 3 ++- util/annotations/annotations.go | 3 +++ 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a0bb1419..507cf664f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## unreleased + +* [ENHANCEMENT] PromQL: Reduce inefficiency introduced by warnings/annotations, and temporarily remove NewPossibleNonCounterInfo. #13012 + ## 2.48.0-rc.0 / 2023-10-10 * [CHANGE] Remote-write: respect Retry-After header on 5xx errors. #12677 diff --git a/promql/engine.go b/promql/engine.go index 161aa85ac..423849567 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2535,7 +2535,7 @@ type groupedAggregation struct { func (ev *evaluator) aggregation(e *parser.AggregateExpr, grouping []string, param interface{}, vec Vector, seriesHelper []EvalSeriesHelper, enh *EvalNodeHelper) (Vector, annotations.Annotations) { op := e.Op without := e.Without - annos := annotations.Annotations{} + var annos annotations.Annotations result := map[uint64]*groupedAggregation{} orderedResult := []*groupedAggregation{} var k int64 diff --git a/promql/functions.go b/promql/functions.go index ecd6f7c8b..5fd54f6c7 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -77,7 +77,7 @@ func extrapolatedRate(vals []parser.Value, args parser.Expressions, enh *EvalNod resultHistogram *histogram.FloatHistogram firstT, lastT int64 numSamplesMinusOne int - annos = annotations.Annotations{} + annos annotations.Annotations ) // We need either at least two Histograms and no Floats, or at least two @@ -88,14 +88,6 @@ func extrapolatedRate(vals []parser.Value, args parser.Expressions, enh *EvalNod return enh.Out, annos.Add(annotations.NewMixedFloatsHistogramsWarning(metricName, args[0].PositionRange())) } - if isCounter && metricName != "" && len(samples.Floats) > 0 && - !strings.HasSuffix(metricName, "_total") && - !strings.HasSuffix(metricName, "_sum") && - !strings.HasSuffix(metricName, "_count") && - !strings.HasSuffix(metricName, "_bucket") { - annos.Add(annotations.NewPossibleNonCounterInfo(metricName, args[0].PositionRange())) - } - switch { case len(samples.Histograms) > 1: numSamplesMinusOne = len(samples.Histograms) - 1 @@ -642,7 +634,7 @@ func funcQuantileOverTime(vals []parser.Value, args parser.Expressions, enh *Eva return enh.Out, nil } - annos := annotations.Annotations{} + var annos annotations.Annotations if math.IsNaN(q) || q < 0 || q > 1 { annos.Add(annotations.NewInvalidQuantileWarning(q, args[0].PositionRange())) } @@ -1105,7 +1097,7 @@ func funcHistogramFraction(vals []parser.Value, args parser.Expressions, enh *Ev func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) { q := vals[0].(Vector)[0].F inVec := vals[1].(Vector) - annos := annotations.Annotations{} + var annos annotations.Annotations if math.IsNaN(q) || q < 0 || q > 1 { annos.Add(annotations.NewInvalidQuantileWarning(q, args[0].PositionRange())) diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 773561c6c..ec5769077 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -1933,7 +1933,8 @@ func TestQuerierWithBoundaryChunks(t *testing.T) { // The requested interval covers 2 blocks, so the querier's label values for blockID should give us 2 values, one from each block. b, ws, err := q.LabelValues(ctx, "blockID") require.NoError(t, err) - require.Equal(t, annotations.Annotations{}, ws) + var nilAnnotations annotations.Annotations + require.Equal(t, nilAnnotations, ws) require.Equal(t, []string{"1", "2"}, b) } diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index 9cfbb121f..1d339646e 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -50,6 +50,9 @@ func (a *Annotations) Add(err error) Annotations { // the first in-place, and returns the merged first Annotation for convenience. func (a *Annotations) Merge(aa Annotations) Annotations { if *a == nil { + if aa == nil { + return nil + } *a = Annotations{} } for key, val := range aa {