Skip to content
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

Performance Regression Testing Revamp Stage 1 #4602

Merged
merged 166 commits into from
Apr 12, 2022

Conversation

nathaniel-may
Copy link
Contributor

@nathaniel-may nathaniel-may commented Jan 20, 2022

resolves #4021

Description

This is the first part of a multi-PR feature. Merging this will have no effect on the code base, but should be reviewed in full because this code will be activated by the actions that are under development in #4872. The goal of the overarching feature is to run performance regression tests on PRs with minimal failures due to noise. This work shifts the entire strategy for detecting performance regressions away from fully modeling performance characteristics on each test, to sampling and comparing to a baseline model. The strategy and the math to support it is outlined in detail in the README.

To start, this PR only includes a single test project with 2000 model files for the performance of dbt parse, but once the actions are merged it will be able to run efficiently on each PR. The next step is to add more commands and projects. This stage is outlined in #4768.

Reviewers

Please read the README in detail. It is intended to provide all the information necessary to develop this without me. If there's something that's unclear, please bring it up.

To navigate the runner, start in main::run_app which handles all the IO, then hit the two top-level functions: fs::model and calculate::regressions. The split between fs and calculate modules isn't perfect and I could probably have some better naming of these functions. Suggestions are welcome.

The runner is not perfect, and I have left TODOs for all the places I think we should improve. It will be worked on in subsequent stages of this feature so there will be time to fix more of these. If anything looks below the code quality we want to merge, I am happy to make those changes in this PR.

Please also review the 1 change PR #4875 if it is not already merged. It updates the test project by regenerating it with a ref. I split it out because it touches 2000 files.

Tasks

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@cla-bot cla-bot bot added the cla:yes label Jan 20, 2022
@nathaniel-may nathaniel-may marked this pull request as ready for review March 16, 2022 16:35
@nathaniel-may nathaniel-may requested a review from a team as a code owner March 16, 2022 16:35
@nathaniel-may nathaniel-may changed the title Regression Testing Revamp Performance Regression Testing Revamp Stage 1 Mar 23, 2022
performance/README.md Outdated Show resolved Hide resolved
@leahwicz
Copy link
Contributor

Overall this LGTM!

Two thing that I would feel like would be good to add to the README is:

  • What happens/example of when a release is completed and now we want to set a baseline for that release- what runs, on what branch, what gets committed, etc.
  • What happens when we test a commit- you mention this in the Concrete Example but it doesn't mention scenario details such as: I commit to main vs I'm backporting to 1.0.latest, if I'm on 1.0.latest then I'm being compared to the latest final release or RC? Basically the decision tree of what is the baseline that I'm getting compared to

performance/runner/src/fs.rs Outdated Show resolved Hide resolved
performance/runner/src/fs.rs Outdated Show resolved Hide resolved
@nathaniel-may nathaniel-may merged commit ec46be7 into main Apr 12, 2022
@nathaniel-may nathaniel-may deleted the nate/regression-testing-2 branch April 12, 2022 20:00
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-14] Enable Regression Testing Notifications
4 participants