-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ref(crons): Add migration to backfill MonitorEnvironment.is_muted #103324
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?
ref(crons): Add migration to backfill MonitorEnvironment.is_muted #103324
Conversation
src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103324 +/- ##
===========================================
+ Coverage 79.87% 80.57% +0.69%
===========================================
Files 9237 9242 +5
Lines 394575 394768 +193
Branches 25146 25146
===========================================
+ Hits 315184 318087 +2903
+ Misses 78944 76234 -2710
Partials 447 447 |
|
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL |
dfd68e6 to
44fa9fa
Compare
| muted_monitors = list( | ||
| Monitor.objects.filter( | ||
| id__gt=last_id, | ||
| is_muted=True, | ||
| ).order_by( | ||
| "id" | ||
| )[:batch_size] | ||
| ) |
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.
You may as well just use RangeQuerySetWrapper and filter is_muted in memory. There's no index on id, is_muted so writing this manually won't help at all.
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.
look again
44fa9fa to
88a0a41
Compare
| muted_monitors = Monitor.objects.filter(is_muted=True) | ||
|
|
||
| for monitor in RangeQuerySetWrapperWithProgressBar(muted_monitors): |
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.
You're better off filtering is_muted in memory here instead of the query, otherwise you might end up with timeouts.
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.
good call
88a0a41 to
9524607
Compare
src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py
Show resolved
Hide resolved
9524607 to
196a9e9
Compare
src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py
Show resolved
Hide resolved
196a9e9 to
2cc96dd
Compare
Implements Step 2 of the migration plan from [NEW-564: There needs to be some way to mute the entire cron detector](https://linear.app/getsentry/issue/NEW-564/there-needs-to-be-some-way-to-mute-the-entire-cron-detector) This data migration backfills MonitorEnvironment.is_muted from Monitor.is_muted for all muted monitors. This ensures existing data is consistent before switching to read from MonitorEnvironment.is_muted. After this migration runs: - Monitors with is_muted=True will have ALL environments muted - Monitors with is_muted=False will have environments unchanged This establishes the correct invariant: - Monitor is muted ↔ ALL environments are muted - Monitor is unmuted ↔ ANY environment is unmuted The migration: - Uses range queries for efficient batching (1000 monitors per batch) - Only updates environments for muted monitors (unmuted is the default) - Is marked as post-deployment for manual execution in production Test verifies: - Muted monitor environments are updated to is_muted=True - Unmuted monitor environments remain unchanged - Monitors without environments are unaffected
2cc96dd to
af3ae18
Compare
Implements Step 2 of the migration plan from NEW-564: There needs to be some way to mute the entire cron detector
This data migration backfills MonitorEnvironment.is_muted from Monitor.is_muted for all muted monitors. This ensures existing data is consistent before switching to read from MonitorEnvironment.is_muted.
After this migration runs:
This establishes the correct invariant:
The migration:
Test verifies: