playframework / play1

Play framework
https://www.playframework.com/documentation/1.4.x/home
Other
1.58k stars 682 forks source link

@On for Jobs that run at least once an hour will skip an hour once a year (when Daylight Saving ends) #1491

Open davidcostanzo opened 3 months ago

davidcostanzo commented 3 months ago

Play Version

1.6.0, 1.7.1

Describe the bug When the @On annotation is used to run a Job with a frequency of at least once an hour, it will skip an hour's worth of job executions when Daylight Saving time ends. When Daylight Saving time ends, the clocks roll back from 2 AM to 1 AM, so there are two 1 AM hours, one immediately after the other. The job does run during the first of these hours.

For example with the annotation @On("0 1/5 * * * ?"), my expectation is that the job runs every five minutes. However, on November 5th, 2023, there was an hour when the job did not run at all.

To Reproduce

The real-world repro is be difficult to set up, as it would requires setting the system clock and location. Instead, I have written a unit test that I think shows the cause of the problem, which is in CronExpression.getTimeAfter(Date). At the time of writing this, my machine is in daylight savings time, which might influence CronExpression, since it creates a new Calendar as a helper.

import java.text.ParseException;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Arrays;
import java.util.Date;
import java.util.TimeZone;

import org.junit.Test;

import play.libs.CronExpression;
import play.test.UnitTest;

public class CronExpressionTest extends UnitTest {

    private static void assertNextValidTimeIsWithinFiveMinutes(CronExpression cronExpression, Instant beforeInstant) {
        Date beforeDate = Date.from(beforeInstant);
        Date nextValidTime = cronExpression.getNextValidTimeAfter(beforeDate);

        long millisecondsUntilNextValidTime = nextValidTime.getTime() - beforeInstant.toEpochMilli();
        long fiveMinutesInMilliseconds = 5 * 60 * 1000;
        assertTrue("duration " + millisecondsUntilNextValidTime + " is greater than interval "
            + fiveMinutesInMilliseconds + " (five minutes)",
            millisecondsUntilNextValidTime <= fiveMinutesInMilliseconds);
    }

    @Test
    public void testDaylightSavingEnd() throws ParseException {
        // Create a CronExpression that runs every five minutes.
        CronExpression everyFiveMinutes = new CronExpression("0 1/5 * * * ?");

        // Set the time zone to Eastern Time.
        String easternTime = "America/New_York";
        assertTrue(easternTime + " does not exist", Arrays.asList(TimeZone.getAvailableIDs()).contains(easternTime));
        TimeZone easternTimeZone = TimeZone.getTimeZone(easternTime);
        everyFiveMinutes.setTimeZone(easternTimeZone);

        // Self-check, the next valid time when the minute rolls over the hour mark
        // but the hour is still within daylight savings time.
        Instant easyDate = Instant.parse("2023-11-05T00:59:00Z");
        assertNextValidTimeIsWithinFiveMinutes(everyFiveMinutes, easyDate);

        // The difficult date is when the next valid time is within the hour that gets
        // repeated by the switch from daylight saving time to standard time.
        Instant aMinuteBeforeDaylightSavingEnd = easyDate.plus(5, ChronoUnit.HOURS); // The EDT offset is -04.
        assertNextValidTimeIsWithinFiveMinutes(everyFiveMinutes, aMinuteBeforeDaylightSavingEnd);
    }
}

Expected behavior The test passes. That is, regardless of what time is given to CronExpression.getNextValidTimeAfter(Date), the next valid time is always within the next five minutes.

Screenshots N/A

Desktop (please complete the following information):

# uname -a
Linux mouse 5.15.0-89-generic #99-Ubuntu SMP Mon Oct 30 20:42:41 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
# java -version
openjdk version "14.0.2" 2020-07-14
OpenJDK Runtime Environment AdoptOpenJDK (build 14.0.2+12)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 14.0.2+12, mixed mode, sharing)

Smartphone (please complete the following information): N/A

Additional context I do see there is some handling of Daylight Saving Time in CronExpression.setCalendarHour, but that looks like it's for when entering Daylight Saving time (when the clocks roll forward). This bug is for when the clocks roll backward.

xabolcs commented 2 months ago

Nice catch!

There is a Linux tool, faketime and there are a few Java lib, e.g. https://github.com/faketime-java/faketime/