From 5636892c9a0e1ef4662cc5a1985993a5a29e4dc8 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 20 Oct 2021 18:06:52 -0700 Subject: [PATCH] feat(insights): Update `GroupHistory.actor` to set null on delete We don't want to delete these rows if the actor that made the changes is deleted, just `SET_NULL` instead. Also corrected some comments --- migrations_lockfile.txt | 2 +- .../api/endpoints/team_time_to_resolution.py | 4 +- .../0241_grouphistory_null_actor.py | 42 +++++++++++++++++++ src/sentry/models/grouphistory.py | 5 ++- 4 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 src/sentry/migrations/0241_grouphistory_null_actor.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 6cbf15a7004a35..86fc15b2c5e29a 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi will then be regenerated, and you should be able to merge without conflicts. nodestore: 0002_nodestore_no_dictfield -sentry: 0240_grouphistory_index +sentry: 0241_grouphistory_null_actor social_auth: 0001_initial diff --git a/src/sentry/api/endpoints/team_time_to_resolution.py b/src/sentry/api/endpoints/team_time_to_resolution.py index 92c17497f17274..d5807f3260d95e 100644 --- a/src/sentry/api/endpoints/team_time_to_resolution.py +++ b/src/sentry/api/endpoints/team_time_to_resolution.py @@ -29,8 +29,8 @@ def get(self, request, team): ) .annotate(bucket=TruncDay("date_added")) .values("bucket", "prev_history_date") - # We do the coalesce here to handle historical data. At some point every `RESOLVED` row - # will have a non-null `prev_history_date`, and at that point we could remove this. + # We need to coalesce here since we won't store the initial `UNRESOLVED` row for every + # group, since it's unnecessary and just takes extra storage. .annotate( ttr=F("date_added") - Coalesce(F("prev_history_date"), F("group__first_seen")) ) diff --git a/src/sentry/migrations/0241_grouphistory_null_actor.py b/src/sentry/migrations/0241_grouphistory_null_actor.py new file mode 100644 index 00000000000000..e14efb2ea3ad29 --- /dev/null +++ b/src/sentry/migrations/0241_grouphistory_null_actor.py @@ -0,0 +1,42 @@ +# Generated by Django 2.2.24 on 2021-10-21 01:06 + +import django.db.models.deletion +from django.db import migrations + +import sentry.db.models.fields.foreignkey + + +class Migration(migrations.Migration): + # This flag is used to mark that a migration shouldn't be automatically run in + # production. We set this to True for operations that we think are risky and want + # someone from ops to run manually and monitor. + # General advice is that if in doubt, mark your migration as `is_dangerous`. + # Some things you should always mark as dangerous: + # - Large data migrations. Typically we want these to be run manually by ops so that + # they can be monitored. Since data migrations will now hold a transaction open + # this is even more important. + # - Adding columns to highly active tables, even ones that are NULL. + is_dangerous = False + + # This flag is used to decide whether to run this migration in a transaction or not. + # By default we prefer to run in a transaction, but for migrations where you want + # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll + # want to create an index concurrently when adding one to an existing table. + # You'll also usually want to set this to `False` if you're writing a data + # migration, since we don't want the entire migration to run in one long-running + # transaction. + atomic = True + + dependencies = [ + ("sentry", "0240_grouphistory_index"), + ] + + operations = [ + migrations.AlterField( + model_name="grouphistory", + name="actor", + field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( + null=True, on_delete=django.db.models.deletion.SET_NULL, to="sentry.Actor" + ), + ), + ] diff --git a/src/sentry/models/grouphistory.py b/src/sentry/models/grouphistory.py index d44aa8c0f030ac..445f08b6a58c00 100644 --- a/src/sentry/models/grouphistory.py +++ b/src/sentry/models/grouphistory.py @@ -1,6 +1,7 @@ from typing import TYPE_CHECKING, Optional, Union from django.db import models +from django.db.models import SET_NULL from django.utils import timezone from django.utils.translation import ugettext_lazy as _ @@ -12,6 +13,8 @@ class GroupHistoryStatus: + # Note that we don't record the initial group creation unresolved here to save on creating a row + # for every group. UNRESOLVED = 0 RESOLVED = 1 SET_RESOLVED_IN_RELEASE = 11 @@ -78,7 +81,7 @@ class GroupHistory(Model): group = FlexibleForeignKey("sentry.Group", db_constraint=False) project = FlexibleForeignKey("sentry.Project", db_constraint=False) release = FlexibleForeignKey("sentry.Release", null=True, db_constraint=False) - actor = FlexibleForeignKey("sentry.Actor", null=True) + actor = FlexibleForeignKey("sentry.Actor", null=True, on_delete=SET_NULL) status = BoundedPositiveIntegerField( default=0,