Skip to content

perf(crons): Adjust specific environment monitor stats query#114277

Merged
scttcper merged 3 commits intomasterfrom
scttcper/monitor-stats-aggregate
Apr 29, 2026
Merged

perf(crons): Adjust specific environment monitor stats query#114277
scttcper merged 3 commits intomasterfrom
scttcper/monitor-stats-aggregate

Conversation

@scttcper
Copy link
Copy Markdown
Member

@scttcper scttcper commented Apr 29, 2026

Monitor stats has two query shapes. This optimizes the environment-backed shape, for example requests that include ?environment=... or otherwise resolve to MonitorEnvironment rows before querying check-ins.

In that shape, the endpoint already has a mapping from monitor_environment_id to monitor_id. This leaves monitor_id out of the aggregate so Postgres can answer from the existing (monitor_environment_id, date_added, status) index, then recovers monitor_id from the MonitorEnvironment rows it already loaded.

The no-monitor-environment fallback keeps the old aggregate shape because it has no environment-to-monitor mapping.

The query plan change from an environment-backed stats request:

Shape Plan Runtime
aggregate included monitor_id index scan on the same composite index, with heap reads for monitor_id about 5.5s
aggregate uses bucket, monitor_environment_id, status index-only scan on the same composite index, with a small number of heap fetches about 12.5ms

attempt to fix SENTRY-3VHC

Monitor stats already fetch monitor environment rows before querying check-ins. Use that mapping to leave monitor_id out of the aggregate when possible, so Postgres can stay on the monitor environment/date/status index.

Keep the legacy fallback shape for rows without monitor environments.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 29, 2026
scttcper and others added 2 commits April 28, 2026 20:45
mypy inferred the first monitor stats group_by assignment as a fixed-width tuple. Make it a variable-length string tuple so both aggregate shapes type check.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Keep the note next to the branch that switches the aggregate shape. That makes it clearer why the environment-backed path can omit monitor_id.
@scttcper scttcper changed the title perf(crons): Avoid heap reads in monitor stats perf(crons): Cover environment-scoped monitor stats Apr 29, 2026
@scttcper scttcper changed the title perf(crons): Cover environment-scoped monitor stats perf(crons): Adjust specific environment monitor stats query Apr 29, 2026
@scttcper
Copy link
Copy Markdown
Member Author

scttcper commented Apr 29, 2026

checking another one via EXPLAIN in redash

Old query shape:
- Plan: Index Scan on `sentry_moni_monitor_1fb26c_idx `
- Rows scanned: 10,479
- Output groups: 1,114
- Buffers: 292 hits, 10,292 reads, 6,521 writes
- Execution time: 5,487 ms

New query shape:
- Plan: Index Only Scan on `sentry_moni_monitor_1fb26c_idx `
- Rows scanned: 10,479
- Output groups: 1,114
- Heap fetches: 160
- Buffers: 4,086 hits, 0 reads
- Execution time: 12.5 ms

@scttcper scttcper marked this pull request as ready for review April 29, 2026 17:25
@scttcper scttcper requested a review from a team as a code owner April 29, 2026 17:25
@wedamija
Copy link
Copy Markdown
Member

checking another one via EXPLAIN in redash

Old query shape:
- Plan: Index Scan on `sentry_moni_monitor_idx`
- Rows scanned: 10,479
- Output groups: 1,114
- Buffers: 292 hits, 10,292 reads, 6,521 writes
- Execution time: 5,487 ms

New query shape:
- Plan: Index Only Scan on `sentry_moni_monitor_idx`
- Rows scanned: 10,479
- Output groups: 1,114
- Heap fetches: 160
- Buffers: 4,086 hits, 0 reads
- Execution time: 12.5 ms

Did you run these for the same organization? If you ran the new query after the old, postgres might have cached it. To be sure, it'd be helpful to find another candidate and run new and then old to see if it still improves

@wedamija
Copy link
Copy Markdown
Member

sentry_moni_monitor_idx

Is this the name of the actual index in your explains? I don't see it in the production table https://redash.getsentry.net/queries/11098/source. It'd be helpful to know if the actual index usage changed here

@scttcper
Copy link
Copy Markdown
Member Author

scttcper commented Apr 29, 2026

@wedamija i tried running the new one first and old one first on some different queries. i've updated the comment with the actual index used sentry_moni_monitor_1fb26c_idx

Copy link
Copy Markdown
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like it should help based on the queries we tested on slack. I'd say try it out, and revert if you don't see the slow queries improve

@scttcper scttcper merged commit f914faf into master Apr 29, 2026
63 checks passed
@scttcper scttcper deleted the scttcper/monitor-stats-aggregate branch April 29, 2026 19:17
cleptric pushed a commit that referenced this pull request May 5, 2026
Monitor stats has two query shapes. This optimizes the
environment-backed shape, for example requests that include
`?environment=...` or otherwise resolve to `MonitorEnvironment` rows
before querying check-ins.

In that shape, the endpoint already has a mapping from
`monitor_environment_id` to `monitor_id`. This leaves `monitor_id` out
of the aggregate so Postgres can answer from the existing
`(monitor_environment_id, date_added, status)` index, then recovers
`monitor_id` from the `MonitorEnvironment` rows it already loaded.

The no-monitor-environment fallback keeps the old aggregate shape
because it has no environment-to-monitor mapping.

The query plan change from an environment-backed stats request:

| Shape | Plan | Runtime |
|---|---|---|
| aggregate included `monitor_id` | index scan on the same composite
index, with heap reads for `monitor_id` | about 5.5s |
| aggregate uses `bucket`, `monitor_environment_id`, `status` |
index-only scan on the same composite index, with a small number of heap
fetches | about 12.5ms |

attempt to fix SENTRY-3VHC

---------

Co-authored-by: OpenAI Codex <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants