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

JdbcRepository doesn't override getJob, jobExists, getJobNames #545

Open
Xiphoseer opened this issue May 27, 2024 · 5 comments
Open

JdbcRepository doesn't override getJob, jobExists, getJobNames #545

Xiphoseer opened this issue May 27, 2024 · 5 comments

Comments

@Xiphoseer
Copy link

Currently, getJob, jobExists and getJobNames don't override the use of the ConcurrentMap jobs in AbstractRepository. That means that in a multi-instance configuration, where multiple applications share the same database, only those that have at least one job started locally will be able to see that jobName.

From my POV:

  • getJobNames should do a SELECT DISTINCT on the job_instances table
  • jobExists should do a SELECT ... WHERE JOBNAME = ?
  • getJob should ideally trigger ArchiveXmlLoader.loadJobXml with the JobXmlResolver if the jobName is unknown but I guess that isn't available by default.
@Xiphoseer
Copy link
Author

FWIW: my current workaround is to load all jobs from META-INF/batch up front via ArchiveXmlLoader.loadJobXml and to call addJob on them

@liweinan
Copy link
Contributor

@Xiphoseer Thanks for raising this! I'll arrange my time to check this.

@Xiphoseer
Copy link
Author

I guess this issue is effectively a duplicate of JBERET-66

@Xiphoseer
Copy link
Author

I want to elaborate on the motivation a bit:

Ultimately, the case where I encountered this was with JobOperator#getJobNames as defined by JSR-352 (and by extension Jakarta Batch 2.0). In our case, this method is used by existing code to trigger multiple calls to JobOperator#getJobInstances to collect all batch jobs in the system. The spec doesn't really say anything about that method, other than the following description in the javadoc:

Returns a set of all job names known to the batch runtime.

What happens though is that JBeret proxies this method to org.jberet.repository.JobRepository#getJobNames in

public Set<String> getJobNames() throws JobSecurityException {
return getJobRepository().getJobNames();
}

which ends up being implemented only by

@Override
public Set<String> getJobNames() {
final Set<String> jobNames = new HashSet<String>();
for (final ApplicationAndJobName e : jobs.keySet()) {
jobNames.add(e.jobName);
}
return jobNames;
}

That implementation is somewhat reasonable for a generic JobRepository, as it matches jobExists and getJob on the same interface. But it's much less obvious from the point of view of the JobOperator. There we'd like behavior that matches the parameters that work for getJobInstances.

@liweinan
Copy link
Contributor

I plan to do a major beta release of JBeret today or tomorrow to include several major jakarta API upgrades. After the release is done I'll go on working on this(and maybe include the work into Final release).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants