ref(grouping): Clear old and invalid secondary grouping config options#114378
Merged
lobsterkatie merged 4 commits intomasterfrom Apr 29, 2026
Merged
Conversation
github-actions
Bot
added
the
Scope: Backend
lobsterkatie
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
lobsterkatie
force-pushed
the
kmclb-clear-old-and-invalid-secondary-grouping-config-options
branch
from
30f6125 to
fb0ac8a
Compare
yuvmen
approved these changes
Apr 29, 2026
Member
yuvmen
left a comment
yuvmen
left a comment
There was a problem hiding this comment.
looks good, just a small question
lobsterkatie
deleted the
kmclb-clear-old-and-invalid-secondary-grouping-config-options
branch
cleptric
pushed a commit
that referenced
this pull request
May 5, 2026
#114378) Right now, we store an expiry date for each project's grouping config transition period, but we never go back and clean out old/expired transition settings once that period has ended. We also don't make any effort to fix invalid data if/when we find it. Other than wasting space in the database, this normally isn't an issue, unless the corrupt data leads our assumptions to be broken and our code to therefore fail, as happened recently when a project somehow got the same value set for its default and secondary grouping configs. (In that case, the fact that we discard new secondary hashes led us to also discard new primary hashes, breaking grouping[1].) To prevent future problems, and to clean up after ourselves, this PR therefore adds a new helper, `_clean_up_expired_config_options` (which, as the name suggests, deletes a project's secondary config and secondary config expiry `ProjectOption` records), and calls it when necessary in the spot where we're already checking the relevant values, namely in the existing `is_in_transition` function. Like grouping config upgrades, these database calls are guarded by both a cache key and a lock in order to prevent race conditions. It also turns out there were no tests specifically covering the transition check, so this adds them, and includes a check of the clean-up work as part of what they test. [1] https://sentry.sentry.io/issues/7368625348
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right now, we store an expiry date for each project's grouping config transition period, but we never go back and clean out old/expired transition settings once that period has ended. We also don't make any effort to fix invalid data if/when we find it.
Other than wasting space in the database, this normally isn't an issue, unless the corrupt data leads our assumptions to be broken and our code to therefore fail, as happened recently when a project somehow got the same value set for its default and secondary grouping configs. (In that case, the fact that we discard new secondary hashes led us to also discard new primary hashes, breaking grouping.)
To prevent future problems, and to clean up after ourselves, this PR therefore adds a new helper,
_clean_up_expired_config_options(which, as the name suggests, deletes a project's secondary config and secondary config expiryProjectOptionrecords), and calls it when necessary in the spot where we're already checking the relevant values, namely in the existingis_in_transitionfunction. Like grouping config upgrades, these database calls are guarded by both a cache key and a lock in order to prevent race conditions. It also turns out there were no tests specifically covering the transition check, so this adds them, and includes a check of the clean-up work as part of what they test.