PeriodicTasks with a TZ are not running

[radoshi]

Hi,

We noticed a failure on all periodic tasks with a timezone set, after moving to 2.8.0. We believe this a regression caused by #844

Any tasks that keyed to a CrontabSchedule set in a specific timezone are not being scheduled.

For example:

def _get_crontab_schedule(
    minute: str = "*",
    hour: str = "*",
    day_of_month: str = "*",
    month_of_year: str = "*",
    day_of_week: str = "*",
    timezone: zoneinfo.ZoneInfo = zoneinfo.ZoneInfo("UTC"),
) -> CrontabSchedule:
    return CrontabSchedule.objects.get_or_create(
        minute=minute,
        hour=hour,
        day_of_month=day_of_month,
        month_of_year=month_of_year,
        day_of_week=day_of_week,
        timezone=timezone,
    )
...
    schedule_every_day_7_am_pst, _ = _get_crontab_schedule(
        minute="0",
        hour="7",
        timezone=zoneinfo.ZoneInfo("America/Los_Angeles"),
    )
    PeriodicTask.objects.update_or_create(...)

This task did not get scheduled.

IntervalSchedules seem fine. CrontabSchedule with a UTC also seems fine.

We're happy to try to do a minimal repro, though its a bit complicated, if that would be helpful.

[radoshi]

cc JinRiYao2001

[auvipy]

Thanks for the report

[ZipBrandon]

@radoshi Thank you for identifying the crux of the issue. We are experience failures on our end, too. I could not find out the root-cause as I thought it was with my arguments being passed to tasks.

[gsvr]

@radoshi @auvipy We've experienced tasks not running as well, but it's a bit of a mystery what the underlying cause is, as I've had ones with timezones set that work, and UTC ones that don't. Below is a screenshot of our periodic tasks and when they last ran. We deployed 2.8.0 on 20 April.

Edit: The second and third items on the screenshot should be ignored, they have correctly not run yet.

[ZipBrandon]

Here is my list with green checkmarks of what is running and red X's for what isn't running on time. I'm having to manually run the jobs. Some jobs update last runtime when I do it, some do not even though I see the output of the success of their runs in my data.

[Azurency]

I'm experiencing the same issue with the new 2.8.0 release. It's also not quite clear why, it seems to happen to tasks with timezone and UTC, some of them are just not scheduled while others run fine.

[Azurency]

I think there is also an issue with #835 it is excluding tasks based on crontab hour without taking timezones into account. Reverting commit fc64741 seems to allow tasks with timezone to run on schedule again for me (but it's still not explaining why some UTC tasks are not scheduled).

[mrzorn]

Just adding that we are experiencing the same issue, some of our PeriodicTasks did not run this morning but got picked up as soon as we downgraded to 2.7.0.

[zoetw88]

I wanted to bring up a potential concern with the exclusion logic in get_excluded_hours_for_crontab_tasks().

The current implementation excludes crontab tasks based on the server's local time, which might lead to unintended issues for UTC or timezone-aware tasks. Specifically:

• UTC tasks should be compared using UTC time rather than the server's local time, to avoid the risk of accidentally excluding them.

• Timezone-aware tasks are more likely to be affected since their scheduled hour is converted to the server’s local time before being compared against the exclusion window.

This approach may unintentionally skip tasks that are scheduled correctly based on their respective timezones.

Suggested Fix:

For UTC tasks, avoid converting them to the server's local time and compare directly against UTC time.

For timezone-aware tasks, convert them to the server's local time and then apply the exclusion check.

Performance Consideration:
I understand that this might introduce additional computation, especially when dealing with a large number of tasks. A possible way to mitigate this impact could be:

Cache the time zone conversion during task creation or update, so that we don't repeatedly convert the time on every exclusion check. This would keep the logic correct without incurring significant performance overhead.

I'd be happy to help with a patch to implement this change and create tests for it. Let me know if you'd like me to proceed with this.

[JinRiYao2001]

@radoshi
Thank you for reporting this. The PR I submitted only affects crontab periodic tasks where start_time is set and it's in the future. Could you please provide more code or a minimal reproduction to help investigate further?

[auvipy]

we have this draft fix #877, can anyone try it ?

[zoetw88]

@auvipy I'm able to take care of this and run some quick tests based on my observations. However, I’ll be available on Friday or Saturday. Please let me know if that works for the timeline.

[auvipy]

I am available on saturday as well

[michalpokusa]

We are also having problems, and I noticed that setting the cron for e.g. 59 14 * * * schedules the task while using e.g. 0 15 * * * does not work. Might be ralated to #875 (comment)

[Azurency]

I don't think #877 will be enough.
It is just making sure that the excluded hours are in UTC, which is needed for UTC tasks but as @zoetw88 said in #875 (comment) for timezone-aware tasks, there needs to be some kind of conversion, the simplest way is to convert in UTC.

This problem is that this line compares excluded hours (preferably UTC with the change in #877) to the hour field of crontab which is in the timezone the user specified, not in UTC

django-celery-beat/django_celery_beat/schedulers.py

Line 265 in cf04a7b

crontab__isnull=False, crontab__hour__in=exclude_hours

The UTC hour could be computed every query with something like that Azurency@ce00300 (this is just something I tried, to see if that worked on some tasks I was scheduling).
But I think the best solution would be to store the UTC hour and UTC minute inside the Crontab model like @zoetw88 said "during task creation or update".