Performance improvement for step execution retrieval#4208
Closed
hpoettker wants to merge 1 commit intospring-projects:4.3.xfrom
hpoettker:get-step-executions-once
Closed
Performance improvement for step execution retrieval#4208hpoettker wants to merge 1 commit intospring-projects:4.3.xfrom hpoettker:get-step-executions-once
hpoettker wants to merge 1 commit intospring-projects:4.3.xfrom
hpoettker:get-step-executions-once
Conversation
|
do you guys have any expectation when the fix be pushed to spring-batch? |
vitenkov
approved these changes
Jan 23, 2023
Contributor
|
The idea of this PR is a clever trick and a nice middle ground between having the performance issue and fixing it with a single query for polling + shallow copies of step executions as mentioned in #3790 (comment). I appreciate the fact that you tried to fix the problem with 4.3.x in mind! This PR has been rebased and merged in v4.3.x and forward ported to v5 (without the JSR related changes, see 93800c6). Thank you for your contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is for versions 4.3.x and addresses #3790 and #4135.
Using
JobExplorer::getStepExecutionfor multiple step executions is not ideal. Each invocation will retrieve the corresponding job execution which in turn triggers the retrieval of all step executions of the job execution. IfJobExplorer::getStepExecutionis used for all step executions, this leads to efforts and memory consumption that scale quadratically in the number of step executions.The idea of the PR is to only call
JobExplorer::getJobExecutionand get the step executions from the returned job execution. The PR is intended to be conservative in its changes due to the late stage of the 4.3.x releases. That's why the filtering on the step execution ids are implemented although I'm not 100% sure that they are actually needed in every case.I tested the change locally with the reproducing project provided here: #3790 (comment) With 250 MB of heap, the application no longer crashes and the memory profile looks much better.
For Spring Batch 5, it makes sense to forward port the change, I think. It should also be considered to deprecate
JobExplorer::getStepExecutionand provide a way to retrieve aStepExecutionfrom aJobExecutionby id instead.