Revert "fix(deletions) Remove events when groups/projects are deleted (#41937)"

This reverts commit 9618872.

Co-authored-by: asottile-sentry <[email protected]>

Author: getsentry-bot

src/sentry/deletions/defaults/group.py

@@ -1,9 +1,6 @@
 import os
-from collections import defaultdict
 
-from more_itertools import flatten
-
-from sentry import eventstore, eventstream, models, nodestore
+from sentry import eventstore, models, nodestore
 from sentry.eventstore.models import Event
 
 from ..base import BaseDeletionTask, BaseRelation, ModelDeletionTask, ModelRelation
@@ -47,11 +44,11 @@ class EventDataDeletionTask(BaseDeletionTask):
     Deletes nodestore data, EventAttachment and UserReports for group
     """
 
-    # Number of events fetched from eventstore per chunk() call.
     DEFAULT_CHUNK_SIZE = 10000
 
-    def __init__(self, manager, groups, **kwargs):
-        self.groups = groups
+    def __init__(self, manager, group_id, project_id, **kwargs):
+        self.group_id = group_id
+        self.project_id = project_id
         self.last_event = None
         super().__init__(manager, **kwargs)
 
@@ -68,77 +65,58 @@ def chunk(self):
                 ]
             )
 
-        project_groups = defaultdict(list)
-        for group in self.groups:
-            project_groups[group.project_id].append(group.id)
-
-        project_ids = list(project_groups.keys())
-        group_ids = list(flatten(project_groups.values()))
-
         events = eventstore.get_unfetched_events(
             filter=eventstore.Filter(
-                conditions=conditions, project_ids=project_ids, group_ids=group_ids
+                conditions=conditions, project_ids=[self.project_id], group_ids=[self.group_id]
             ),
             limit=self.DEFAULT_CHUNK_SIZE,
             referrer="deletions.group",
             orderby=["-timestamp", "-event_id"],
         )
+
         if not events:
-            # Remove all group events now that their node data has been removed.
-            for project_id, group_ids in project_groups.items():
-                eventstream_state = eventstream.start_delete_groups(project_id, group_ids)
-                eventstream.end_delete_groups(eventstream_state)
             return False
 
         self.last_event = events[-1]
 
         # Remove from nodestore
-        node_ids = [Event.generate_node_id(event.project_id, event.event_id) for event in events]
+        node_ids = [Event.generate_node_id(self.project_id, event.event_id) for event in events]
         nodestore.delete_multi(node_ids)
 
         # Remove EventAttachment and UserReport *again* as those may not have a
         # group ID, therefore there may be dangling ones after "regular" model
         # deletion.
         event_ids = [event.event_id for event in events]
         models.EventAttachment.objects.filter(
-            event_id__in=event_ids, project_id__in=project_ids
+            event_id__in=event_ids, project_id=self.project_id
         ).delete()
         models.UserReport.objects.filter(
-            event_id__in=event_ids, project_id__in=project_ids
+            event_id__in=event_ids, project_id=self.project_id
         ).delete()
 
         return True
 
 
 class GroupDeletionTask(ModelDeletionTask):
-    # Delete groups in blocks of 1000. Using 1000 aims to
-    # balance the number of snuba replacements with memory limits.
-    DEFAULT_CHUNK_SIZE = 1000
-
-    def delete_bulk(self, instance_list):
-        """
-        Group deletion operates as a quasi-bulk operation so that we don't flood
-        snuba replacements with deletions per group.
-        """
-        self.mark_deletion_in_progress(instance_list)
+    def get_child_relations(self, instance):
+        relations = []
 
-        group_ids = [group.id for group in instance_list]
-
-        # Remove child relations for all groups first.
-        child_relations = []
-        for model in _GROUP_RELATED_MODELS:
-            child_relations.append(ModelRelation(model, {"group_id__in": group_ids}))
+        relations.extend(
+            [ModelRelation(m, {"group_id": instance.id}) for m in _GROUP_RELATED_MODELS]
+        )
 
-        # If this isn't a retention cleanup also remove event data.
+        # Skip EventDataDeletionTask if this is being called from cleanup.py
         if not os.environ.get("_SENTRY_CLEANUP"):
-            child_relations.append(
-                BaseRelation(params={"groups": instance_list}, task=EventDataDeletionTask)
+            relations.extend(
+                [
+                    BaseRelation(
+                        {"group_id": instance.id, "project_id": instance.project_id},
+                        EventDataDeletionTask,
+                    )
+                ]
             )
 
-        self.delete_children(child_relations)
-
-        # Remove group objects with children removed.
-        return self.delete_instance_bulk(instance_list)
+        return relations
 
     def delete_instance(self, instance):
         from sentry import similarity

src/sentry/deletions/defaults/project.py

@@ -54,11 +54,19 @@ def get_child_relations(self, instance):
                 for m in model_list
             ]
         )
-        relations.append(ModelRelation(models.Group, {"project_id": instance.id}))
+
+        model_list = (models.GroupMeta, models.GroupResolution, models.GroupSnooze)
+        relations.extend(
+            [
+                ModelRelation(m, {"group__project": instance.id}, ModelDeletionTask)
+                for m in model_list
+            ]
+        )
 
         # Release needs to handle deletes after Group is cleaned up as the foreign
         # key is protected
         model_list = (
+            models.Group,
             models.ReleaseProject,
             models.ReleaseProjectEnvironment,
             models.ProjectDebugFile,

src/sentry/utils/query.py

@@ -222,13 +222,8 @@ def bulk_delete_objects(
             params.append(value)
 
     for column, value in filters.items():
-        if column.endswith("__in"):
-            column, _ = column.split("__")
-            query.append(f"{quote_name(column)} = ANY(%s)")
-            params.append(value)
-        else:
-            query.append(f"{quote_name(column)} = %s")
-            params.append(value)
+        query.append(f"{quote_name(column)} = %s")
+        params.append(value)
 
     query = """
         delete from %(table)s

tests/sentry/deletions/test_group.py

@@ -49,7 +49,7 @@ def setUp(self):
             project_id=self.project.id,
         )
 
-        self.keep_event = self.store_event(
+        self.store_event(
             data={
                 "event_id": self.event_id3,
                 "timestamp": iso_format(before_now(minutes=1)),
@@ -102,31 +102,6 @@ def test_simple(self):
         assert not nodestore.get(self.node_id)
         assert not nodestore.get(self.node_id2)
         assert nodestore.get(self.node_id3), "Does not remove from second group"
-        assert Group.objects.filter(id=self.keep_event.group_id).exists()
-
-    def test_simple_multiple_groups(self):
-        other_event = self.store_event(
-            data={
-                "event_id": "d" * 32,
-                "timestamp": iso_format(before_now(minutes=1)),
-                "fingerprint": ["group3"],
-            },
-            project_id=self.project.id,
-        )
-        other_node_id = Event.generate_node_id(self.project.id, other_event.event_id)
-        keep_node_id = Event.generate_node_id(self.project.id, self.keep_event.event_id)
-
-        group = self.event.group
-        with self.tasks():
-            delete_groups(object_ids=[group.id, other_event.group_id])
-
-        assert not Group.objects.filter(id=group.id).exists()
-        assert not Group.objects.filter(id=other_event.group_id).exists()
-        assert not nodestore.get(self.node_id)
-        assert not nodestore.get(other_node_id)
-
-        assert Group.objects.filter(id=self.keep_event.group_id).exists()
-        assert nodestore.get(keep_node_id)
 
     @mock.patch("os.environ.get")
     @mock.patch("sentry.nodestore.delete_multi")

tests/sentry/deletions/test_project.py

@@ -1,4 +1,3 @@
-from sentry import eventstore
 from sentry.models import (
     Commit,
     CommitAuthor,
@@ -10,7 +9,6 @@
     GroupAssignee,
     GroupMeta,
     GroupResolution,
-    GroupSeen,
     Project,
     ProjectDebugFile,
     Release,
@@ -21,7 +19,6 @@
 )
 from sentry.tasks.deletion import run_deletion
 from sentry.testutils import TransactionTestCase
-from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.silo import region_silo_test
 
 
@@ -100,30 +97,3 @@ def test_simple(self):
         assert not ProjectDebugFile.objects.filter(id=dif.id).exists()
         assert not File.objects.filter(id=file.id).exists()
         assert not ServiceHook.objects.filter(id=hook.id).exists()
-
-    def test_delete_error_events(self):
-        keeper = self.create_project(name="keeper")
-        project = self.create_project(name="test")
-        event = self.store_event(
-            data={
-                "timestamp": iso_format(before_now(minutes=1)),
-                "message": "oh no",
-            },
-            project_id=project.id,
-        )
-        group = event.group
-        group_seen = GroupSeen.objects.create(group=group, project=project, user=self.user)
-
-        deletion = ScheduledDeletion.schedule(project, days=0)
-        deletion.update(in_progress=True)
-
-        with self.tasks():
-            run_deletion(deletion.id)
-
-        assert not Project.objects.filter(id=project.id).exists()
-        assert not GroupSeen.objects.filter(id=group_seen.id).exists()
-        assert not Group.objects.filter(id=group.id).exists()
-
-        conditions = eventstore.Filter(project_ids=[project.id, keeper.id], group_ids=[group.id])
-        events = eventstore.get_events(conditions)
-        assert len(events) == 0

tests/sentry/utils/test_query.py

@@ -1,6 +1,6 @@
-from sentry.models import Team, User
+from sentry.models import User
 from sentry.testutils import TestCase
-from sentry.utils.query import RangeQuerySetWrapper, bulk_delete_objects
+from sentry.utils.query import RangeQuerySetWrapper
 
 
 class RangeQuerySetWrapperTest(TestCase):
@@ -26,29 +26,3 @@ def test_loop_and_delete(self):
             user.delete()
 
         assert User.objects.all().count() == 0
-
-
-class BulkDeleteObjectsTest(TestCase):
-    def setUp(self):
-        super().setUp()
-        Team.objects.all().delete()
-
-    def test_basic(self):
-        total = 10
-        records = []
-        for _ in range(total):
-            records.append(self.create_team())
-
-        result = bulk_delete_objects(Team, id__in=[r.id for r in records])
-        assert result, "Could be more work to do"
-        assert len(Team.objects.all()) == 0
-
-    def test_limiting(self):
-        total = 10
-        records = []
-        for _ in range(total):
-            records.append(self.create_team())
-
-        result = bulk_delete_objects(Team, id__in=[r.id for r in records], limit=5)
-        assert result, "Still more work to do"
-        assert len(Team.objects.all()) == 5