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

Allow to set DisallowConcurrentExecution in JobDetail instead of hard-code in class #780

Closed
flozano opened this issue Mar 7, 2022 · 11 comments
Labels
is:feature New feature stale Inactive items that will be automatically closed if not resurrected

Comments

@flozano
Copy link

flozano commented Mar 7, 2022

Right now the only way to set DisallowConcurrentExecution is to set it at job-class level. JobDetailImpl does this when checking for this setting:

    /**
     * @return whether the associated Job class carries the {@link DisallowConcurrentExecution} annotation.
     */
    public boolean isConcurrentExectionDisallowed() {
        
        return ClassUtils.isAnnotationPresent(jobClass, DisallowConcurrentExecution.class);
    }

This is not very flexible. It would be great if it was possible to set it in the JobBuilder, as other parameters.

I'm now evaluating the creation of a JobDetail wrapper implementation just to allow overriding it... it feels this should be part of the JobBuilder API also, and the class annotation just a default in case it's not set there.

So, this is a feature request and a question.

  • A feature request to consider implementing it if possible
  • A question to people more familiar with quartz code-base than me: Do you think that wrapping JobDetail to override this value will explode in my face or it's something that should work in principle? I see this value is taken to the job-store to be stored, but i'm not sure if after retrieval the value is ignored and still used the annotation presence/absence.
@flozano
Copy link
Author

flozano commented Mar 8, 2022

adding to my question, in StdJDBCConstants this is the only place where COL_IS_NONCONCURRENT is referenced, but SELECT_JOB_NONCONCURRENT seems never used:

    String SELECT_JOB_NONCONCURRENT = "SELECT "
            + COL_IS_NONCONCURRENT + " FROM " + TABLE_PREFIX_SUBST
            + TABLE_JOB_DETAILS + " WHERE " 
            + COL_SCHEDULER_NAME + " = " + SCHED_NAME_SUBST 
            + " AND " + COL_JOB_NAME
            + " = ? AND " + COL_JOB_GROUP + " = ?";

because this seems never called:

~/Projects/java/quartzmasterag isJobNonConcurrent
quartz-core/src/main/java/org/quartz/impl/jdbcjobstore/DriverDelegate.java
291:    boolean isJobNonConcurrent(Connection conn, JobKey jobKey) throws SQLException;

quartz-core/src/main/java/org/quartz/impl/jdbcjobstore/StdJDBCDelegate.java
741:    public boolean isJobNonConcurrent(Connection conn, JobKey jobKey) throws SQLException {

@stale
Copy link

stale bot commented Jun 6, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Inactive items that will be automatically closed if not resurrected label Jun 6, 2022
@flozano
Copy link
Author

flozano commented Jun 6, 2022

Yes, it is still relevant. The ability to set this at the job level in JobBuilder instead of class level would add much needed flexibility.

@stale stale bot removed the stale Inactive items that will be automatically closed if not resurrected label Jun 6, 2022
@codeaches
Copy link

Just curious to know if there will be any release in the near future? The last one was [Quartz 2.3.2] few years ago.

@stale
Copy link

stale bot commented Sep 7, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Inactive items that will be automatically closed if not resurrected label Sep 7, 2022
@flozano
Copy link
Author

flozano commented Sep 7, 2022

This is still relevant to me, the current approach is not flexible and enforces class-level instead of job-level, there are leftovers from allowing job-level (row in database exists but it's never used).

@stale stale bot removed the stale Inactive items that will be automatically closed if not resurrected label Sep 7, 2022
@stale
Copy link

stale bot commented Dec 8, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Inactive items that will be automatically closed if not resurrected label Dec 8, 2022
@stale stale bot closed this as completed Dec 18, 2022
@flozano
Copy link
Author

flozano commented Dec 18, 2022

this is far from complete

@flozano
Copy link
Author

flozano commented Dec 18, 2022

Could this issue be reopened please?

@jhouserizer
Copy link
Contributor

This was purposely designed this way. Jobs that allow/disallow concurrent execution typically have some implementation that is very specific to the reason why it cannot be ran concurrently, and therefore the annotation was "designed" to provide a strong indicator/self-documenting in the code that the concerns need to be watched out for.

I certainly understand the counter-point, but this would be a notable design change to precedent that was set well over a decade ago (and was actually a switch away-from not using an annotation).

@flozano
Copy link
Author

flozano commented Apr 5, 2023

Sometimes the job class is not what defines the concurrency or non-concurrency. Enforcing it at that level all the time is not a great option from my point of view, it looks natural to handle by allowing to persist/override JobDetail; even more when taking into account that there is a database column for it.

I understand the state of the project and past decisions weight against this change anyway, thanks for considering it!

@jhouserizer jhouserizer added the is:feature New feature label Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:feature New feature stale Inactive items that will be automatically closed if not resurrected
Projects
None yet
Development

No branches or pull requests

3 participants