From 340a9756d9cc8602a6adcc96cd56be0fe798fff6 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 18 Jul 2023 10:45:31 +0100 Subject: [PATCH 1/4] Alerting: Improve performance of matching captures This commit updates eval.go to improve the performance of matching captures in the general case. In some cases we have reduced the runtime of the function from 10s of minutes to a couple 100ms. In the case where no capture matches the exact labels, we revert to the current subset/superset match, but with a reduced search space due to grouping captures. --- pkg/services/ngalert/eval/eval.go | 38 ++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index 77a3d5e6ae271..789ac02694e54 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -302,10 +302,11 @@ type NumberValueCapture struct { } func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.QueryDataResponse) ExecutionResults { - // eval captures for the '__value_string__' annotation and the Value property of the API response. - captures := make([]NumberValueCapture, 0, len(execResp.Responses)) - captureVal := func(refID string, labels data.Labels, value *float64) { - captures = append(captures, NumberValueCapture{ + // captures contains the values of all instant queries and expressions for each dimension + captures := make(map[data.Fingerprint][]NumberValueCapture, len(execResp.Responses)) + captureFn := func(refID string, labels data.Labels, value *float64) { + fp := labels.Fingerprint() + captures[fp] = append(captures[fp], NumberValueCapture{ Var: refID, Value: value, Labels: labels.Copy(), @@ -357,7 +358,7 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q if frame.Fields[0].Len() == 1 { v = frame.At(0, 0).(*float64) // type checked above } - captureVal(frame.RefID, frame.Fields[0].Labels, v) + captureFn(frame.RefID, frame.Fields[0].Labels, v) } if refID == c.Condition { @@ -379,13 +380,28 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q if len(frame.Fields) == 1 { theseLabels := frame.Fields[0].Labels - for _, cap := range captures { - // matching labels are equal labels, or when one set of labels includes the labels of the other. - if theseLabels.Equals(cap.Labels) || theseLabels.Contains(cap.Labels) || cap.Labels.Contains(theseLabels) { - if frame.Meta.Custom == nil { - frame.Meta.Custom = []NumberValueCapture{} + fp := theseLabels.Fingerprint() + + // First look for a capture whose labels are an exact match + if groupedCaps, ok := captures[fp]; ok { + if frame.Meta.Custom == nil { + frame.Meta.Custom = []NumberValueCapture{} + } + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), groupedCaps...) + } else { + // If no exact match was found, look for captures whose labels are either subsets + // or supersets + for _, groupedCaps := range captures { + if len(groupedCaps) > 0 { + firstCap := groupedCaps[0] + // matching labels are equal labels, or when one set of labels includes the labels of the other. + if theseLabels.Equals(firstCap.Labels) || theseLabels.Contains(firstCap.Labels) || firstCap.Labels.Contains(theseLabels) { + if frame.Meta.Custom == nil { + frame.Meta.Custom = []NumberValueCapture{} + } + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), groupedCaps...) + } } - frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), cap) } } } From 5c487c12819fb73049b671d33f55248e158509af Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 18 Jul 2023 13:28:31 +0100 Subject: [PATCH 2/4] Add benchmarks --- pkg/services/ngalert/eval/eval_bench_test.go | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 pkg/services/ngalert/eval/eval_bench_test.go diff --git a/pkg/services/ngalert/eval/eval_bench_test.go b/pkg/services/ngalert/eval/eval_bench_test.go new file mode 100644 index 0000000000000..18e2d23ad3e92 --- /dev/null +++ b/pkg/services/ngalert/eval/eval_bench_test.go @@ -0,0 +1,59 @@ +package eval + +import ( + "context" + "github.com/grafana/grafana/pkg/util" + "strconv" + "testing" + "time" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/services/ngalert/models" +) + +func BenchmarkEvaluate(b *testing.B) { + var dataResp backend.QueryDataResponse + seedDataResponse(&dataResp, 10000) + var evaluator ConditionEvaluator = &conditionEvaluator{ + expressionService: &fakeExpressionService{ + hook: func(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) { + return &dataResp, nil + }, + }, + condition: models.Condition{ + Condition: "B", + }, + } + for i := 0; i < b.N; i++ { + _, err := evaluator.Evaluate(context.Background(), time.Now()) + if err != nil { + b.Fatalf("Unexpected error: %s", err) + } + } +} + +func seedDataResponse(r *backend.QueryDataResponse, n int) { + resps := make(backend.Responses, n) + for i := 0; i < n; i++ { + labels := data.Labels{ + "foo": strconv.Itoa(i), + "bar": strconv.Itoa(i + 1), + } + a, b := resps["A"], resps["B"] + a.Frames = append(a.Frames, &data.Frame{ + Fields: data.Fields{ + data.NewField("Time", labels, []time.Time{time.Now()}), + data.NewField("Value", labels, []*float64{util.Pointer(1.0)}), + }, + }) + b.Frames = append(b.Frames, &data.Frame{ + Fields: data.Fields{ + data.NewField("Value", labels, []*float64{util.Pointer(1.0)}), + }, + }) + resps["A"], resps["B"] = a, b + } + r.Responses = resps +} From cef8a5c14bd081f65b480d0d1502183adf7085a2 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 18 Jul 2023 13:59:36 +0100 Subject: [PATCH 3/4] Fix lint --- pkg/services/ngalert/eval/eval_bench_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/ngalert/eval/eval_bench_test.go b/pkg/services/ngalert/eval/eval_bench_test.go index 18e2d23ad3e92..600120cbe96fe 100644 --- a/pkg/services/ngalert/eval/eval_bench_test.go +++ b/pkg/services/ngalert/eval/eval_bench_test.go @@ -2,7 +2,6 @@ package eval import ( "context" - "github.com/grafana/grafana/pkg/util" "strconv" "testing" "time" @@ -11,6 +10,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/util" ) func BenchmarkEvaluate(b *testing.B) { From df2b61269361a98c543754028a5ae7850381c5c8 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Wed, 19 Jul 2023 16:46:53 +0100 Subject: [PATCH 4/4] Fix matching for Ref IDs --- pkg/services/ngalert/eval/eval.go | 38 +++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index 789ac02694e54..71a0aee28cdb2 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -303,14 +303,19 @@ type NumberValueCapture struct { func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.QueryDataResponse) ExecutionResults { // captures contains the values of all instant queries and expressions for each dimension - captures := make(map[data.Fingerprint][]NumberValueCapture, len(execResp.Responses)) + captures := make(map[string]map[data.Fingerprint]NumberValueCapture) captureFn := func(refID string, labels data.Labels, value *float64) { + m := captures[refID] + if m == nil { + m = make(map[data.Fingerprint]NumberValueCapture) + } fp := labels.Fingerprint() - captures[fp] = append(captures[fp], NumberValueCapture{ + m[fp] = NumberValueCapture{ Var: refID, Value: value, Labels: labels.Copy(), - }) + } + captures[refID] = m } // datasourceUIDsForRefIDs is a short-lived lookup table of RefID to DatasourceUID @@ -382,24 +387,23 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q theseLabels := frame.Fields[0].Labels fp := theseLabels.Fingerprint() - // First look for a capture whose labels are an exact match - if groupedCaps, ok := captures[fp]; ok { - if frame.Meta.Custom == nil { - frame.Meta.Custom = []NumberValueCapture{} - } - frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), groupedCaps...) - } else { - // If no exact match was found, look for captures whose labels are either subsets - // or supersets - for _, groupedCaps := range captures { - if len(groupedCaps) > 0 { - firstCap := groupedCaps[0] + for _, fps := range captures { + // First look for a capture whose labels are an exact match + if v, ok := fps[fp]; ok { + if frame.Meta.Custom == nil { + frame.Meta.Custom = []NumberValueCapture{} + } + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), v) + } else { + // If no exact match was found, look for captures whose labels are either subsets + // or supersets + for _, v := range fps { // matching labels are equal labels, or when one set of labels includes the labels of the other. - if theseLabels.Equals(firstCap.Labels) || theseLabels.Contains(firstCap.Labels) || firstCap.Labels.Contains(theseLabels) { + if theseLabels.Equals(v.Labels) || theseLabels.Contains(v.Labels) || v.Labels.Contains(theseLabels) { if frame.Meta.Custom == nil { frame.Meta.Custom = []NumberValueCapture{} } - frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), groupedCaps...) + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), v) } } }