Skip to content

Fix parameters handling on a restart of a Spring Batch job #14933

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
Closed

Fix parameters handling on a restart of a Spring Batch job #14933

wants to merge 1 commit into from

Conversation

fmbenhassine
Copy link
Contributor

This PR resolves gh-14484.

Before this commit, Spring Boot was calling the JobParametersBuilder#getNextJobParameters
method from batch. This method was getting the previous parameters of the wrong job instance in a restart scenario.

This commit fixes the issue by first getting the right job instance with the provided parameters, then restarting it if necessary.

To correctly fix the issue reported in gh-14484, a fix needs to be applied on both sides (batch and boot). This PR is coupled with the one opened against batch: spring-projects/spring-batch#660.

Before this commit, Spring Boot was calling the `getNextJobParameters`
method of the `JobParametersBuilder` from batch. This method was getting
the previous parameters of the wrong job instance in a restart scenario.

This commit fixes the issue by first getting the right job instance with
the provided parameters, then restarting it.

Resolves gh-14484
@dturanski
Copy link

dturanski commented Oct 25, 2018

@benas - I'm seeing the same behavior on a COMPLETED job. Just trying to run multiple times with a different parameter value for localFilePath. Works fine for non-id parameter -localFilePath

@fmbenhassine
Copy link
Contributor Author

fmbenhassine commented Oct 26, 2018

@dturanski Does your job have a job parameters incrementer?

@dturanski
Copy link

@benas - yes RunIdIncrementer. I will try against this PR

@philwebb
Copy link
Member

The PR to batch is going to be backported so we'll rebase this PR onto 2.0.x

@fmbenhassine
Copy link
Contributor Author

I see the milestone for this PR changed to 2.0.x. Will this be merged to boot 2.1 too? The PR on batch has been merged in both the master branch for batch 4.1 (for boot 2.1) and the 4.0.x branch (for boot 2.0.x)

@philwebb
Copy link
Member

@benas Yeah, we always forward merge from the maintenance branches.

@philwebb philwebb removed the status: blocked An issue that's blocked on an external project change label Oct 26, 2018
@philwebb philwebb self-assigned this Oct 26, 2018
philwebb added a commit that referenced this pull request Oct 26, 2018
* pr/14933:
  Polish "Fix Spring Batch job restart parameters handling"
  Fix Spring Batch job restart parameters handling
@philwebb philwebb modified the milestones: 2.0.x, 2.0.7 Oct 26, 2018
@philwebb philwebb closed this in ad3c3ad Oct 26, 2018
@fmbenhassine fmbenhassine deleted the gh-14484 branch November 5, 2018 13:39
@aritzbastida
Copy link

aritzbastida commented Nov 6, 2018

Hi @benas! Thanks for the PR! Just one small detail, which I think could be simplified a bit:

In a restart scenario, we calculate the job parameters as:

	JobParameters previousParameters = lastJobExecution.getJobParameters();
	JobParameters previousIdentifyingParameters = removeNonIdentifying(previousParameters);
	parameters = merge(previousIdentifyingParameters, jobParameters);

but, afaik, the result will be exactly the same jobParameters specified as input!! So we could use the jobParameters variable with no further calculation, right?

@fmbenhassine
Copy link
Contributor Author

@aritzbastida Please let's discuss this point on the gitter channel of Spring Batch as this PR got merged and I will explain in details the code snippet above.

@aritzbastida
Copy link

Ok, you're right. Where is that? Can you post the link?

@fmbenhassine
Copy link
Contributor Author

It is here: https://gitter.im/spring-batch/Lobby

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

Successfully merging this pull request may close these issues.

JobLauncherCommandLineRunner picks a wrong job instance.
5 participants