quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

alanmscott · 2024-07-05T19:28:32Z

Describe the bug

When using the Quarkus Scheduler, cron triggering works properly with respect to the configured timezone. However, when interrogating the scheduler for the "getNextFireTime", the timezone is not factored in. This results in incorrect behaviour, most problematically, is that it can return a next fire time in the past.

See example class which demonstrates the results coming back.

Expected behavior

When calling getNextFireTime from the scheduler, the Instant returned should match the actual nextFireTime defined by the cron AND timezone, not simply the cron as parsed in the local timezone.

Actual behavior

When calling getNextFireTime from the scheduler, the Instant returned treats the cron as if it had the local timezone.

How to Reproduce?

package org.example;

import java.time.Instant;

import io.quarkus.runtime.StartupEvent;
import io.quarkus.scheduler.Scheduler;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.event.Observes;
import jakarta.inject.Inject;

@ApplicationScoped
public class TestScheduler {

  @Inject
  Scheduler scheduler;

  public void init(@Observes StartupEvent startupEvent) {

    scheduler.newJob("testJob1")
             .setCron("0 30 20 ? * * *")
             .setTimeZone("Europe/Berlin")
             .setTask(execution -> System.out.println("Job has run"))
             .schedule();

    scheduler.newJob("testJob2")
             .setCron("0 30 20 ? * * *")
             .setTimeZone("Europe/London")
             .setTask(execution -> System.out.println("Job has run"))
             .schedule();

    Instant nextFireTime1 = scheduler.getScheduledJob("testJob1").getNextFireTime();
    Instant nextFireTime2 = scheduler.getScheduledJob("testJob2").getNextFireTime();

    System.out.println("=============================================================");
    System.out.println("NextFireTimes:");
    System.out.println("TestJob1(Berlin)=" + nextFireTime1);
    System.out.println("TestJob2(London)=" + nextFireTime2);
    System.out.println("compareTo result=" + nextFireTime1.compareTo(nextFireTime2) + "    <-- should be greater than 0");
    System.out.println("=============================================================");
    // These two times should be different as they are crons with same local time but diff timezone, so UTC time expressed by the Instant should be different.
  }

}

Output of `uname -a` or `ver`

Linux alan-test-redis-cacher-56d9f6f6d4-ssstk 5.4.0-42-generic #46-Ubuntu SMP Fri Jul 10 00:24:02 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Output of `java -version`

openjdk version "21.0.1" 2023-10-17 LTS

Quarkus version or git rev

3.10.0

Build tool (ie. output of `mvnw --version` or `gradlew --version`)

No response

Additional information

No response

The text was updated successfully, but these errors were encountered:

quarkus-bot · 2024-07-05T19:28:35Z

/cc @manovotn (scheduler), @mkouba (scheduler)

mkouba · 2024-07-08T09:43:52Z

The Trigger#getNextFireTime() method and others (i.e. ScheduledExecution#getFireTime()) intentionally return an Instant which lacks the timezone info. If you need to work with a specific timezone then use the ZonedDateTime returned from the Instant#atZone(ZoneId).

alanmscott · 2024-07-08T10:03:32Z

Hi @mkouba - thanks for chipping in.
Sadly the Instant being returned does not match the actual instant that the job will next run at though.

I tried putting in a work around by returning the Instant and then forcing it into the timezone, using the methods you mention, but that has the consequence that it can return "nextFireTime" of an event that has already triggered (i.e. in the past).

As the scheduled job has the capability of defining a timezone, then that must be considered when returning a timezone agnostic Instant. The Instant of 20:30 London is not the same as the Instant of 20:30 Berlin.

mkouba · 2024-07-08T13:05:03Z

The Instant of 20:30 London is not the same as the Instant of 20:30 Berlin.

Of course, it isn't. And I agree that it's confusing.

It works like this - all Instants are obtained from the default time zone. Also all triggers are compared to now() which is obtained from the default time zone. For example, on my machine the default timezone is Europe/Prague and the next fire time for the cron 0 30 20 ? * * * would be something like 2024-07-08T18:30:00Z.

Now if I have 0 30 20 ? * * * for Europe/Prague then getNextFireTime().atZone(ZoneId.of("Europe/Prague") returns 2024-07-08T20:30+02:00[Europe/Prague]. And 0 30 20 ? * * * for Europe/London returns 2024-07-08T19:30+01:00[Europe/London]. Which is correct because it should be triggered one hour earlier.

In your code, nextFireTime1.compareTo(nextFireTime2) should return 1 because testJob1 should be executed after testJob2. And it does return 1 on my machine.

Does that make sense? Would it help if we clarify the javadoc in a way that getNextFireTime() returns an Instant obtained from the default time zone?

alanmscott · 2024-07-08T13:58:30Z

Hi,

Thanks for sticking with me!

And 0 30 20 ? * * * for Europe/London returns 2024-07-08T19:30+01:00[Europe/London]. Which is correct because it should be triggered one hour earlier.

Shouldn't this return 2024-07-08T20:30+01:00[Europe/London] if the cron has been set to 0 30 20 with a timezone of Europe/London?

I always get the same Instant for both cron schedules, so the compareTo always returns 0.

Out of interest, if you try the same experiment, using a cron schedule of 10-15mins in your future with a more easterly timezone, e.g. Europe/Istantbul, do you get a value for today? Or is it correctly recognising that the trigger actually will next take place tomorrow?
In my experiments, it still returns a value from today, even though that time has passed in the cron target timezone.

In case it helps, here is my output:

Expanded test class:

package org.example;

import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;

import io.quarkus.runtime.StartupEvent;
import io.quarkus.scheduler.Scheduler;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.event.Observes;
import jakarta.inject.Inject;

@ApplicationScoped
public class TestScheduler {

  @Inject
  Scheduler scheduler;

  public void init(@Observes StartupEvent startupEvent) {

    String localCron = "0 30 15 ? * * *";

    scheduler.newJob("testJob1")
             .setTimeZone("UTC")
             .setCron(localCron)
             .setTask(execution -> System.out.println("Job1 has run"))
             .schedule();

    scheduler.newJob("testJob2")
             .setTimeZone("Europe/London")
             .setCron(localCron)
             .setTask(execution -> System.out.println("Job2 has run"))
             .schedule();

    scheduler.newJob("testJob3")
             .setTimeZone("Europe/Berlin")
             .setCron(localCron)
             .setTask(execution -> System.out.println("Job3 has run"))
             .schedule();

    scheduler.newJob("testJob4")
             .setTimeZone("Europe/Istanbul")
             .setCron(localCron)
             .setTask(execution -> System.out.println("Job4 has run"))
             .schedule();

    Instant nextFireTime1 = scheduler.getScheduledJob("testJob1").getNextFireTime();
    Instant nextFireTime2 = scheduler.getScheduledJob("testJob2").getNextFireTime();
    Instant nextFireTime3 = scheduler.getScheduledJob("testJob3").getNextFireTime();
    Instant nextFireTime4 = scheduler.getScheduledJob("testJob4").getNextFireTime();

    System.out.println("=============================================================");
    System.out.println("NextFireTimes:");
    System.out.println("TestJob1(UTC     )=" + nextFireTime1 + " / " + nextFireTime1.atZone(ZoneId.of("UTC")).format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
    System.out.println("TestJob2(London  )=" + nextFireTime2 + " / " + nextFireTime1.atZone(ZoneId.of("Europe/London")).format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
    System.out.println("TestJob3(Berlin  )=" + nextFireTime3 + " / " + nextFireTime1.atZone(ZoneId.of("Europe/Berlin")).format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
    System.out.println("TestJob4(Istanbul)=" + nextFireTime4 + " / " + nextFireTime1.atZone(ZoneId.of("Europe/Istanbul")).format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
    System.out.println("compareTo result1-2=" + nextFireTime1.compareTo(nextFireTime2) + "    <-- should be greater than 0");
    System.out.println("compareTo result2-3=" + nextFireTime2.compareTo(nextFireTime3) + "    <-- should be greater than 0");
    System.out.println("compareTo result3-4=" + nextFireTime3.compareTo(nextFireTime4) + "    <-- should be greater than 0");
    System.out.println("=============================================================");
    // These two times should be different as they are crons with same local time but diff timezone, so UTC time should be different.
  }


}

OUTPUT:

=============================================================
NextFireTimes:
TestJob1(UTC     )=2024-07-08T14:30:00Z / 2024-07-08T14:30:00Z[UTC]
TestJob2(London  )=2024-07-08T14:30:00Z / 2024-07-08T15:30:00+01:00[Europe/London]
TestJob3(Berlin  )=2024-07-08T14:30:00Z / 2024-07-08T16:30:00+02:00[Europe/Berlin]
TestJob4(Istanbul)=2024-07-08T14:30:00Z / 2024-07-08T17:30:00+03:00[Europe/Istanbul]
compareTo result1-2=0    <-- should be greater than 0
compareTo result2-3=0    <-- should be greater than 0
compareTo result3-4=0    <-- should be greater than 0
=============================================================

Expected result:

TestJob1(UTC     )=2024-07-08T15:30:00Z / 2024-07-08T15:30:00Z[UTC]
TestJob2(London  )=2024-07-08T14:30:00Z / 2024-07-08T15:30:00+01:00[Europe/London]       <-- same day as 15:30 London has not yet passed 
TestJob3(Berlin  )=2024-07-09T13:30:00Z / 2024-07-08T15:30:00+02:00[Europe/Berlin]        <-- next day as 15:30 Berlin has passed already
TestJob4(Istanbul)=2024-07-09T12:30:00Z / 2024-07-08T15:30:00+03:00[Europe/Istanbul]       <-- next day as 15:30 Istanbul has passed already

alanmscott · 2024-07-08T14:03:03Z

And just to clarify, the triggering itself is working fine. The jobs are only actually triggered at the expected times.

mkouba · 2024-07-08T14:29:24Z

And 0 30 20 ? * * * for Europe/London returns 2024-07-08T19:30+01:00[Europe/London]. Which is correct because it should be triggered one hour earlier.

Shouldn't this return 2024-07-08T20:30+01:00[Europe/London] if the cron has been set to 0 30 20 with a timezone of Europe/London?

Not really. I've tried to explain in my previous comment that the next fire time is always derived from the default time zone. So if you have a cron 0 30 20 ? * * * and the default time zone is Europe/Berlin then no matter what the timezone attribute is the next fire time is an Instant derived from yyyy-MM-ddT20:30+02:00[Europe/Berlin].

I always get the same Instant for both cron schedules, so the compareTo always returns 0.

That is expected, although not very clear.

I will try to improve the wording of the javadoc and send a PR tomorrow.

alanmscott · 2024-07-08T14:42:48Z

OK, but that's not the correct value for NextFireTime then, surely?
If it's always returning the same Instant, regardless of the cron timezone, and the timezone DOES change the actual cron trigger time, then the getNextFireTime is not correct.

This example showing a trigger highlights it.
If the NextFireTime Instant was correct, then all 4 would be expected to trigger simultaneously, but instead, only the London one does, which is correct from a triggering point of view.

2024-07-08 15:33:30,858 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) =============================================================
2024-07-08 15:33:30,863 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) NextFireTimes:
2024-07-08 15:33:30,866 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob1(UTC     )=2024-07-08T14:35:00Z / 2024-07-08T14:35:00Z[UTC]
2024-07-08 15:33:30,866 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob2(London  )=2024-07-08T14:35:00Z / 2024-07-08T15:35:00+01:00[Europe/London]
2024-07-08 15:33:30,867 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob3(Berlin  )=2024-07-08T14:35:00Z / 2024-07-08T16:35:00+02:00[Europe/Berlin]
2024-07-08 15:33:30,867 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) TestJob4(Istanbul)=2024-07-08T14:35:00Z / 2024-07-08T17:35:00+03:00[Europe/Istanbul]
2024-07-08 15:33:30,867 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result1-2=0     <-- should be greater than 0
2024-07-08 15:33:30,867 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result2-3=0     <-- should be greater than 0
2024-07-08 15:33:30,868 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) compareTo result3-4=0     <-- should be greater than 0
2024-07-08 15:33:30,868 INFO  [org.exa.TestScheduler] (Quarkus Main Thread) =============================================================
...
2024-07-08 15:35:01,003 INFO  [org.exa.TestScheduler] (vert.x-worker-thread-1) Job2 has run

mkouba · 2024-07-08T15:52:43Z

This example showing a trigger highlights it.
If the NextFireTime Instant was correct, then all 4 would be expected to trigger simultaneously, but instead, only the London one does, which is correct from a triggering point of view.

That's a good point and indeed it does not make a lot of sense from this point of view.

So I tested the quarkus-quartz extension and it seems that getNextFireTime() returns the Instant derived from the zoned next fire time.

I will send a PR tomorrow. We should be consistent here.

alanmscott · 2024-07-08T15:55:32Z

Thanks @mkouba - really appreciate your persistence.

- fixes quarkusio#41717

- fixes quarkusio#41717 (cherry picked from commit 30af1a6)

alanmscott added the kind/bug Something isn't working label Jul 5, 2024

quarkus-bot bot added the area/scheduler label Jul 5, 2024

mkouba self-assigned this Jul 8, 2024

mkouba added a commit to mkouba/quarkus that referenced this issue Jul 9, 2024

Scheduler: fix Trigger#getNextFireTime() for cron-based jobs

243a4a6

- fixes quarkusio#41717

mkouba mentioned this issue Jul 9, 2024

Scheduler: fix Trigger#getNextFireTime() for cron-based jobs #41778

Merged

mkouba added a commit to mkouba/quarkus that referenced this issue Jul 9, 2024

Scheduler: fix Trigger#getNextFireTime() for cron-based jobs

30af1a6

- fixes quarkusio#41717

manovotn closed this as completed in #41778 Jul 10, 2024

quarkus-bot bot added this to the 3.13 - main milestone Jul 10, 2024

gsmet modified the milestones: 3.13 - main, 3.12.3 Jul 16, 2024

gsmet pushed a commit to gsmet/quarkus that referenced this issue Jul 16, 2024

Scheduler: fix Trigger#getNextFireTime() for cron-based jobs

3b3dc13

- fixes quarkusio#41717 (cherry picked from commit 30af1a6)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

alanmscott commented Jul 5, 2024

quarkus-bot bot commented Jul 5, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024 •

edited

Loading

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024 •

edited

Loading

alanmscott commented Jul 8, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024

quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

quarkus-scheduler trigger getNextFireTime does not consider cron timezone #41717

Comments

alanmscott commented Jul 5, 2024

Describe the bug

Expected behavior

Actual behavior

How to Reproduce?

Output of uname -a or ver

Output of java -version

Quarkus version or git rev

Build tool (ie. output of mvnw --version or gradlew --version)

Additional information

quarkus-bot bot commented Jul 5, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024 • edited Loading

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024 • edited Loading

alanmscott commented Jul 8, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024

mkouba commented Jul 8, 2024

alanmscott commented Jul 8, 2024

Output of `uname -a` or `ver`

Output of `java -version`

Build tool (ie. output of `mvnw --version` or `gradlew --version`)

alanmscott commented Jul 8, 2024 •

edited

Loading

alanmscott commented Jul 8, 2024 •

edited

Loading