Quarkus: Make scheduler ready for final

[dmlloyd]

The scheduler uses Quartz, which uses wrong cron syntax and is also far, far heavier than necessary to meet this use case. It was useful to prove the point but now that the PoC is out of the door and we're going towards final, we should use a standard/conventional cron syntax. @jamezp was looking into cron parsing as a part of his work on log rotation, so there's probably something reusable there.

Using Quartz for the implementation is very much overkill (it's a large library for what it does) and it's hard to customize its backend behavior. Really this should just translate into a job that consumes a global ScheduledExecutorService and uses it to submit each task as needed; it should be very lightweight.

[jamezp]

If something immediate is needed this seems to work well https://github.com/frode-carlsen/cron. It a single class and seems to work with what I've tested.

I've got something locally, but it's not quite ready yet. The main difference is I was trying to use something more generic than a ZonedDateTime, but that's likely not a big issue.

[stuartwdouglas]

I disagree that this is needed for the first public release, and I don't think 700K is a particularly large library (that is the same size as XNIO).

This is also not a simple item, the EJB implementation in WildFly has been a constant source of problems and hard to diagnose edge conditions that only happen in certain time zones/DST switches etc. Quartz is the de-facto standard for scheduling libraries for Java, it has been around for a long time and I really don't see how we can justify replacing it at this stage, especially given how resource constrained we are.

IMHO this should be closed, if quartz does end up causing issues down the road we can look at a custom implementation later, but this is definitely not as simple as 'just use a scheduled executor'.

[mkouba]

Hm, do we plan to support ManagedScheduledExecutorService?

In theory, the scheduler extension could just manage its own ScheduledExecutorService (JDK impl or something from jboss threads?), use it to schedule @Scheduled methods and also make it injectable into app beans.

The only thing I'm concerned about is that those quartz guys spend a lot of time making their impl robust enough, fixed a lot of bugs, etc. In other words, quartz is probably not perfect but it's very likely not that simple to implement it from scratch. For example you will need a dedicated scheduler thread to check triggers, handle misfire instructions, etc.

[dmlloyd]

I disagree that this is needed for the first public release, and I don't think 700K is a particularly large library (that is the same size as XNIO).

It is very large for what it does.

This is also not a simple item, the EJB implementation in WildFly has been a constant source of problems and hard to diagnose edge conditions

This is because that code is not very well designed, and written by someone who doesn't understand the APIs in question. Also iirc it does not use JSR 310 which makes it even harder.

Quartz is the de-facto standard for scheduling libraries for Java, it has been around for a long time and I really don't see how we can justify replacing it at this stage

We can't ship with wrong cron expression syntax. This simply needs to be fixed.

IMHO this should be closed, if quartz does end up causing issues down the road we can look at a custom implementation later, but this is definitely not as simple as 'just use a scheduled executor'.

I'll leave it open; if nobody else can manage it, then I'll do it.

[dmlloyd]

Hm, do we plan to support ManagedScheduledExecutorService?

We could do so. We support plain ExecutorService now and could extend it to include a scheduled executor as well; adding the managed part is something of an ongoing discussion with @FroMage.

The only thing I'm concerned about is that those quartz guys spend a lot of time making their impl robust enough, fixed a lot of bugs, etc.

It's true, but it's also true that Quartz does many other things that we don't want or need. It's not some black box of mysteries; it's a well understood problem with many existent implementations other than Quartz that we could be using.

In other words, quartz is probably not perfect but it's very likely not that simple to implement it from scratch. For example you will need a dedicated scheduler thread to check triggers, handle misfire instructions, etc.

By far the hardest part of this is understanding what the behavior should be; using Quartz doesn't give us a pass on this, it just kicks the ball down the road a bit.

[mkouba]

So I looked at https://en.wikipedia.org/wiki/Cron and it seems the only difference is "day of the week" - (0 - 6) Sun - Sat in linux Cron VS (1 - 7) Sun - Sat in Quartz (both are BTW wrong because everyone knows that week starts with Monday ;-) and standard Cron is also missing seconds which is imho pretty useful.

[dmlloyd]

A feature comparison between the following is probably a good idea:

• Standard crontab
• Spring scheduler
• Quartz
• node.js cron-parser
• http://cron-parser.com/
• https://github.com/grahamar/cron-parser
• etc. etc.

[jamezp]

I do have an expression parser mostly done now. One of the questions I did have was the what number represents Sunday. From the reading I did it looks like most implementations support 0 and 7 for Sunday. Quartz seemed to be the only outlier. So if we go with Quartz and then change we'd have to support the 1 is Sunday. If we use our own we can make the determination ourselves :)

If it was up to me, I'd probably go with:

Number | Day
------------ | -------------
0 | Sunday
1 | Monday
2 | Tuesday
3 | Wednesday
4 | Thursday
5 | Friday
6 | Saturday
7 | Sunday

I don't know this for sure, but I think Spring uses Quartz so it could make sense to use the Quartz syntax. However Quartz also supports a year which seems odd to me. I can't think of a useful case for that.

[mkouba]

A feature comparison between the following is probably a good idea: ...

Yes, but that would be a bit time consuming. I think that our feature set is ok for the first public release.

• http://cron-parser.com/

This one is interesting, it can migrate cron expressions. @jamezp Did you get a chance to look at this library too? I think we could support multiple syntaxes and let the user choose by config?

[dmlloyd]

This one looks very good: https://github.com/jmrozanec/cron-utils

[mkouba]

This one looks very good: https://github.com/jmrozanec/cron-utils

It's the same lib as the http://cron-parser.com/ ;-)

[dmlloyd]

Great :)

[dmlloyd]

Note that it comes with utilities to calculate the next execution

[dmlloyd]

And it's only 150K

[jamezp]

It definitely needs some clean up, but this is what I ended up with. https://gist.github.com/jamezp/bb0288accf092c71e214cb063e981e90. It's currently using the Quartz days for number days.

[dmlloyd]

@jamezp the point is that we don't want to use the Quartz style, we want the standard style for days.

[jamezp]

@dmlloyd Okay that would be a very easy fix. I mainly did it that way so when I compared my output with theirs it matched :) Anyway if we want to use it I can fix it, if not I won't make it a priority.

[mkouba]

@jamezp the point is that we don't want to use the Quartz style, we want the standard style for days.

I'd say we want to have this configurable.

For the record - Micronaut does not care about standards either, they use 1-7 or MON-SUN "day of week". See also https://github.com/micronaut-projects/micronaut-core/blob/master/runtime/src/main/java/io/micronaut/scheduling/cron/CronExpression.java#L90.

[jamezp]

@mkouba Yeah. I noticed Micronaut just uses https://github.com/frode-carlsen/cron which mostly works, but doesn't work for things like LW or L-2. It does seem it could be configurable though without too much issue. That was one idea I had for the one I was working on.

[Sanne]

@mkouba @dmlloyd I take it this could be closed?

[dmlloyd]

My opinion hasn't changed, only my authority :) If you guys still disagree with me, go ahead and close.

[Sanne]

My opinion hasn't changed, only my authority :) If you guys still disagree with me, go ahead and close.

TBH I had no voice in this matter, staying out of it as it's far from my area of experience :)

Just reminding that whatever you all decide, the title here "Make scheduler ready for final" implies something is being decided soon... either by action or inaction, so we might as well close this unless someone manages to squeeze in a happy ending.

So what's the status, are we based on Quartz and we're using its unusual non-cron syntax? The concerning aspect of that is that since we'll have to stick with the syntax it will be harder to get rid of it if we ever want.

Personally I think it has a good track record so I'm not too concerned about needing to get rid of it, but I hope we'll find ways to make it at least light to boot. Hopefully they'll be open to "modernizing" patches.

[stuartwdouglas]

Would it be possible just to translate the standard style into Quartz style? Maybe with an option for the user to pick which one the prefer?

Based on our experience with EAP EJB timers I am very much against us writing our own scheduler.

I guess in an ideal world I would prefer:

• A generic scheduler extension that uses standard cron syntax, and uses quartz on the backend by translating the cron statements, to give us flexibility in future
• A Quartz specific extension, that provides access to all the advanced Quartz features like clustered timers, and lets you use Quartz syntax if you prefer.

[dmlloyd]

I never advocated writing our own scheduler. IIRC the cron-utils library allows parsing and deparsing of cron strings in a variety of formats, plus all the common extensions. It handles all the ugly scheduling math that we continually got wrong in EJB, and it does so using modern JSR-310 APIs. It really only needs the addition of a scheduled thread pool and some glue to make it all work, and it's smaller than Quartz.

I don't think that using Quartz excuses us from having to think about questions like job overlap policies (which people have already asked about), DST handling policy, etc. There is no single right answer for these things which is why they should be policy which is configurable by the user.

I'd agree that having a Quartz extension for Quartz features is a good idea though.

[mkouba]

My opinion hasn't changed, only my authority :)

@dmlloyd Your authority hasn't changed at all! ;-)

So I believe that...

from user POV we should provide the following:

• quarkus-scheduler - for simple use cases; @Scheduled API + config property to switch the syntax (CRON, quartz, etc.), users don't care about the implementation
• quarkus-quartz for power users relying on some quartz-specific APIs/features; builds on top of quarkus-scheduler API

from impl POV we should:

• quarkus-scheduler - use cron-utils + ScheduledExecutorService if used without quarkus-quartz
• quarkus-quartz - use cron-utils + quartz; depends on quarkus-scheduler, provides quartz-specific config and features

I'd like to spend some more time on this during the next few weeks. I will create some new issues and close this one afterwards.

[machi1990]

@mkouba I'll be interested to contribute in fixing some of these issues. Tag me along :-)

[mkouba]

@machi1990 I will ;-).

[mkouba]

Here we go: https://github.com/quarkusio/quarkus/pull/5481

[mkouba]

Closig this issue. Feel free to re-open if needed.