Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CronExpression fails to calculate properly next execution when running on the day of winter daylight saving time #28095

Closed
guilroux opened this issue Feb 22, 2022 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@guilroux
Copy link

Affects: Spring Framework 5.3.16

The PR #28044 fixing the issue #28038 did cause a regression for winter daylight saving time

Problem: CronExpression fails to calculate properly next execution when running on the day of winter daylight saving time, just before DST is applied.

Here is a unit test case that you can add to CronExpressionTests#daylightSaving to reproduce the problem :

cronExpression = CronExpression.parse("0 5 0 * * *");

last = ZonedDateTime.parse("2019-10-27T01:05+02:00[Europe/Paris]");
expected = ZonedDateTime.parse("2019-10-28T00:05+01:00[Europe/Paris]");
actual = cronExpression.next(last);
assertThat(actual).isNotNull();
assertThat(actual).isEqualTo(expected);

Please note that in the previous version 5.3.15 everything worked fine for that case

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 22, 2022
@poutsma poutsma self-assigned this Feb 23, 2022
@poutsma poutsma added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 24, 2022
@poutsma poutsma added this to the 5.3.17 milestone Feb 24, 2022
@poutsma poutsma added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 24, 2022
@poutsma
Copy link
Contributor

poutsma commented Feb 24, 2022

@guilroux Thank you for spotting this, without it users would have had invalid cron executions in October.

I have completely revised the way CronExpression rolls forward temporal units, see 7e2106b

Let me know if you see additional issues with this implementation, or have other concerns.

@mf81bln
Copy link

mf81bln commented Mar 28, 2022

@poutsma

We had an issue with Spring Boot 2.6.4 / Framework 5.3.16 on DST switch yesterday (March 27)
We're using a simple TaskScheduler bean and a method annotated with @Scheduler(cron = "0 30 2 * * ?")

The execution was just skipped when time was jumping from 2am to 3am. I've tried to...

  • add the time zone to the taskScheduler
    @Bean
    public TaskScheduler taskScheduler(final TaskSchedulerBuilder taskSchedulerBuilder)
    {
        return taskSchedulerBuilder
            .poolSize(10)
            .customizers(taskScheduler -> taskScheduler.setClock(Clock.system(ZoneId.of("Europe/Zurich"))))
            .build();
    }
  • add zone property to @Scheduled
    @Scheduled(cron = "0 30 2 * * ?", zone = "Europe/Zurich")
    public void run() { ... }

Both didn't work - tried it by resetting system time to 01:59:30am.

During debugging I found CronTrigger.nextExecutionTime() always returning 2022-03-28T02:30:00+02:00
Is there anything else I have to configure so that the execution is not just skipped? Or do I need a custom handling for this?

I've checked documentation as well as previous issues / fixes and couldn't find any specifics on how to properly handle this scenario where an execution falls into the "skipped" time frame between 2am ... 3am

// edit:
I've also tried it with the newest versions Spring Boot 2.6.5 / Framework 5.3.17 (because this was fixed) and resetting the time of my machine to March 27, 01:59:30 - the execution was skipped again

@poutsma
Copy link
Contributor

poutsma commented Mar 29, 2022

@mf81bln Please create a new issue instead of commenting on older, resolved issues. Feel free to ping me on said new issue.

FWIW, this works fine for me:

cronExpression = CronExpression.parse("0 30 2 * * ?"); // 01:59:30am
last =     ZonedDateTime.parse("2022-03-26T01:59:30+01:00[Europe/Zurich]");
expected = ZonedDateTime.parse("2022-03-26T02:30:00+01:00[Europe/Zurich]");
actual = cronExpression.next(last);
assertThat(actual).isEqualTo(expected);

@mf81bln
Copy link

mf81bln commented Mar 29, 2022

@poutsma Sorry and thanks for your answer, I've created a new issue 👍

Your example works because it's running on March 26, switch to DST happened on Sunday, March 27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants