Skip to content

ref(db): Remove unused unique constrained from Group #103144

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+33 −2
Closed

ref(db): Remove unused unique constrained from Group #103144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ releases: 0003_real_delete_dual_written_commit_tables

replays: 0006_add_bulk_delete_job

sentry: 1003_group_history_prev_history_safe_removal
sentry: 1004_remove_group_project_short_id_unique_constraint

social_auth: 0003_social_auth_json_field

Expand Down
32 changes: 32 additions & 0 deletions src/sentry/migrations/1004_remove_group_project_short_id_unique_constraint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 5.2.1 on 2025-11-10 12:30

from django.db import migrations

from sentry.new_migrations.migrations import CheckedMigration


class Migration(CheckedMigration):
# This flag is used to mark that a migration shouldn't be automatically run in production.
# This should only be used for operations where it's safe to run the migration after your
# code has deployed. So this should not be used for most operations that alter the schema
# of a table.
# Here are some things that make sense to mark as post deployment:
# - Large data migrations. Typically we want these to be run manually so that they can be
# monitored and not block the deploy for a long period of time while they run.
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
# run this outside deployments so that we don't block them. Note that while adding an index
# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False

dependencies = [
("sentry", "1003_group_history_prev_history_safe_removal"),
]

operations = [
migrations.AlterUniqueTogether(
name="group",
unique_together=set(),
),
]
Comment on lines +22 to +32
Copy link

@sentry sentry bot Nov 11, 2025

Choose a reason for hiding this comment

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

Bug: Removing the (project, short_id) unique constraint disables critical "stuck counter" error detection, leading to silent duplicate group creation.
Severity: CRITICAL | Confidence: 0.95

🔍 Detailed Analysis

The removal of the unique constraint (project, short_id) from the Group model will disable the existing error handling mechanism for "stuck counter" conditions. Specifically, the _is_stuck_counter_error function, which relies on catching psycopg2.errors.UniqueViolation for the sentry_groupedmessage_project_id_short_id constraint, will no longer trigger. This means that if the Counter mechanism for generating short_id values gets out of sync with existing group short_ids (a condition that has occurred in production, as evidenced by the comment "< 20 ever that we know of" in _handle_stuck_project_counter), duplicate groups with identical (project_id, short_id) values will be silently created instead of being detected and recovered.

💡 Suggested Fix

Either re-evaluate the necessity of removing the (project, short_id) unique constraint, or update the _is_stuck_counter_error and _handle_stuck_project_counter functions to use an alternative, robust mechanism for detecting and recovering from short_id synchronization issues without relying on a database unique constraint.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
src/sentry/migrations/1004_remove_group_project_short_id_unique_constraint.py#L1-L32

Potential issue: The removal of the unique constraint `(project, short_id)` from the
`Group` model will disable the existing error handling mechanism for "stuck counter"
conditions. Specifically, the `_is_stuck_counter_error` function, which relies on
catching `psycopg2.errors.UniqueViolation` for the
`sentry_groupedmessage_project_id_short_id` constraint, will no longer trigger. This
means that if the `Counter` mechanism for generating `short_id` values gets out of sync
with existing group `short_id`s (a condition that has occurred in production, as
evidenced by the comment "< 20 ever that we know of" in
`_handle_stuck_project_counter`), duplicate groups with identical `(project_id,
short_id)` values will be silently created instead of being detected and recovered.

Did we get this right? 👍 / 👎 to inform future reviews.

1 change: 0 additions & 1 deletion src/sentry/models/group.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,6 @@ class Meta:
models.Index(fields=("status", "substatus", "first_seen")),
models.Index(fields=("project", "status", "priority", "last_seen", "id")),
]
unique_together = (("project", "short_id"),)

__repr__ = sane_repr("project_id")

Expand Down
Loading