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
timgl
pushed a commit
that referenced
this pull request
Oct 21, 2024
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
fuziontech
added a commit
that referenced
this pull request
Oct 21, 2024
* master: (100 commits) revert: perf: Speed up filtering persons (#25715) chore: count data ingested by source (#25714) fix(bi): Dont try to be too clever with variables (#25712) fix(cdp): Ask for kinesis stream name, not the ARN (#25711) fix(batch-exports): Deal with deeply nested events (#25710) fix(batch-exports): Ingest as JSON in Redshift super type (#25709) perf: Speed up filtering persons (#25604) feat(data-warehouse): Add button to cancel batch export backfill run (#25706) feat(cdp): add discord template (#25649) feat(max): Show query summary (#25696) feat(errors): Use exception list and remove stack trace raw (#25655) fix: smaller cohort resource usage for replay filters (#25699) chore: use meta columns in data visualization table (#25697) feat(cdp): add klaviyo templates (#25693) feat(max): Answer feedback buttons (#25670) feat(cdp): add google ads integration warning (#25698) feat(hog): print ast in console (#25608) feat(data-warehouse): edit SQL based import configs (#25685) fix(insights): handle negation property in cohort property filters (#25691) fix: warming and filter (#25674) ...
timgl
pushed a commit
that referenced
this pull request
Oct 25, 2024
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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