dhiaayachi / temporal

Temporal service
https://docs.temporal.io
MIT License
0 stars 0 forks source link

Wrong handling of sophisticated CRON schedule #368

Open dhiaayachi opened 2 months ago

dhiaayachi commented 2 months ago

Expected Behavior

We have a new workflow that should be effectively disabled during PROD deployment until the development is fully completed. To achieve that, we specify a CRON schedule 0 0 29 2 1. The idea is:

The next execution date for this crontab entry would be February 29th of the next leap year which falls on a Monday. According to the calendar, that should happen in the year 2032 so we have plenty of time to complete the development.

Actual Behavior

In fact, for some reason Temporal CRON schedules that differently image

I've tried checking the sleep period manually and it does not correspond to the CRON specified. Is there some integer overflow?

> date -d 'now + 6604 hours 52 minutes 13 seconds'
Mon 05 Feb 2024 02:13:18 AM EET

Steps to Reproduce the Problem

  1. specify a CRON schedule 0 0 29 2 1
  2. schedule some workflow
  3. take a look at expected date

Specifications

dhiaayachi commented 1 month ago

Thank you for reporting this issue. It seems that there might be a problem with the Temporal Cron Schedule parsing logic, as the expected execution date is not what the CLI is showing.

I recommend you to use the temporal schedule create command with --calendar, --interval, or --cron options to set the desired schedule. This allows you to create a schedule that will execute on the desired date.

If you are still encountering the issue, please provide the following information:

This information will help us to diagnose the problem and provide you with a more specific solution.

dhiaayachi commented 1 month ago

Thank you for reporting this issue. The issue appears to be related to the way Temporal CRON schedules are handled. We are aware of this issue.
Based on the documentation, Temporal does not use the traditional CRON implementation, instead it uses a more reliable and robust approach. Temporal does not guarantee that the CRON implementation will be compatible with other CRON implementations. You may want to consider using the Schedule API instead of CRON, which provides a more robust and feature-rich scheduling mechanism. More information can be found here: https://docs.temporal.io/workflows#schedule You may also want to take a look at the Temporal CRON implementation: https://github.com/temporalio/temporal/blob/master/common/service/history/events/event_converter.go

dhiaayachi commented 1 month ago

Thank you for reporting this issue!

I can see you're observing an issue with Temporal's CRON schedule implementation where the computed next execution time doesn't align with your expectations, particularly for leap years. It seems like the computed next execution time is not considering the leap year properly.

The date command you used to manually calculate the next execution time appears to be correct, but the Temporal server's CRON implementation is different. You've also tried to debug the issue by manually computing the next execution time and comparing it with the one Temporal calculates, and the discrepancy is quite significant.

To investigate the discrepancy, I'd need some additional information:

Once I have more details, I'll do my best to find a solution or provide a workaround for this issue.