Skip to content

Conversation

ceorourke
Copy link
Member

#96221 had to be reverted due to https://sentry.sentry.io/issues/6799376702 so we thought adding an index on GroupOpenPeriod's data jsonfield to the frequently looked up pending_incident_detector_id key would help speed things up.

@ceorourke ceorourke requested a review from a team as a code owner August 8, 2025 22:46
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 8, 2025
@@ -73,6 +73,10 @@ class Meta:
indexes = (
# get all open periods since a certain date
models.Index(fields=("group", "date_started")),
models.Index(
models.F("data__pending_incident_detector_id"),
name="data__pend_inc_detector_id_idx",
Copy link
Member Author

Choose a reason for hiding this comment

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

name is required and it's limited to 30 chars so I did the best I could here

Copy link
Contributor

github-actions bot commented Aug 8, 2025

This PR has a migration; here is the generated SQL for src/sentry/migrations/0966_groupopenperiod_data_pending_inc_detector_id_index.py

for 0966_groupopenperiod_data_pending_inc_detector_id_index in sentry

--
-- Create index data__pend_inc_detector_id_idx on F(data__pending_incident_detector_id) on model groupopenperiod
--
CREATE INDEX CONCURRENTLY "data__pend_inc_detector_id_idx" ON "sentry_groupopenperiod" ((("data" -> 'pending_incident_detector_id')));

# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False
Copy link
Member

Choose a reason for hiding this comment

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

This should be post-deployment migration, as this table has >260M rows, and adding index will probably time out if we do it as in-deploy migration.

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #97534    +/-   ##
========================================
  Coverage   80.62%   80.62%            
========================================
  Files        8560     8560            
  Lines      376889   377106   +217     
  Branches    24538    24538            
========================================
+ Hits       303870   304048   +178     
- Misses      72649    72688    +39     
  Partials      370      370            

Copy link
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.

lgtm, hopefully this isn't too hard to create

@ceorourke ceorourke merged commit f0ef2cd into master Aug 11, 2025
65 checks passed
@ceorourke ceorourke deleted the ceorourke/groupopenperiod-index-jsonfield branch August 11, 2025 17:09
ceorourke added a commit that referenced this pull request Aug 12, 2025
Redo of #96221 after [adding an
index](#97534) to the
`GroupOpenPeriod`'s `data` column on `pending_incident_detector_id`
which should hopefully not result in [the
error](https://sentry.sentry.io/issues/6799376702/) anymore. There are
no changes to this PR from the original one.

---------

Co-authored-by: Snigdha Sharma <[email protected]>
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
#96221 had to be reverted due to
https://sentry.sentry.io/issues/6799376702 so we thought adding an index
on `GroupOpenPeriod`'s `data` jsonfield to the frequently looked up
`pending_incident_detector_id` key would help speed things up.
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
Redo of #96221 after [adding an
index](#97534) to the
`GroupOpenPeriod`'s `data` column on `pending_incident_detector_id`
which should hopefully not result in [the
error](https://sentry.sentry.io/issues/6799376702/) anymore. There are
no changes to this PR from the original one.

---------

Co-authored-by: Snigdha Sharma <[email protected]>
priscilawebdev pushed a commit that referenced this pull request Aug 25, 2025
#96221 had to be reverted due to
https://sentry.sentry.io/issues/6799376702 so we thought adding an index
on `GroupOpenPeriod`'s `data` jsonfield to the frequently looked up
`pending_incident_detector_id` key would help speed things up.
priscilawebdev pushed a commit that referenced this pull request Aug 25, 2025
Redo of #96221 after [adding an
index](#97534) to the
`GroupOpenPeriod`'s `data` column on `pending_incident_detector_id`
which should hopefully not result in [the
error](https://sentry.sentry.io/issues/6799376702/) anymore. There are
no changes to this PR from the original one.

---------

Co-authored-by: Snigdha Sharma <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Aug 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants