-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(preprod): add unique constraint to commit comparison #107371
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.
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
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.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
src/sentry/migrations/1023_add_commitcomparison_unique_constraints.py
Outdated
Show resolved
Hide resolved
|
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL
--
-- Create constraint sentry_commitcomparison_org_comparison_uniq on model commitcomparison
--
CREATE UNIQUE INDEX CONCURRENTLY "sentry_commitcomparison_org_comparison_uniq" ON "sentry_commitcomparison" ("organization_id", "head_repo_name", "head_sha", "base_sha") WHERE "base_sha" IS NOT NULL;
--
-- Create constraint sentry_commitcomparison_org_commit_uniq on model commitcomparison
--
CREATE UNIQUE INDEX CONCURRENTLY "sentry_commitcomparison_org_commit_uniq" ON "sentry_commitcomparison" ("organization_id", "head_repo_name", "head_sha") WHERE "base_sha" IS NULL; |
Tests were creating CommitComparison records with the same unique key values as the setUp fixture. Changed head_sha values to be unique per test to avoid IntegrityError.
| PreprodArtifact.objects.filter(commit_comparison_id=duplicate_id).update( | ||
| commit_comparison_id=kept_id | ||
| ) | ||
| CommitComparison.objects.filter(id__in=duplicate_to_kept.keys()).delete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how many rows there are to delete here, but it could be safer to just delete individually so that you don't hit any transaction timeouts.
Fixes https://sentry.sentry.io/issues/7229263157/?query=is%3Aunresolved%20assigned%3A%23emerge-tools&referrer=issue-stream&sort=date by adding a unique constraint.
Basically due to race conditions of this particular user sending a high volume of writes to us, we saw cases where multiple objects are created despite the
CommitComparison.objects.get_or_create()call within the transaction. This can cause some downstream issues that assume only oneCommitComparisonexists.