-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(deletions): Break circular reference in GroupHistory #102470
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.
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
Conversation
When deleting a GroupHistory record, Django would CASCADE to all records that reference it via `prev_history`, which can create circular dependency loops. After this change, Django will just set prev_history=NULL on any records that reference it, breaking the circular chain. Fixes [SENTRY-3WN8](https://sentry.sentry.io/issues/6619377437/) and [SENTRY-3WKX](https://sentry.sentry.io/issues/6618890707/).
|
This PR has a migration; here is the generated SQL for for --
-- Alter field prev_history on grouphistory
--
-- (no-op) |
|
I prefer removing the field completely: #102473 |
|
That other PR will take multiple steps. Let's start with getting this one out the door. |
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making any reference to it ([docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns).
markstory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me. Instead of recursing deletes, django will only have to modify the one adjacent record.
When deleting a GroupHistory record, Django would CASCADE to all records that reference it via `prev_history`, which can create circular dependency loops. After this change, Django will just set prev_history=NULL on any records that reference it, breaking the circular chain. Fixes [SENTRY-3WN8](https://sentry.sentry.io/issues/6619377437/) and [SENTRY-3WKX](https://sentry.sentry.io/issues/6618890707/).
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making use of it (see [docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns) for instructions).
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making use of it (see [docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns) for instructions).
This is how it used to be before #76608. Now that we have fixed the circular relationship (#102470) I believe we should give this approach another attempt. Fixes [SENTRY-5BX2](https://sentry.sentry.io/issues/6993998110/): For posterity: ``` OperationalError canceling statement due to user request SQL: SELECT "sentry_grouphistory"."id" AS "id" FROM "sentry_grouphistory" WHERE "sentry_grouphistory"."group_id" = %s ORDER BY 1 ASC LIMIT 100 ``` The group in question has only been seen 4 times yet it has 905,729 rows of group history.
This is how it used to be before #76608. Now that we have fixed the circular relationship (#102470) I believe we should give this approach another attempt. Fixes [SENTRY-5BX2](https://sentry.sentry.io/issues/6993998110/): For posterity: ``` OperationalError canceling statement due to user request SQL: SELECT "sentry_grouphistory"."id" AS "id" FROM "sentry_grouphistory" WHERE "sentry_grouphistory"."group_id" = %s ORDER BY 1 ASC LIMIT 100 ``` The group in question has only been seen 4 times yet it has 905,729 rows of group history. This is caused by the priority of the group changing back and forth between high and medium priority. We're investigating the root cause of it.
When deleting a GroupHistory record, Django would CASCADE to all records that reference it via `prev_history`, which can create circular dependency loops. After this change, Django will just set prev_history=NULL on any records that reference it, breaking the circular chain. Fixes [SENTRY-3WN8](https://sentry.sentry.io/issues/6619377437/) and [SENTRY-3WKX](https://sentry.sentry.io/issues/6618890707/).
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making use of it (see [docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns) for instructions).
This is how it used to be before #76608. Now that we have fixed the circular relationship (#102470) I believe we should give this approach another attempt. Fixes [SENTRY-5BX2](https://sentry.sentry.io/issues/6993998110/): For posterity: ``` OperationalError canceling statement due to user request SQL: SELECT "sentry_grouphistory"."id" AS "id" FROM "sentry_grouphistory" WHERE "sentry_grouphistory"."group_id" = %s ORDER BY 1 ASC LIMIT 100 ``` The group in question has only been seen 4 times yet it has 905,729 rows of group history. This is caused by the priority of the group changing back and forth between high and medium priority. We're investigating the root cause of it.
This is how it used to be before #76608. Now that we have fixed the circular relationship (#102470) I believe we should give this approach another attempt. Fixes [SENTRY-5BX2](https://sentry.sentry.io/issues/6993998110/): For posterity: ``` OperationalError canceling statement due to user request SQL: SELECT "sentry_grouphistory"."id" AS "id" FROM "sentry_grouphistory" WHERE "sentry_grouphistory"."group_id" = %s ORDER BY 1 ASC LIMIT 100 ``` The group in question has only been seen 4 times yet it has 905,729 rows of group history. This is caused by the priority of the group changing back and forth between high and medium priority. We're investigating the root cause of it.
When deleting a GroupHistory record, Django would CASCADE to all records that reference it via
prev_history, which can create circular dependency loops.After this change, Django will just set prev_history=NULL on any records that reference it, breaking the circular chain.
Fixes SENTRY-3WN8 and SENTRY-3WKX.