Celery Beat Health Check: Check for Overdue Tasks Accounting for Scheduler Interval #297
Conversation
Maker-Mark
changed the title
|
@codingjoe Is there a contributing doc that I should be looking at? I can't seem to find how to request a review from a maintainer. Thanks. |
|
@skarzi Can I please get a review on this? I've had this open for a long time. |
| @@ -16,9 +16,11 @@ The following health checks are bundled with this project: | |||
| - AWS S3 storage | |||
| - Celery task queue | |||
| - Celery ping | |||
| - Celery Beat Health Check (via `django_celery_beat`) | |||
There was a problem hiding this comment.
Why using django_celery_beat is required? Have you checked if this check could be implemented based only on celery?
| technically due according to the scheduler, but that's only because the scheduler has not hit its | ||
| interval to check and process due tasks. | ||
| """ | ||
| from celery.app import default_app as celery_app |
There was a problem hiding this comment.
Let's use the current_app proxy to get the currently used app, not the default one (fallback one)
Also, I think (but haven't tested it) that this import can be moved to the root module imports section because current_app is just a proxy.
| scheduler = import_string(scheduler_module_path) | ||
| try: | ||
| # Get the celery scheduler for the current app and scheduler class via a beat Service | ||
| schedule: Dict = ( |
There was a problem hiding this comment.
NIP: always add type hintings for key and value when using Dict
| schedule_tasks: Union[ModelEntry, Any] = schedule.values() | ||
| tasks_due: List[Union[ModelEntry, Any]] = [] |
There was a problem hiding this comment.
Let's try to get rid of Any in type hintings because it makes them too vague
| bool: If the task is overdue by at least the buffer. | ||
| """ | ||
| EXPECTED_SCHEDULE_CLASSES = [solar, crontab, schedule] | ||
| DEFAULT_DUE_BUFFER_SECONDS = 30 |
There was a problem hiding this comment.
NIP: let's move it to module-level constant. Also Final type annotation can be used to indicate it's a constant
| try: | ||
| celery_schedule: Union[solar, crontab, schedule] = task_schedule.schedule | ||
| due_in = celery_schedule.remaining_estimate(task.last_run_at) | ||
| except BaseException: |
There was a problem hiding this comment.
Let's use Exception or narrow possible exception type even more
|
|
||
| # Get the celery schedule from the Modelentry. | ||
| celery_scheduler = task_schedule.schedule | ||
| if celery_scheduler.__class__ not in EXPECTED_SCHEDULE_CLASSES: |
There was a problem hiding this comment.
Why not isinstance(celery_scheduler, ...)?
Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>
Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>
Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>
Co-authored-by: Łukasz Skarżyński <skarzynski_lukasz@protonmail.com>
|
Is this PR going to be updated or merged sooner o later? It will be very good to also have a checker for celery-beat. |
|
@sevdog Hey, sorry I got busy and did not update the author's feedback. Do you want to finish it up? Or I can try to get to it in a few weeks, but no promises. |
|
Hi @Maker-Mark, currently I also have few spare time to work on this. I was just trying to ping for more help regarding celery support in this package. Feel free to work on this when you get some time! |
Overview of Health Check beat_health_check
The CeleryBeatHealth Check:
Checks for overdue tasks in a celery beat scheduler.
Uses the scheduler module dotted path that is specified in settings.py with
CELERY_BEAT_SCHEDULER.If not specified, defaults to
django_celery_beat'sdjango_celery_beat.schedulers.DatabaseScheduler.Allows a custom buffer to be set using
BEAT_HEALTH_CHECK_BUFFER_SECONDSin settings.py. The bufferdefaults to 30 seconds if not defined. The buffer will offset the scheduler interval for when due
tasks are processed. Using a buffer avoids false positives, such as the case where a task is
technically due according to the scheduler, but that's only because the scheduler has not hit its
interval to check and process due tasks.
I would love a review on this check and will be happy to address comments in a timely fashion.
Thanks!