jberet / jsr352

Implementation of Jakarta Batch Specification and API
Eclipse Public License 2.0
126 stars 76 forks source link

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

Open Xiphoseer opened 4 months ago

Xiphoseer commented 4 months ago

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:

Xiphoseer commented 4 months ago

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 commented 4 months ago

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

Xiphoseer commented 4 months ago

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

Xiphoseer commented 4 months ago

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 https://github.com/jberet/jsr352/blob/c7daf8126599e7c49aaf7db2a07c6e81eec0a6c3/jberet-core/src/main/java/org/jberet/operations/AbstractJobOperator.java#L171-L173

which ends up being implemented only by https://github.com/jberet/jsr352/blob/c7daf8126599e7c49aaf7db2a07c6e81eec0a6c3/jberet-core/src/main/java/org/jberet/repository/AbstractRepository.java#L74-L81

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 commented 4 months ago

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).