mvniekerk / tokio-cron-scheduler

Schedule tasks on Tokio using cron-like annotation
Apache License 2.0
529 stars 59 forks source link

TimeZone schedulers are using FixedOffset instead of Timezone #87

Open marioloko opened 3 weeks ago

marioloko commented 3 weeks ago

I have a problem with schedulers using timezones. They are using FixedOffset instead of Timezone, this causes to continue applying the same FixedOffset even when the timezone date is updated to SummerTime or WinterTime.

For example, this weekend, the 27th October 2024, was the "fall back", changing from Summer Time to Winter time, so in Berlin timezone, the 1:00AM was updated to 12:00AM, in the same way, 30th March 2025 will be the "spring forward", so in Berlin timezone the 2:00 AM will become the 3:00AM. For more info check here.

The problem with using FixedOffset with Timezones, is that Timezones are variable offsets, which change along the time, so using a fixed offset it will take the offset at the Job creation time, and it will not updated it at the "fall back" or "spring forward".

In our case, we expected the Job to be executed everyday at 10:00 AM Berlin time (8:00 AM UTC), but after the "fall back", the Job is being executed at 9:00 AM Berlin time (8:00 AM UTC).

The following Rust snippet shows the difference between TimeZones and FixedOffset:

use chrono::{DateTime, TimeZone, Utc, Offset, FixedOffset};
use chrono_tz::Europe::Berlin;
use croner::Cron;

fn main() {
    let schedule = Cron::new("0 0 10 * * *")
        .with_seconds_required()
        .with_dom_and_dow()
        .parse()
        .expect("Cannot parse cron");

    println!("Using timezone:");
    for (idx, time) in schedule
        .iter_from(Utc.timestamp_opt(1729728000, 0).unwrap().with_timezone(&Berlin)).take(5).enumerate()
    {
        let utc_time = DateTime::<Utc>::from_naive_utc_and_offset(time.naive_utc(), Utc);
        println!("{idx} - Timezoned: '{time}' - UTC: '{utc_time}'");
    }

    println!();

    println!("Using fixed offset:");
    let time_offset_seconds = Berlin
            .offset_from_utc_datetime(&Utc.timestamp_opt(1729728000, 0).unwrap().naive_local())
            .fix()
            .local_minus_utc();

    let fixed_offset = FixedOffset::east_opt(time_offset_seconds)
                                    .unwrap_or(FixedOffset::east_opt(0).unwrap());

    for (idx, time) in schedule
        .iter_from(Utc.timestamp_opt(1729728000, 0).unwrap().with_timezone(&fixed_offset)).take(5).enumerate()
    {
        let utc_time = DateTime::<Utc>::from_naive_utc_and_offset(time.naive_utc(), Utc);
        println!("{idx} - Timezoned: '{time}' - UTC: '{utc_time}'");
    }

}

The output is:

Using timezone:
0 - Timezoned: '2024-10-24 10:00:00 CEST' - UTC: '2024-10-24 08:00:00 UTC'
1 - Timezoned: '2024-10-25 10:00:00 CEST' - UTC: '2024-10-25 08:00:00 UTC'
2 - Timezoned: '2024-10-26 10:00:00 CEST' - UTC: '2024-10-26 08:00:00 UTC'
3 - Timezoned: '2024-10-27 10:00:00 CET' - UTC: '2024-10-27 09:00:00 UTC'
4 - Timezoned: '2024-10-28 10:00:00 CET' - UTC: '2024-10-28 09:00:00 UTC'

Using fixed offset:
0 - Timezoned: '2024-10-24 10:00:00 +02:00' - UTC: '2024-10-24 08:00:00 UTC'
1 - Timezoned: '2024-10-25 10:00:00 +02:00' - UTC: '2024-10-25 08:00:00 UTC'
2 - Timezoned: '2024-10-26 10:00:00 +02:00' - UTC: '2024-10-26 08:00:00 UTC'
3 - Timezoned: '2024-10-27 10:00:00 +02:00' - UTC: '2024-10-27 08:00:00 UTC'
4 - Timezoned: '2024-10-28 10:00:00 +02:00' - UTC: '2024-10-28 08:00:00 UTC'

The line that is causing the problem is this one. As it should apply the timezone instead of the offset, and the timezone should be stored in the Job itself.

marioloko commented 3 weeks ago

@mvniekerk I think the solution is store the timezone in JobStoreData object. However, it brings some problems:

  1. TZ is not object safe, so it cannot be stored as Box. Maybe and enum covering Local, Utc, and chrono_tz::Tz will be enough for all cases, but is less flexible than the TimeZone trait.
  2. It will not be serializable into prost easily, at least without redefining the struct to asign to it de tags to each enum variant.

Any idea of how to tackle this problem?