Do we want build failures on performance regressions? #802
jdmarshall
started this conversation in
Ideas
Replies: 1 comment
|
I would be cautious of failing builds due to benchmarks. Is it possible to run the benchmarks and have some automation leave a comment on the PR with the results. That way, the results are always immediately visible. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
There's a young man who is the third person to attempt to fix #791 in a three day period. The first two were rejected because they resulted in a nearly 20% regression in the performance of the
metrics()call, which is already our most problematic call sequence. When I joined this project it was the most common complaint I found in the backlog. Unfortunately I was able to speed up literally everything exceptmetrics()in the time since then, most notablyaggregate(), which I am particularly proud of.The only open issue remaining in the
faceoffbacklog (since I'm the only one filing issues) was one to support putting limits on how many slow tests can be tolerated in a single branch of the code. I shipped that feature yesterday, in addition to being able to adjust the definition of 'slow'.I don't know if I'm intending to use this feature myself, yet. The jitter between runs is still relatively high. One of my smoke tests usually reports a 6-9% slowdown for one test, but in one run out of ten shows an almost 16% slowdown instead. If bench-node ever gets more sophisticated about detecting statistical anomalies then I am more likely to use it. I did end up leveraging the t-test logic in that library, which reduced the noise quite a bit but still leaves something to be desired. Unfortunately that level of statistical analysis is outside my wheelhouse so I cannot fix it myself.
The problem with flaky tests is that they turn CI failures into noisy alerts. Once people start tuning them out they start ignoring legitimately red builds (who hasn't caught a coworker blindly pushing 'rebuild' 3 times before you have to tell them there was a merge conflict of some sort and trunk is legitimately broken right now?)
I'm probably okay for now manually checking the benchmark job for changes that seem likely to cause regressions. However I don't know if that will scale once we have released under the Prometheus umbrella. I'm hoping for an uptick of third party contributions once we are back on people's radars as an active project.
Thoughts?
All reactions