Skip to content

Add Quartz actuator endpoint #10364

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
wants to merge 1 commit into from

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Sep 20, 2017

This PR adds Actuator endpoint that lists Quartz Scheduler jobs and allows retrieval of their details.

The updated spring-boot-sample-quartz sample can be used to try out the new endpoint.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 20, 2017
@snicoll
Copy link
Member

snicoll commented Sep 20, 2017

See also #9623

@vpavic vpavic force-pushed the quartz-endpoint branch 2 times, most recently from 647fb5c to e99ff64 Compare October 19, 2017 10:55
@wilkinsona
Copy link
Member

I suspect we'll want a single endpoint that provides information about quartz scheduling and "normal" scheduling. We'll consider this one once the Framework changes blocking #9623 are available.

@wilkinsona wilkinsona added priority: normal status: on-hold We can't start working on this issue yet type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 27, 2017
@vpavic
Copy link
Contributor Author

vpavic commented Oct 27, 2017

Thanks for the feedback.

How would the actual response payload look in the case these two are combined? Obviously the model of the scheduling related entities differs quite a bit between the two so I doubt it will be possible to fit them into the same representation. Something like this comes in mind:

{
    "spring": [
        // ...
    ],
    "quartz": [
        // ...
    ]
}

However I'm not sure I like that since at some point we could support operations other than reads. There's also the question of disabling the information from specific scheduler - for example user would like to have Quartz scheduling report but not Spring scheduling. All of this adds complexity to endpoint if it's a single one.

OTOH this could be viewed from the perspective of Flyway/Liquibase endpoints - both are database migration tools yet there are separate endpoints.

@vpavic
Copy link
Contributor Author

vpavic commented Nov 16, 2017

With #8831 resolved, are there any ideas with regard to the direction this PR will be taking?

@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review and removed status: on-hold We can't start working on this issue yet labels Nov 16, 2017
@wilkinsona
Copy link
Member

wilkinsona commented Nov 17, 2017

@vpavic Having implemented the new scheduled tasks endpoint and what you're proposing here, I'm now in agreement with keeping them separate.

I'd quite like to get this into 2.0 if we can. One thing that would help with that is adding some documentation for the endpoint along the lines of what's just be done in 4de208b. Do you have time to take a look at that?

@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Nov 17, 2017
@vpavic
Copy link
Contributor Author

vpavic commented Nov 17, 2017

Do you have time to take a look at that?

Yes, I should be able to take care of that over the next few days.

@vpavic
Copy link
Contributor Author

vpavic commented Nov 19, 2017

@wilkinsona I've added the endpoint docs.

@wilkinsona wilkinsona self-assigned this Nov 22, 2017
@wilkinsona wilkinsona added this to the 2.0.0.RC1 milestone Nov 22, 2017
@wilkinsona
Copy link
Member

@vpavic Thanks for the docs.

What was the motivation for splitting the endpoint into two operations? Perhaps retrieving details of all the scheduled jobs is prohibitively expensive when using Quartz? I'm leaning towards reworking this to provide a single operation that shows details of all the scheduled jobs. I think it would make the endpoint easier to use and would also bring it into line with the schedulers endpoint.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jan 9, 2018
@wilkinsona
Copy link
Member

Having looked at this some more, I'm wondering if the endpoint should present jobs and triggers as two separate, top-level concerns. That would open up the possibility of having separate operations for manipulating jobs and triggers in the future. In web terms, something like the following:

  • GET /actuator/quartz/jobs lists all jobs known to the scheduler
  • GET /actuator/quartz/triggers lists all groups known to the scheduler

Then, in the future, we could introduce the following:

  • GET /actuator/quartz/jobs/{group}/{name} details of a specific job
  • GET /actuator/quartz/triggers/{group}/{name} details of a specific trigger
  • POST /actuator/quartz/jobs/{group}/{name} pause/resume a job
  • POST /actuator/quartz/triggers/{group}/{name} pause/resume a trigger

Beyond that, I could also see operations for pausing or resuming whole groups:

  • POST /actuator/quartz/jobs/{group} pause/resume a group of jobs
  • POST /actuator/quartz/triggers/{group} pause/resume a group of triggers

@vpavic
Copy link
Contributor Author

vpavic commented Jan 15, 2018

Thanks for the feedback @wilkinsona. The idea of splitting the endpoint into two operations originates from the chat @snicoll and I had shortly after #4299 was merged.

Personally I'm still in favor of such arrangement as listing all the jobs can indeed be quite a costly operation. For each job, we need to go to Scheduler API to retrieve job details and triggers which are basically JobStore operations and in a typical Quartz use case, with database store, this can potentially results in quite big amount of queries.

I'd also avoid looking at Quartz endpoint through comparison with Schedulers endpoint as the two are quite different in nature - the latter usually contains a smaller set of jobs that are static and not backed by a external store, while the former is dynamic and can contain hundreds or even thousands of jobs at a given moment.

@wilkinsona
Copy link
Member

Thanks, @vpavic.

I’m rather sceptical that the cost would be prohibitive but do realise it could be the case. IMO, as things stand, the changes proposed here are compromising the usability of the endpoint for a theoretical performance problem. Until the performance cost has been quantified and found to be too large, I don’t think that compromise is justified. Even if the cost is relatively high, endpoint response caching will mitigate it to some degree.

Unfortunately, time is running out for RC1 and I don’t have time to gather performance data that would inform the design of the endpoint. I’m going to drop this from RC1 as I’d rather this landed in 2.1 having been carefully considered than we rush it into 2.0 and regret it.

@wilkinsona wilkinsona removed this from the 2.0.0.RC1 milestone Jan 15, 2018
@wilkinsona
Copy link
Member

To make progress, this PR requires the following:

  • Performance data proving or disproving the need for the proposed split.
  • If the split is needed, a proposal for how the possible future operations discussed above would be modelled.

@philwebb philwebb added the status: pending-design-work Needs design work before any code can be developed label May 31, 2019
@philwebb philwebb added this to the General Backlog milestone May 31, 2019
return new QuartzJob(jobDetail, triggers);
}
catch (SchedulerException e) {
return null;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I new here. So this is just a question why we didn't log what is the error?

@wilkinsona
Copy link
Member

#17836 shows that the performance of the preferred structure isn't a problem. Thanks for your efforts here, @vpavic, but I think it makes sense to close this one at this point in favour of #17836.

@wilkinsona wilkinsona closed this Mar 20, 2020
@wilkinsona wilkinsona removed this from the General Backlog milestone Mar 20, 2020
@wilkinsona wilkinsona added status: superseded An issue that has been superseded by another and removed status: pending-design-work Needs design work before any code can be developed labels Mar 20, 2020
@snicoll snicoll reopened this Mar 29, 2021
@snicoll snicoll removed the status: superseded An issue that has been superseded by another label Mar 29, 2021
@snicoll snicoll self-assigned this Mar 29, 2021
@snicoll snicoll added this to the 2.5.x milestone Mar 29, 2021
@snicoll
Copy link
Member

snicoll commented Mar 29, 2021

I've rebased this change on top of master (quite a ride!) and I am now reviewing it. After having spent quite some time on the alternative proposal, I think this change is simpler and more aligned with what we'd like for a first version. I am sure we'll refine it based on the community feedback in a future release.

snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Mar 29, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Mar 29, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Mar 30, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Mar 30, 2021
snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Mar 31, 2021
snicoll added a commit to snicoll/spring-boot that referenced this pull request Mar 31, 2021
snicoll pushed a commit that referenced this pull request Apr 6, 2021
snicoll added a commit that referenced this pull request Apr 6, 2021
This commit reworks the initial proposal so that jobs and triggers are
treated as first class concepts.

`/actuator/quartz` now returns the group names for jobs and triggers.

`actuator/quartz/jobs` returns the job names, keyed by the available
group names, while `/actuator/quartz/triggers` does the same for
triggers.

`/actuator/jobs/{groupName}` provides an overview of a job group. It
provides a map of job names with the class name of the job.
implementation

`/actuator/triggers/{groupName}` provides an overview of a trigger
group. There are five supported trigger implementations: cron, simple,
daily time interval, calendar interval, and custom for any other
implementation. Given that each implementation has specific settings,
triggers are split in five objects.

`/actuator/jobs/{groupName}/{jobName}` provides the full details of a
particular job. This includes a sanitized data map and a list of
triggers ordered by next fire time.

`/actuator/triggers/{groupName}/{triggerName}` provides the full details
of a particular trigger. This includes the state, its type, and a
dedicate object containing implementation-specific settings.

See gh-10364
@snicoll snicoll closed this in a07c8e6 Apr 6, 2021
@snicoll snicoll modified the milestones: 2.5.x, 2.5.0-RC1 Apr 6, 2021
wilkinsona added a commit that referenced this pull request Apr 6, 2021
@vpavic vpavic deleted the quartz-endpoint branch April 13, 2021 19:49
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.

7 participants