fix: warming and filter #25674
Merged
fix: warming and filter #25674
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: posthog/caching/warming.py
Did you find this useful? React with a 👍 or 👎 |
| dashboard = None | ||
|
|
||
| tag_queries(team_id=insight.team_id, insight_id=insight.pk, trigger="warmingV2") | ||
| if dashboard_id: | ||
| tag_queries(dashboard_id=dashboard_id) | ||
| dashboard = insight.dashboards.get(pk=dashboard_id) | ||
| dashboard = insight.dashboards.filter(pk=dashboard_id).first() |
There was a problem hiding this comment.
why do we need to change this? Insights can be added to a dashboard multiple times?
There was a problem hiding this comment.
Get throws an exception if it doesn’t exist, but we were expecting a none, which was leading to sentry errors
anirudhpillai
approved these changes
Oct 18, 2024
There was a problem hiding this comment.
Looks good!
We can also check this to see if we hit concurrency limit a lot more once this is deployed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Lots of queries are failing at cache warming because estimates of runtime or actual runtime are too long.
This is causing cache warming to fail a lot for larger users (like us).
Changes
Allow cache warming to take as long as running the query ourselves.
Also add a pretty simple filter to funnels to cut back on the number of events that have to be serialized in certain big funnel queries.
Also fix issues sentry bot pulled in of insight and dashboard throwing errors sometimes.
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Tested the query in metabase