-
-
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
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
Show resolved
Hide resolved
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
|
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
2cc96dd to
af3ae18
Compare
af3ae18 to
581c5fa
Compare
2df1031 to
f4e9402
Compare
f4e9402 to
22949a2
Compare
tests/sentry/monitors/migrations/test_0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
22949a2 to
ed8a517
Compare
ed8a517 to
d7a51de
Compare
tests/sentry/monitors/migrations/test_0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
tests/sentry/monitors/migrations/test_0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
d7a51de to
beedaf8
Compare
beedaf8 to
18864ae
Compare
18864ae to
c6b7907
Compare
c6b7907 to
fa4915d
Compare
src/sentry/monitors/migrations/0011_backfill_monitor_environment_is_muted.py
Outdated
Show resolved
Hide resolved
fa4915d to
f96279b
Compare
f96279b to
7b5bb18
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
7b5bb18 to
bd3e3d1
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: