ref(deletions): Remove unnecessary Seer calls in endpoint #93541

armenzg · 2025-06-13T18:45:37Z

During the deletion API call we were calling seer & a couple more changes which the Group deletion model already does, thus, we don't need to do it during the API call.

Prior to this change, if the Seer call failed, the deletion task would not get scheduled, thus, many groups stay in a pending deletion state and never deleted.

Fixes #93509
Fixes ID-716

armenzg · 2025-06-13T18:46:34Z

src/sentry/api/helpers/group_index/delete.py

    create_audit_entries(request, project, group_list, delete_type, transaction_id)

-    # Tell seer to delete grouping records for these groups
-    call_delete_seer_grouping_records_by_hash(error_ids)


Already part of delete_bulk:

sentry/src/sentry/deletions/defaults/group.py

Lines 275 to 276 in 29dc6ac

# Tell seer to delete grouping records with these group hashes

call_delete_seer_grouping_records_by_hash(error_group_ids)

If this call to seer fails then the task is not get scheduled.

This is one of the main reasons we have so many groups in a pending deletion state.

Redash query: https://redash.getsentry.net/queries/9170/source

armenzg · 2025-06-13T18:47:20Z

src/sentry/api/helpers/group_index/delete.py

-
-    # Removing GroupHash rows prevents new events from associating to the groups
-    # we just deleted.
-    GroupHash.objects.filter(project_id=project.id, group__id__in=group_ids).delete()


This is one of the models that are deleted.

sentry/src/sentry/deletions/defaults/group.py

Line 29 in 29dc6ac

models.GroupHash,

Wouldn't this allow new events to be added to the existing issues until the delete is processed? Is that going to do what customers want still?

@markstory The group is already marked as pending deletion, thus, the customer can't even see the group.

See lines 67-69:

We also need to keep the group hash until the Seer task gets scheduled, thus, even the more reason to do it during the Group deletion task.

src/sentry/api/helpers/group_index/delete.py

armenzg · 2025-06-13T18:48:00Z

src/sentry/deletions/defaults/group.py

-            except InvalidGroupTypeError:
-                pass
-        # Tell seer to delete grouping records with these group hashes
-        call_delete_seer_grouping_records_by_hash(error_group_ids)


Moving the call under _delete_children.

armenzg · 2025-06-13T18:54:51Z

src/sentry/deletions/defaults/group.py

+        try:
+            call_delete_seer_grouping_records_by_hash([group.id for group in error_groups])
+        except Exception:
+            logger.exception("Error calling seer delete grouping records by hash")


I have seen the call fail as part of the API deletion calls, thus, I want to put this guard against it.

codecov · 2025-06-13T19:13:18Z

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   #93541      +/-   ##
==========================================
- Coverage   87.83%   87.81%   -0.02%     
==========================================
  Files       10478    10492      +14     
  Lines      605960   606303     +343     
  Branches    23674    23674              
==========================================
+ Hits       532216   532403     +187     
- Misses      73380    73536     +156     
  Partials      364      364

armenzg · 2025-07-09T14:59:02Z

src/sentry/api/helpers/group_index/delete.py

    # delete the "smaller" groups first
    group_list.sort(key=lambda g: (g.times_seen, g.id))
-    group_ids = []
-    error_ids = []


We don't need this variable anymore, thus, removing the whole block.

armenzg · 2025-07-09T15:00:47Z

src/sentry/api/helpers/group_index/delete.py

-
-    # Removing GroupHash rows prevents new events from associating to the groups
-    # we just deleted.
-    GroupHash.objects.filter(project_id=project.id, group__id__in=group_ids).delete()


@markstory The group is already marked as pending deletion, thus, the customer can't even see the group.

See lines 67-69:

armenzg · 2025-07-09T15:10:10Z

src/sentry/api/helpers/group_index/delete.py

    create_audit_entries(request, project, group_list, delete_type, transaction_id)

-    # Tell seer to delete grouping records for these groups
-    call_delete_seer_grouping_records_by_hash(error_ids)


If this call to seer fails then the task is not get scheduled.

This is one of the main reasons we have so many groups in a pending deletion state.

Redash query: https://redash.getsentry.net/queries/9170/source

During the API call we were doing work that the Group model already does.

armenzg · 2025-07-11T13:43:43Z

tests/sentry/api/helpers/test_group_index.py

            add_group_to_inbox(group, GroupInboxReason.NEW)

-        delete_groups(request, [self.project], self.organization.id)
+        with self.tasks():


If the task does not execute the hashes will still exist.

armenzg · 2025-07-11T13:44:10Z

tests/snuba/api/endpoints/test_project_group_index.py

-
-    @patch("sentry.api.helpers.group_index.delete.may_schedule_task_to_delete_hashes_from_seer")
-    @patch("sentry.utils.audit.log_service.record_audit_log")
-    def test_audit_log_even_if_exception_raised(


This test does not make sense after this change.

cursor

Bug: Group Deletion Logic Inconsistency

The GroupHash deletion was removed from the delete_group_list API endpoint, despite PR discussion acknowledging the need to restore it. This allows new events to associate with groups marked as PENDING_DELETION during the window before the asynchronous deletion task completes. This contradicts the original intent to prevent such associations and is inconsistent with the retained GroupInbox deletion logic.

src/sentry/api/helpers/group_index/delete.py#L78-L84

sentry/src/sentry/api/helpers/group_index/delete.py

Lines 78 to 84 in c49bea9

    
           # We remove `GroupInbox` rows here so that they don't end up influencing queries for 
        
           # `Group` instances that are pending deletion 
        
           GroupInbox.objects.filter(project_id=project.id, group__id__in=group_ids).delete() 
        
           delete_groups_task.apply_async( 
        
               kwargs={

Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

armenzg · 2025-07-14T19:47:50Z

Bug: Group Deletion Logic Inconsistency

The GroupHash deletion was removed from the delete_group_list API endpoint, despite PR discussion acknowledging the need to restore it. This allows new events to associate with groups marked as PENDING_DELETION during the window before the asynchronous deletion task completes. This contradicts the original intent to prevent such associations and is inconsistent with the retained GroupInbox deletion logic.

src/sentry/api/helpers/group_index/delete.py#L78-L84
Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

We don't want to delete GroupHash since it will be needed during the task execution to call Seer.

lobsterkatie

LGTM!

markstory

Makes sense to me. Consolidating the deletion logic is a great idea.

…3541)" This reverts commit e9e09e1. This reverts #93541

…3541) (#95828) This reverts commit e9e09e1. A customer deleted an issue but new events are being associated with the old group rather than creating a new one. I will add a test to document the expected behaviour to prevent future regressions.

…3541)" This reverts commit e9e09e1. This reverts #93541

When deleting an issue, a failure to query the DB would prevent scheduling the group deletion task. From the customer's perspective, it would look that the group has been deleted, however, all relationships would still be left behind. The biggest problem would be that the group hashes are left behind, thus, any new events matching those group hashes would not create a new issue. This is a better fix than the one proposed in #93541. Fixes #93509. Fixes [ID-716](https://linear.app/getsentry/issue/ID-716/)

This is the test missing from #93541 that would have prevented the regression.

When deleting an issue, a failure to query the DB would prevent scheduling the group deletion task. From the customer's perspective, it would look that the group has been deleted, however, all relationships would still be left behind. The biggest problem would be that the group hashes are left behind, thus, any new events matching those group hashes would not create a new issue. This is a better fix than the one proposed in #93541. Fixes #93509. Fixes [ID-716](https://linear.app/getsentry/issue/ID-716/)

During the deletion API call we were calling seer & a couple more changes which the Group deletion model already does, thus, we don't need to do it during the API call. Prior to this change, if the Seer call failed, the deletion task would not get scheduled, thus, many groups stay in a pending deletion state and never deleted. Fixes #93509 Fixes [ID-716](https://linear.app/getsentry/issue/ID-716/move-endpoint-code-to-the-deletion-task)

…3541) (#95828) This reverts commit e9e09e1. A customer deleted an issue but new events are being associated with the old group rather than creating a new one. I will add a test to document the expected behaviour to prevent future regressions.

This is the test missing from #93541 that would have prevented the regression.

When deleting an issue, a failure to query the DB would prevent scheduling the group deletion task. From the customer's perspective, it would look that the group has been deleted, however, all relationships would still be left behind. The biggest problem would be that the group hashes are left behind, thus, any new events matching those group hashes would not create a new issue. This is a better fix than the one proposed in #93541. Fixes #93509. Fixes [ID-716](https://linear.app/getsentry/issue/ID-716/)

armenzg self-assigned this Jun 13, 2025

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 13, 2025

vercel bot deployed to Preview June 13, 2025 18:47 View deployment

armenzg commented Jun 13, 2025

View reviewed changes

armenzg requested a review from markstory June 13, 2025 18:55

armenzg marked this pull request as ready for review June 13, 2025 18:55

armenzg requested review from a team as code owners June 13, 2025 18:55

armenzg requested a review from a team as a code owner June 13, 2025 18:56

vercel bot deployed to Preview June 13, 2025 18:57 View deployment

armenzg requested a review from a team as a code owner June 16, 2025 16:48

vercel bot deployed to Preview June 16, 2025 16:50 View deployment

armenzg marked this pull request as draft June 16, 2025 17:23

armenzg removed the request for review from markstory July 3, 2025 18:53

armenzg force-pushed the feat/hash/group/deletion/armenzg branch from 2467a33 to 2f607b6 Compare July 9, 2025 14:42

vercel bot deployed to Preview July 9, 2025 14:44 View deployment

armenzg commented Jul 9, 2025

View reviewed changes

armenzg added 7 commits July 11, 2025 08:45

ref(deletions): Remove unnecessary calls in endpoint

095232f

During the API call we were doing work that the Group model already does.

Fix some tests

39c0e6a

Move seer call to the end

841f9df

Minor

e660160

Use with self.task

933f7b9

Reduce diff

d5c2ceb

Remove unnecessary test

449c4dd

armenzg force-pushed the feat/hash/group/deletion/armenzg branch from 2f607b6 to 449c4dd Compare July 11, 2025 12:46

armenzg mentioned this pull request Jul 11, 2025

WIP #95259

Closed

vercel bot deployed to Preview July 11, 2025 12:48 View deployment

Fix this

1d65ea3

vercel bot deployed to Preview July 11, 2025 12:50 View deployment

armenzg commented Jul 11, 2025

View reviewed changes

armenzg changed the title ~~ref(deletions): Remove unnecessary calls in endpoint~~ ref(deletions): Remove unnecessary Seer calls in endpoint Jul 11, 2025

armenzg requested a review from markstory July 11, 2025 13:44

armenzg marked this pull request as ready for review July 11, 2025 13:44

armenzg requested a review from lobsterkatie July 14, 2025 18:48

Add groupinbox back

0a82586

vercel bot deployed to Preview July 14, 2025 19:14 View deployment

Fix typing

c49bea9

vercel bot deployed to Preview July 14, 2025 19:17 View deployment

cursor bot reviewed Jul 14, 2025

View reviewed changes

lobsterkatie approved these changes Jul 14, 2025

View reviewed changes

markstory approved these changes Jul 15, 2025

View reviewed changes

armenzg merged commit e9e09e1 into master Jul 15, 2025
65 checks passed

armenzg deleted the feat/hash/group/deletion/armenzg branch July 15, 2025 16:58

armenzg added a commit that referenced this pull request Jul 17, 2025

Revert "ref(deletions): Remove unnecessary Seer calls in endpoint (#9…

9059537

…3541)" This reverts commit e9e09e1. This reverts #93541

cursor bot pushed a commit that referenced this pull request Jul 18, 2025

Revert "ref(deletions): Remove unnecessary Seer calls in endpoint (#9…

bf0c298

…3541)" This reverts commit e9e09e1. This reverts #93541

armenzg mentioned this pull request Jul 18, 2025

fix(deletions): Prevent DB call from deleting issue #95915

Merged

armenzg added a commit that referenced this pull request Jul 21, 2025

tests(deletion): Events after group deletion create new groups

aaabe91

This is the test missing from #93541 that would have prevented the regression.

armenzg mentioned this pull request Jul 21, 2025

tests(deletion): Events after group deletion create new groups #95976

Merged

armenzg added a commit that referenced this pull request Jul 21, 2025

tests(deletion): Events after group deletion create new groups (#95976)

32de56d

This is the test missing from #93541 that would have prevented the regression.

andrewshie-sentry pushed a commit that referenced this pull request Jul 21, 2025

tests(deletion): Events after group deletion create new groups (#95976)

b3bc6f5

This is the test missing from #93541 that would have prevented the regression.

github-actions bot locked and limited conversation to collaborators Jul 31, 2025

	# Tell seer to delete grouping records with these group hashes
	call_delete_seer_grouping_records_by_hash(error_group_ids)


	# We remove `GroupInbox` rows here so that they don't end up influencing queries for
	# `Group` instances that are pending deletion
	GroupInbox.objects.filter(project_id=project.id, group__id__in=group_ids).delete()

	delete_groups_task.apply_async(
	kwargs={

Uh oh!

ref(deletions): Remove unnecessary Seer calls in endpoint #93541

ref(deletions): Remove unnecessary Seer calls in endpoint #93541

Uh oh!

Conversation

armenzg commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Group Deletion Logic Inconsistency

Uh oh!

armenzg commented Jul 14, 2025

Bug: Group Deletion Logic Inconsistency

Uh oh!

lobsterkatie left a comment

Choose a reason for hiding this comment

Uh oh!

markstory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

armenzg commented Jun 13, 2025 •

edited

Loading

codecov bot commented Jun 13, 2025 •

edited

Loading