-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DEV-47346-extract-grafana-evaluation-into-goroutine #71
DEV-47346-extract-grafana-evaluation-into-goroutine #71
Conversation
run-test |
1 similar comment
run-test |
run-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
} else { | ||
evaluationsErrors = append(evaluationsErrors, apimodels.AlertEvalRunResult{UID: evalRequest.AlertRule.UID, EvalTime: evalRequest.EvalTime, RunResult: "success"}) | ||
} | ||
go func(ctx context.Context, request ngmodels.ExternalAlertEvaluationRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take into consideration the way Garafana did it with thetime.AfterFunc(time.Duration(int64(i)*step), func()
the step is: step = sch.baseInterval.Nanoseconds() / int64(len(readyToRun))
what is basically do is to spread the load on the query engine in the duration of the tick (but we can do it half tick)
this was an issue in the past in the data hub - but I didn't encounter it in the alerts - WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good point. why do we want to do it half of the tick? It will make things tight only in the first 30 seconds of the minute
run-test |
1 similar comment
run-test |
Grafana evaluations extracted to goroutines to enable skipping of the slow alerts and reduce API response time