Skip to content

Add Quartz Scheduler support #4299

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

Closed

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Oct 26, 2015

Spring Framework has had Quartz Scheduler support for a very long time - it would be nice to make use of it in Spring Boot.

This PR adds:

  • auto-configuration for SchedulerFactoryBean
  • spring-boot-starter-quartz
  • spring-boot-sample-quartz
  • relevant documentation updates

Please review, comments are welcome.
If this PR is accepted I'd also like to add Actuator support in a separate PR.

I've signed the CLA.

@snicoll
Copy link
Member

snicoll commented Oct 26, 2015

Thanks for the PR! it looks quite complete overall.

See also #3919; we could have a scheduling and async processing theme for the next version maybe...

@vpavic
Copy link
Contributor Author

vpavic commented Oct 26, 2015

Scheduling/async theme sounds good.

I was hoping this PR could sneak into the 1.3.0.RELEASE, but looking at the release schedule I guess it's too late for that now.

@snicoll
Copy link
Member

snicoll commented Oct 26, 2015

Yes, we are post RC1 now.

@philwebb philwebb added this to the 1.4.0 milestone Nov 6, 2015
@philwebb
Copy link
Member

philwebb commented Jan 7, 2016

Thanks for the contribution, but since there's quite a lot of code in the pull-request we'd like to wait for a while and see how much demand there is for quartz. We'll mark this one as waiting-for-votes for now and monitor for +1's.

@philwebb philwebb removed this from the 1.4.0 milestone Jan 7, 2016
@gnuphie
Copy link

gnuphie commented Jan 7, 2016

+1 we use quartz all the time

@iturcino
Copy link

iturcino commented Jan 7, 2016

+1 , we also need quartz support

@beegor
Copy link

beegor commented Jan 8, 2016

+1, this would be very useful

@sschedl
Copy link

sschedl commented Jan 8, 2016

+1, would be very useful

@vpugar
Copy link

vpugar commented Jan 10, 2016

+1 would be very useful

@thackel
Copy link

thackel commented Jan 10, 2016

+1, I don't know any other free solution with the feature set and maturity like quartz.

@davidkiss
Copy link

+1, adding first-class support to quartz would be very helpful when working with this framework

@vpavic
Copy link
Contributor Author

vpavic commented Jan 19, 2016

Please reconsider adding this to one of 1.4.0 milestone releases.
User interest obviously exists, and Spring Boot doesn't provide much auto-configuration support in the area of advanced job scheduling.

Regarding the lot of code argument, IMO implementation is pretty straightforward and it is based on Quartz support in the core Spring Framework, test coverage is good, sample app and documentation are also in the mix so it shouldn't be a risky addition.

If this is added to the upcoming milestones, I can rebase onto current master and update the PR to fix the conflicts.

@snicoll
Copy link
Member

snicoll commented Jan 20, 2016

Regarding the lot of code argument, IMO implementation is pretty straightforward and it is based on Quartz support in the core Spring Framework

Well IMO, that's exactly the problem. It is a one-to-one mapping for what the FactoryBean does and this is not how we're integrating things in Spring Boot. We usually have an opinion with a sensible default and some easy way to fine tune well-known use cases. For instance, the Spring Batch integration looks for Job beans and execute them on startup by default.

I am not denying that having some kind of scheduling solution in Spring Boot is interesting but I'd rather focus on the 80% use cases first. And this PR doesn't do that.

Maybe those who voted could share their most frequent use case and we could try to derive a proper default behaviour from that?

@vpavic
Copy link
Contributor Author

vpavic commented Jan 20, 2016

It is a one-to-one mapping for what the FactoryBean does and this is not how we're integrating things in Spring Boot. We usually have an opinion with a sensible default and some easy way to fine tune well-known use cases. For instance, the Spring Batch integration looks for Job beans and execute them on startup by default.

@snicoll I'm confused by your comment. Auto-configuration for SchedulerFactoryBean implemented in this PR offers automatic registration of any JobDetail, Calendar, Trigger and various Quartz listener Spring beans with SchedulerFactoryBean. Normally you need to register those manually with SchedulerFactoryBean. Also if there is a DataSource available persistent JobStore will be enabled.

@snicoll
Copy link
Member

snicoll commented Jan 20, 2016

I guess what I tried to say is that it maps all the features of the FactoryBean and maybe it shouldn't do that for a first version (i.e. we should concentrate on the regular use case and let users configure things manually if they use something a bit more specific). We usually fine tune in increment with user's feedback.

@vpavic
Copy link
Contributor Author

vpavic commented Jan 20, 2016

OK, I'm open to suggestions.

Persistent JobStore as well as JobDetails, Calendars and Triggers are the bare minimum one would expect. Listeners and JobFactory might not be used that often and one could live without them being auto-configured I guess.

As far as configuration properties are concerned, we regulary use taskExecutor to specify executor, applicationContextSchedulerContextKey to get hold of Spring beans within jobs, startupDelay and waitForJobsToCompleteOnShutdown to fine tune startup/shutdown behavior and additional quartzProperties (to configure clustering and fine-tune JobStore configuration).

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jan 20, 2016
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Feb 10, 2016
@vpavic vpavic force-pushed the quartz-scheduler-support branch from 2416da6 to f6e0f2c Compare February 27, 2016 09:54
@vpavic
Copy link
Contributor Author

vpavic commented Feb 27, 2016

I've updated the PR with following changes:

  • amount of configuration properties significantly reduced
  • auto configuration now picks up only JobDetail, Trigger and Calendar beans
  • introduced QuartzDatabaseInitializer with sensible defaults (inspired by BatchDatabaseInitializer)
  • introducted QuartzSchedulerFactoryBeanCustomizer
  • amount of code reduced by approximately 20% from the original PR
  • rebased on to the current master

@snicoll, @philwebb, please review the changes, I'm looking forward to your feedback.

@vpavic
Copy link
Contributor Author

vpavic commented Feb 27, 2016

Also this PR is another use case which would benefit from #5082.

@cemo
Copy link
Contributor

cemo commented Feb 27, 2016

👍

@jgoldhammer
Copy link

+1

@vpavic vpavic force-pushed the quartz-scheduler-support branch from 82ab523 to 10cbae2 Compare April 20, 2016 08:59
@vpavic
Copy link
Contributor Author

vpavic commented Apr 21, 2016

Any feedback on the latest changes?
Considering there are many votes on this PR, is there anything else that can be done to have this merged?

@sergeysimonov
Copy link

+1

1 similar comment
@akkamburov
Copy link

+1

@philwebb philwebb added this to the 2.0.0.M2 milestone Jan 11, 2017
@laguiar
Copy link

laguiar commented Jan 13, 2017

+1

quartz + (proper) cluster exec it's crucial item for my microservice architecture.

@vpavic vpavic force-pushed the quartz-scheduler-support branch from 8292a96 to 1649d5b Compare January 18, 2017 17:29
@CraKeyBoy
Copy link

+1

@philwebb
Copy link
Member

philwebb commented Mar 7, 2017

No more +1's please :) We're going to try and get this into Boot 2.0

@vpavic
Copy link
Contributor Author

vpavic commented Mar 30, 2017

Quartz Scheduler 2.3.0 should be released in mid-April.

This release will include Quartz database schema scripts as a part of quartz-core artifact (see quartz-scheduler/quartz#46) which will provide room for improvement of Quartz support proposed by this PR. I'll update the PR shortly after 2.3.0 has been released.

@ptahchiev
Copy link
Contributor

This project: https://github.com/FlavioF/quartz-scheduler-hazelcast-jobstore
provides implementation of a Quartz Scheduler Job Store using Hazelcast distributed Maps and Sets. Might be interesting to autoconfigure it if hazelcast is found in the classpath.

@vpavic vpavic force-pushed the quartz-scheduler-support branch from 1649d5b to 4666702 Compare April 20, 2017 18:37
@vpavic
Copy link
Contributor Author

vpavic commented Apr 20, 2017

PR updated to align with newly released Quartz 2.3.0.

@vpavic vpavic force-pushed the quartz-scheduler-support branch 2 times, most recently from f22054e to 940eacd Compare April 25, 2017 22:48
@poznachowski
Copy link

As stated here: https://github.com/spring-projects/spring-boot/pull/4299/files?diff=unified#r118478148 current solution disallow using constructor injection (and possibly more things - aspects etc.) in Quartz job.

@snicoll snicoll self-assigned this May 29, 2017
@vpavic vpavic force-pushed the quartz-scheduler-support branch from 940eacd to 9d04ce2 Compare May 29, 2017 20:31
snicoll pushed a commit that referenced this pull request May 30, 2017
@snicoll snicoll closed this in 59a15b2 May 30, 2017
snicoll added a commit that referenced this pull request May 30, 2017
* pr/4299:
  Polish "Add Quartz Scheduler support"
  Add Quartz Scheduler support
@vpavic vpavic deleted the quartz-scheduler-support branch May 30, 2017 19:25
wilkinsona added a commit that referenced this pull request Jun 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet