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

Incorrect retrieval of job parameters in SimpleJobExplorer#getJobExecutions #4246

Closed
hannosgit opened this issue Nov 25, 2022 · 5 comments
Closed
Labels
for: backport-to-4.3.x Issues that will be back-ported to the 4.3.x line has: minimal-example Bug reports that provide a minimal complete reproducible example in: core type: bug
Milestone

Comments

@hannosgit
Copy link

Bug description
The following method List<JobExecution> getJobExecutions(JobInstance jobInstance) in the SimpleJobExplorer class returns the wrong job parameters when a job instance is started multiple times with different job parameters (identifying job parameters are the same but the non-identifying parameters were changed)

Environment
Versions:

  • Spring Batch: 4.3.7
  • Java: 17.0.3
  • Database: Postgres

Steps to reproduce

  • Define a job with identifying parameters and non-identifying parameters
  • Launch a job and stop it
  • Restart the job with the same identifying parameters but changed non-identifying parameters
  • Fetch the job executions with List<JobExecution> getJobExecutions(JobInstance jobInstance)
  • Returned job executions will have the same job parameters even though the second execution has changed non-identifying parameters

Expected behavior
Either document this behavior or change the implementation to fetch the job parameters of each job execution.

Implementation hint

I think this behavior is because the JdbcJobExecutionDao only fetches the job parameters once for a job instance and then reuses them for each job execution.
JdbcJobExecutionDao

public List<JobExecution> findJobExecutions(final JobInstance job) {

Assert.notNull(job, "Job cannot be null.");
Assert.notNull(job.getId(), "Job Id cannot be null.");

return getJdbcTemplate().query(getQuery(FIND_JOB_EXECUTIONS), new JobExecutionRowMapper(job), job.getId());
}

A fixed version would fetch the job parameters for each execution because each job execution has its own job parameters, see batch_job_execution_params table.
This would impact the performance of the method, because more data would need to be fetched.

@hannosgit hannosgit added status: waiting-for-triage Issues that we did not analyse yet type: bug labels Nov 25, 2022
@fmbenhassine fmbenhassine added in: core and removed status: waiting-for-triage Issues that we did not analyse yet labels Jan 16, 2023
@fmbenhassine
Copy link
Contributor

Thank you for opening this issue. We do not exclude that this could be a bug in Spring Batch. However, we would like to validate that with a minimal complete verifiable example.

Could you please take some time to create a minimal example that reproduces the problem? To help you in reporting your issue, we have prepared a project template that you can use as a starting point. Please check the Issue Reporting Guidelines for more details about this.

Thank you for your collaboration.

@fmbenhassine fmbenhassine added status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter status: need-help-to-reproduce Issues that we welcome the community to help us reproduce labels Apr 3, 2023
@hannosgit
Copy link
Author

Hi,
you can find a minimal complete verifiable example here:
https://github.com/hannosgit/spring-batch-issue-4246
or
you can download the attached ZIP file.
spring-batch-issue-4246.zip

Feel free to reach out to me, if you need more information!
Thank you!

@fmbenhassine fmbenhassine added has: minimal-example Bug reports that provide a minimal complete reproducible example and removed status: waiting-for-reporter Issues for which we are waiting for feedback from the reporter status: need-help-to-reproduce Issues that we welcome the community to help us reproduce labels Apr 4, 2023
@fmbenhassine fmbenhassine added this to the 5.0.2 milestone Apr 4, 2023
@baezzys
Copy link
Contributor

baezzys commented May 2, 2023

Hi @fmbenhassine, I think I've found the cause. Is it okay if I try to contribute?

@fmbenhassine
Copy link
Contributor

@CNJingo Sure! Thank you for your offer to help.

Any issue without an already open PR or tagged with status: for-internal-team is open for contribution. So you are welcome to contribute if you want.

FYI, we are planning to include this in v5.0.2 coming out on May 17th. Let me know if you need help on the fix.

@baezzys
Copy link
Contributor

baezzys commented May 12, 2023

@fmbenhassine Thank you so much for explaining the guidelines so well.

In fact, I've already created a PR. I would really appreciate it if you could review the PR.

fmbenhassine added a commit that referenced this issue May 16, 2023
Add integration test

Related to #4246
fmbenhassine pushed a commit that referenced this issue May 16, 2023
Before this commit, SimpleJobExplorer#getJobExecutions returned
job executions with wrong job parameters, ie a job execution could
have the parameter of another execution.

This commit fixes the implementation so that each returned job
execution has its own parameters.

Resolves #4246
fmbenhassine added a commit that referenced this issue May 16, 2023
Add integration test

Related to #4246
@fmbenhassine fmbenhassine changed the title SimpleJobExplorer getJobExecutions returns job parameters of first job execution Incorrect retrieval of job parameters in SimpleJobExplorer#getJobExecutions May 16, 2023
@fmbenhassine fmbenhassine added the for: backport-to-4.3.x Issues that will be back-ported to the 4.3.x line label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: backport-to-4.3.x Issues that will be back-ported to the 4.3.x line has: minimal-example Bug reports that provide a minimal complete reproducible example in: core type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants