Skip to content

fix(cleanup): Sort IDs before bulk deletion to prevent deadlocks - #128136

Open
sentry[bot] wants to merge 3 commits into
masterfrom
seer/fix/cleanup-bulk-delete-deadlock
Open

sentry[bot] wants to merge 3 commits into
masterfrom
seer/fix/cleanup-bulk-delete-deadlock

Conversation

@sentry

@sentry sentry Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When running cleanup with concurrency > 1, multiple worker processes concurrently deleting child relations (e.g. sentry_groupemailthread) hit PostgreSQL deadlocks (OperationalError: deadlock detected).

The root cause was that bulk_delete_objects generated a self-referencing subquery DELETE:

DELETE FROM sentry_groupemailthread WHERE id IN (
    SELECT id FROM sentry_groupemailthread WHERE group_id IN (...)
) LIMIT 10000

This two-phase lock acquisition (reading rows via the subquery, then acquiring exclusive delete locks) causes circular waits when multiple workers touch overlapping or interleaved rows.

Solution

  1. Materialize IDs first via a separate SELECT ... ORDER BY id query, breaking the self-join pattern.
  2. Pass the concrete IDs to DeleteQuery, keeping the raw single-query DELETE with no ORM cascading or signal overhead.
  3. Early-exit if the ID list is empty to avoid an unnecessary DELETE execution.
  4. Update test_bulk_delete_single_query to reflect the 2-query pattern (SELECT + DELETE).

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Fixes SENTRY-5XCB

@sentry <feedback>: Autofix iterates on these changes
@sentry stop iterating: Autofix stops iterating on this run

@sentry
sentry Bot requested a review from a team as a code owner October 19, 2026 23:42
@sentry sentry Bot assigned yuvmen Oct 19, 2026
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 19, 2026
@yuvmen yuvmen added the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Oct 19, 2026
@github-actions github-actions Bot removed the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Oct 19, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit af31763. Configure here.

Comment thread src/sentry/utils/query.py Outdated
@sentry
sentry Bot force-pushed the seer/fix/cleanup-bulk-delete-deadlock branch from af31763 to bb46d99 Compare October 19, 2026 23:47
Comment thread src/sentry/utils/query.py Outdated
) -> bool:
qs = model.objects.filter(**filters).values_list("id")[:limit]
# Sort IDs first to ensure consistent lock ordering across concurrent workers, preventing deadlocks.
ids = sorted(model.objects.filter(**filters).values_list("id", flat=True)[:limit])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sentry another clanker said this python sorting may not guarantee deletion lock order b/c when processing the SQL IN, postgres could ultimately scan in a different order than the sorted input. Can you confirm or deny? Is there a better approach to use here?

…dlocks

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
@jamieQ

jamieQ commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

@sentry here's additional feedback from another clanker:


Production PostgreSQL logs at 2026-10-19 07:30:22 UTC show the competing process, PID 2408941, executing:
DELETE FROM sentry_groupemailthread
WHERE id = ANY(ARRAY(
SELECT id FROM sentry_groupemailthread
WHERE "date" < ''::timestamptz
LIMIT 10000
));
The failing process, PID 2374048, was executing the group-based deletion captured in the Sentry event.
Investigate how these two statements interact and whether the current patch addresses that interaction. Substantiate the proposed cause with PostgreSQL’s locking behavior.
Before implementing another fix, build a concurrency reproduction involving both paths. Use it to evaluate the current patch, then validate your revised fix.


edit:
@sentry seems like you got stuck... can you try again?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants