-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(crons): Implement fallback clock pulse task #54647
feat(crons): Implement fallback clock pulse task #54647
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #54647 +/- ##
==========================================
- Coverage 79.78% 79.77% -0.01%
==========================================
Files 5000 5000
Lines 212248 212271 +23
Branches 36162 36164 +2
==========================================
+ Hits 169340 169348 +8
- Misses 37700 37712 +12
- Partials 5208 5211 +3
|
d8418ef
to
2fb5dfe
Compare
@@ -129,6 +146,30 @@ def try_monitor_tasks_trigger(ts: datetime): | |||
_dispatch_tasks(ts) | |||
|
|||
|
|||
@instrumented_task(name="sentry.monitors.tasks.clock_pulse", silo_mode=SiloMode.REGION) | |||
def clock_pulse(current_datetime=None): |
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.
Shouldn't this run conditionally based on a setting?
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.
No, as per fillipos comment #53661 (comment), we decided to just always have it run. Then we don't have a "high" or "low" volume mode. it is just adaptive.
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.
If this is running all the time then doesn't it break the case where there's an ingest backlog?
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.
No because it is still putting the message into the kafka topic, thus into the queue.
This would be problematic if we turned off producers (relay)
2fb5dfe
to
c66b615
Compare
This is the last piece of GH-53661 to ensure tasks are triggered in scenarios where there is not enough volume.