Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(escalating-issues): Create new index on Group for more efficient query #56180

Merged
merged 2 commits into from
Sep 15, 2023
Merged

feat(escalating-issues): Create new index on Group for more efficient query #56180

merged 2 commits into from
Sep 15, 2023

Conversation

NisanthanNanthakumar
Copy link
Contributor

Objective:

Required for #56168

@NisanthanNanthakumar NisanthanNanthakumar requested a review from a team September 13, 2023 17:44
@NisanthanNanthakumar NisanthanNanthakumar marked this pull request as ready for review September 13, 2023 17:44
@NisanthanNanthakumar NisanthanNanthakumar requested a review from a team as a code owner September 13, 2023 17:44
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 13, 2023
@NisanthanNanthakumar NisanthanNanthakumar mentioned this pull request Sep 13, 2023
Merged
@codecov
Copy link

codecov bot commented Sep 13, 2023 •
edited
Loading

Codecov Report

Merging #56180 (0834595) into master (9ecf338) will not change coverage.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #56180   +/-   ##
=======================================
  Coverage   78.67%   78.67%           
=======================================
  Files        5070     5070           
  Lines      217986   217986           
  Branches    36897    36897           
=======================================
  Hits       171502   171502           
- Misses      40946    40948    +2     
+ Partials     5538     5536    -2
Files Changed Coverage
src/sentry/models/group.py ø

armenzg
armenzg approved these changes Sep 14, 2023
View reviewed changes
Copy link
Member

@armenzg armenzg left a comment

Choose a reason for hiding this comment

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

Do we need to remove some of the older indexes? I would ask Hubert. I don't know how much memory or cost these have.

@NisanthanNanthakumar
Copy link
Contributor Author

@armenzg I'll follow up with Hubert on that!

@NisanthanNanthakumar NisanthanNanthakumar force-pushed the create-new-index-for-groupedmessage-table branch from cb4338e to abd5570 Compare September 14, 2023 18:03
@NisanthanNanthakumar NisanthanNanthakumar requested a review from a team as a code owner September 14, 2023 18:03
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0552_add_new_index_to_groupedmessage_table.py ()

--
-- Alter index_together for group (8 constraint(s))
--
CREATE INDEX CONCURRENTLY "sentry_groupedmessage_status_substatus_id_e00e92ae_idx" ON "sentry_groupedmessage" ("status", "substatus", "id");

@NisanthanNanthakumar NisanthanNanthakumar force-pushed the create-new-index-for-groupedmessage-table branch from abd5570 to 0834595 Compare September 15, 2023 17:04
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0553_add_new_index_to_groupedmessage_table.py ()

--
-- Alter index_together for group (8 constraint(s))
--
CREATE INDEX CONCURRENTLY "sentry_groupedmessage_status_substatus_id_e00e92ae_idx" ON "sentry_groupedmessage" ("status", "substatus", "id");

@NisanthanNanthakumar NisanthanNanthakumar merged commit 381818a into master Sep 15, 2023
51 checks passed
@NisanthanNanthakumar NisanthanNanthakumar deleted the create-new-index-for-groupedmessage-table branch September 15, 2023 17:43
@NisanthanNanthakumar NisanthanNanthakumar added the Trigger: Revert add to a merged PR to revert it (skips CI) label Sep 15, 2023
@getsentry-bot
Copy link
Contributor

PR reverted: b8241ac

getsentry-bot added a commit that referenced this pull request Sep 15, 2023
@getsentry-bot @NisanthanNanthakumar
Revert "feat(escalating-issues): Create new index on Group for more e…
b8241ac 
…fficient query (#56180)"

This reverts commit 381818a.

Co-authored-by: NisanthanNanthakumar <10491193+NisanthanNanthakumar@users.noreply.github.com>
@NisanthanNanthakumar NisanthanNanthakumar mentioned this pull request Sep 15, 2023
Merged
NisanthanNanthakumar pushed a commit that referenced this pull request Sep 15, 2023
feat(escalating-issues): Create new index on Group for more efficient…
d6d67fe 
… query (#56345)

## Objective:

Re-revert of #56180

This time with `is_dangerous=True`
@github-actions github-actions bot locked and limited conversation to collaborators Oct 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@armenzg armenzg armenzg approved these changes

@snigdhas snigdhas snigdhas approved these changes

Assignees
No one assigned
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NisanthanNanthakumar @getsentry-bot @armenzg @snigdhas