fix(dashboards): prefetch alert threshold and subscribers on read paths - #70832
Conversation
AlertSerializer serializes threshold and subscribed_users for every alert, but the
alert prefetch on dashboard GET and insight list only select_related created_by. Each
alert on the page therefore costs two extra Postgres queries, so alert-heavy dashboards
degrade to O(alerts) queries per load.
Extend both prefetch sites with select_related("threshold") and
prefetch_related("subscribed_users"). Measured on a 30-tile dashboard with two alerts
per insight: 176 -> 27 queries, wall time -72%; the query count is now flat as tiles grow.
Adds constant-query-count regression tests for both endpoints; each fails on the
previous prefetch and passes with this one.
Generated-By: PostHog Code
Task-Id: edcf7979-e685-4b20-a3b1-940d04c033ad
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from |
|
Reviews (1): Last reviewed commit: "fix(dashboards): prefetch alert threshol..." | Re-trigger Greptile |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained N+1 query fix adding select_related/prefetch_related to existing alert querysets, with new regression tests proving the fix; no schema, API contract, or security changes, and diff matches the description.
- Author wrote 0% of the modified lines and has 13 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 12L, 2F substantive, 100L/4F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (100L, 4F, two-areas, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 182dd44 · reviewed head 4ea189d |
🤖 CI report
|
|
Retaining stamphog approval — delta since last review classified as |
Per-change benchmark of this fix(Comment posted by PostHog Code — benchmarked each half of the prefetch change separately.) Same harness as before: local PG, dashboard GET with 30 tiles, 2 alerts per insight (one threshold + one detector, 1 subscriber each → 60 alerts, 30 thresholds). Median of 19 requests.
Cost of the heavier prefetch statement itself (60 alerts, statement-level micro-benchmark): the threshold LEFT JOIN takes the alert prefetch from 3.5ms → 4.7ms; adding the Reading: the |
1 participant
TL;DR (eli5)
When a dashboard has alerts on its insights, opening it made the server ask the database two extra questions for every single alert (who's subscribed? what's the threshold?). A dashboard with lots of alerted insights asked hundreds of tiny questions one by one. Now it asks once for all of them upfront — same answer, a fraction of the round trips.
Problem
AlertSerializerserializesthresholdandsubscribed_usersfor every alert, but the alert prefetch on dashboard GET (DashboardSerializer.get_tiles) and insight list only doesselect_related("created_by"). Each alert on the page costs two extra Postgres queries, so alert-heavy dashboards degrade to O(alerts) queries per load. Found while benchmarking dashboard loads against real dashboards: an alert-per-insight dashboard paid ~3 extra queries per tile, dwarfing its fixed query cost.Why: follow-up from the query benchmarking of #70085 — the biggest per-tile query multiplier on dashboard load turned out to be alerts, not the fixed costs that PR addresses.
Changes
select_related("created_by", "threshold")+prefetch_related("subscribed_users").How did you test this code?
test_insight_alerts_do_not_nplus1_dashboard_gets(dashboard GET) andtest_listing_insights_with_alerts_does_not_nplus1(insight list): constant-query-count tests that create insights with threshold + detector alerts and assert the per-request query count doesn't grow with more insights. Each catches the regression of droppingthreshold/subscribed_usersfrom its endpoint's prefetch — no existing test creates alerts on dashboard/listed insights, so this N+1 was invisible to the suite. Both fail on the previous prefetch and pass with this change (verified locally by stashing the fix).Automatic notifications
Docs update
No user-facing behavior change; no docs affected.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored with PostHog Code (Claude). Skills invoked: /improving-drf-endpoints, /writing-tests. The N+1 was located by benchmarking dashboard GETs per-commit for #70085, then diffing captured SQL against production dashboards; the repeating statements were per-alert
posthog_user(subscribed_users, serialized twice per alert) andposthog_thresholdselects. Fix validated with a standalone query-count benchmark (176 → 27 queries at 30 tiles) and the two new regression tests, run locally against Postgres + ClickHouse.Created with PostHog Code