-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
ci: Cache postgres volume after first migration #3488
Conversation
Should prevent us from doing migrations over and over for the same Sentry image
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3488 +/- ##
=======================================
Coverage 98.06% 98.06%
=======================================
Files 3 3
Lines 207 207
=======================================
Hits 203 203
Misses 4 4 ☔ View full report in Codecov by Sentry. |
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.
Can we have an env or something like that to punch through cache, I don't think of a reason which someone would need this right now (not using cache), and we can always add it later, what do you think?
On another matter, shouldn't we take postgres, clickhouse version into cache id, so if that gets changed, cache gets invalidated?
Don't think this would be useful as this is only happening on CI and changing the cache env variable would require a new commit where we can simply bump the cache key or disable the cache step if we want to.
Don't think so as we should be fine with upgrades too. We still run migrations, upgrade etc. We just happen to start from a warm state which is quite similar to our upgrade test. |
key: db-volumes-v4-${{ steps.cache_key.outputs.SENTRY_IMAGE_SHA }}-${{ steps.cache_key.outputs.SNUBA_IMAGE_SHA }} | ||
restore-keys: | | ||
db-volumes-v4-${{ steps.cache_key.outputs.SENTRY_IMAGE_SHA }} | ||
db-volumes-v4- |
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.
It does not seem like this is intentional? Perhaps
db-volumes-v4-${{ steps.cache_key.outputs.SNUBA_IMAGE_SHA }}
?
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.
Completely intentional. These are prefix matched in order for stale cache restoration. So if we cannot match on both sentry and snuba image SHA, then we fallback to one with sentry image sha regardless of snuba, and finally, it just restores any cache as long as it is v4.
This is great, wondering how much time this shaves off CI time |
It is in the description 😅:
|
Follow up to #3488 A new record: 2m 8s for installing self-hosted: ![image](https://github.com/user-attachments/assets/7cc6409d-5388-49ba-ad87-b7a1e99c9acc)
This patch caches all DB volumes based on the sentry and snuba images to avoid doing the same migrations over and over for every test run.
This shaved off a whole minute from "Install self-hosted" jobs (so ~20% speed increase).
Left side: cached re-run -- Right side: no-cache initial run