Stop the ad dashboard losing its graphs to a timed-out query - #228
Merged
Merged
Conversation
/dashboard/ads renders no graphs, intermittently: the chart is replaced by "No delivery in this range." and the campaign sparklines by "no traffic yet", over a network that delivered 177,844 impressions in the window. Delivery was healthy the whole time. Three separate faults. 1. The reporting RPCs sat at the statement_timeout wall. Measured over 24h: ad_account_series avg 1,453ms / max 8,039ms, and ad_campaign_daily_series avg 1,719ms / max 8,027ms, against the 8s timeout on `authenticated` -- 14 of ~150 calls returned HTTP 500, so roughly one load in ten came back empty and a reload "fixed" it. Not the RLS nested loop from #226; that fix held and the plan is a clean hash join. Every load simply re-aggregated the whole archive: Seq Scan on ad_impressions (actual rows=177858) Filter: ((NOT duplicate) AND (ts >= now() - '30 days')) Rows Removed by Filter: 198394 Buffers: shared hit=9767 -- the entire 78MB heap, per load and the page fires three such RPCs per render. ad_impressions is 376k rows growing ~90k/day, so cost tracked the archive rather than the window asked for. No index fixes that: the 30-day window selects 47% of the table. A covering partial index measured 157ms against 194ms for the seq scan -- a 20% gain on a query needing to be 10x faster. So pre-aggregate. Three rollups, each at the grain its surface plots (owner/hour, campaign/day, slot/day), refreshed by pg_cron every ten minutes. Closed periods come from the rollup, the current hour and current day always from raw, so a stale or un-run refresh can never show a wrong live edge and the raw slice is never wider than a day. A 30-day account query now reads ~720 rows where it read 246,506. Verified by swapping the functions inside a REPEATABLE READ transaction and diffing old against new across all 8 ranges and 3 day windows: byte-identical output, 3,312ms -> 380ms for the battery. 2. ad_campaign_daily_series had outgrown PostgREST's 1000-row cap. 139 campaigns x 30 days is 2,731 rows and the call was returning `206 Partial Content, content-range 0-999/2731`. With no ORDER BY, which two thirds got dropped was down to join order: campaigns lost days off their sparkline and five lost every row, rendering "no traffic yet" beside a row reading "Impressions: 24". It returns one jsonb array now -- one row, whatever the campaign count. Live, that took the page from 9 "no traffic yet" rows to the 4 that really have no delivery. 3. AccountTrend and MiniTrend still asserted emptiness on failure. Both receive a zero-filled series when the query dies, so "No delivery in this range." was a claim about the network made from a query that never returned -- and it sat directly under the banner #226 added saying the figures could not be loaded. They now take `failed` and say so. Tracked per loader, since the chart and the sparklines come from different RPCs and either can fail alone. Both migrations are applied to prod. The rollups reconcile exactly against raw events across all 57 closed days on both the advertiser and publisher sides, including after the first scheduled mid-day refresh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7rVKbcZdT7tABboosr1Re
ThreatCrush Security Scan39 finding(s) HIGH/CRITICAL: 2 | MEDIUM: 28 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The ad dashboard renders no graphs, intermittently — the chart replaced by "No delivery in this range." and the campaign sparklines by "no traffic yet", over a network that delivered 177,844 impressions in that window. Delivery was healthy throughout. Three separate faults produce that one symptom.
1. The reporting RPCs were sitting at the statement_timeout wall
Measured over 24h of production edge logs:
ad_account_seriesad_campaign_daily_seriesad_campaign_totalsAgainst the 8s
statement_timeoutonauthenticated. 14 of ~150 reporting calls returned HTTP 500, so roughly one page load in ten came back empty and a reload "fixed" it. That intermittency is the tell.This is not the RLS nested loop from #226 — that fix held, and the plan is a clean hash join. Every dashboard load simply re-aggregated the entire archive from scratch:
and the page fires three such RPCs per render.
ad_impressionsis 376k rows growing ~90k/day, so the cost tracked the size of the archive rather than the window being asked for.No index fixes this. The 30-day window selects 47% of the table, far past where an index scan can win. A covering partial index (kept, it serves the live-edge reads) measured 157ms against 194ms for the seq scan — a 20% gain on a query that needed to be 10x faster.
So pre-aggregate. Three rollups, each at the grain its surface actually plots:
ad_stats_owner_hourlyad_stats_campaign_dailyad_stats_slot_dailyRefreshed by
pg_cronevery ten minutes. Closed periods come from the rollup; the current hour and current UTC day always come from raw, so a stale — or completely un-run — refresh can never show a wrong live edge, and the raw slice read is never wider than one day. A 30-day account query now reads ~720 rows where it read 246,506.Per-campaign is deliberately daily, not hourly:
(campaign_id, hour)is 2,997 distinct cells in a single day against 139 campaigns, ~170k rows over the archive — barely smaller than the raw table, so it would buy nothing. Account-wide has no campaign dimension, which is what makes hourly affordable there, and hourly is required there because the 1W range plots 4-hour buckets.2.
ad_campaign_daily_serieshad outgrown PostgREST's 1000-row cap139 campaigns x 30 days is 2,731 rows. The call was coming back
206 Partial Content, content-range 0-999/2731. The function has noORDER BY, so which two thirds got dropped was down to join order: campaigns silently lost days off the end of their sparkline, and five lost every row — rendering "no traffic yet" beside a row reading "Impressions: 24".It returns a single
jsonbarray now: one row, whatever the campaign count. Live, this took the page from 9 "no traffic yet" rows down to the 4 that genuinely have no delivery.3. The chart still asserted emptiness when the query had failed
AccountTrendandMiniTrendboth receive a zero-filled series on failure, so"No delivery in this range."was a statement of fact about the network made from a query that never returned — sitting directly underneath the banner #226 added saying the figures could not be loaded. Both now takefailedand say so instead. Tracked per loader, because the chart and the sparklines come from different RPCs and either can fail on its own.Verification
REPEATABLE READtransaction and diffed old vs new across all 8 ranges and 3 day-windows — byte-identical output on every relation, and 3,312ms -> 380ms for the whole battery./dashboard/adsrenders its chart and 137 sparklines with no failure banner.Both migrations are already applied to prod (the DB change is what fixed the symptom; the old TypeScript parses the jsonb fine, so the page improved before this merges). Rollback for the RPCs is
20260901153000_ad_reporting_rpcs_security_definer.sql, plus adrop function ad_campaign_daily_series(integer)first since the return type changed.Unrelated, found while checking the other ad surfaces
/dashboard/ads/slotshas a 21s TTFB. Every DB query on it is under 0.3s, so it is the un-timeboxedfetchSupportedTokens()awaited after thePromise.all. Pre-existing and untouched here.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y7rVKbcZdT7tABboosr1Re