From cba31c9636ce297ed6edfa60e73518696193f315 Mon Sep 17 00:00:00 2001 From: Krishna Bhamidipati Date: Wed, 15 May 2019 09:28:05 -0400 Subject: [PATCH] Performance improvement for partitions with a large number of steps New method on JobRepository that enables better performance of SimpleStepExecutionSplitter. By retrieving all step executions at the time of partitioning outside the for loop, the number of calls to the database is reduced. Also updated JSR version to be performant. Deprecated the older step execution methods and cleaned up the javadocs. Stub implementations provided for common support classes https://jira.spring.io/browse/BATCH-2716 --- .../partition/JsrStepExecutionSplitter.java | 6 +++- .../support/SimpleStepExecutionSplitter.java | 35 +++++++++++++++---- .../batch/core/repository/JobRepository.java | 16 ++++++++- .../support/SimpleJobRepository.java | 26 ++++++++++---- .../configuration/xml/DummyJobRepository.java | 10 ++++++ .../batch/core/step/JobRepositorySupport.java | 10 ++++++ .../step/item/TaskletStepExceptionTests.java | 10 ++++++ .../integration/JobRepositorySupport.java | 10 ++++++ 8 files changed, 108 insertions(+), 15 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/partition/JsrStepExecutionSplitter.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/partition/JsrStepExecutionSplitter.java index 257489de85..fbdb24d6d1 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/partition/JsrStepExecutionSplitter.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/partition/JsrStepExecutionSplitter.java @@ -17,12 +17,14 @@ import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobExecutionException; +import org.springframework.batch.core.JobInstance; import org.springframework.batch.core.StepExecution; import org.springframework.batch.core.jsr.launch.JsrJobOperator; import org.springframework.batch.core.partition.support.SimpleStepExecutionSplitter; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.item.ExecutionContext; +import java.util.Collection; import java.util.Comparator; import java.util.Set; import java.util.TreeSet; @@ -78,13 +80,15 @@ public int compare(StepExecution arg0, StepExecution arg1) { } }); JobExecution jobExecution = stepExecution.getJobExecution(); + JobInstance jobInstance = stepExecution.getJobExecution().getJobInstance(); + Collection allPriorStepExecutions = jobRepository.getStepExecutions(jobInstance); for(int i = 0; i < gridSize; i++) { String stepName = this.stepName + ":partition" + i; JobExecution curJobExecution = new JobExecution(jobExecution); StepExecution curStepExecution = new StepExecution(stepName, curJobExecution); - if(!restoreState || isStartable(curStepExecution, new ExecutionContext())) { + if(!restoreState || isStartable(curStepExecution, new ExecutionContext(), allPriorStepExecutions)) { executions.add(curStepExecution); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/partition/support/SimpleStepExecutionSplitter.java b/spring-batch-core/src/main/java/org/springframework/batch/core/partition/support/SimpleStepExecutionSplitter.java index 522cfb2fbb..8b3525123a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/partition/support/SimpleStepExecutionSplitter.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/partition/support/SimpleStepExecutionSplitter.java @@ -178,6 +178,9 @@ public Set split(StepExecution stepExecution, int gridSize) throw Map contexts = getContexts(stepExecution, gridSize); Set set = new HashSet<>(contexts.size()); + JobInstance jobInstance = stepExecution.getJobExecution().getJobInstance(); + Collection allPriorStepExecutions = jobRepository.getStepExecutions(jobInstance); + for (Entry context : contexts.entrySet()) { // Make the step execution name unique and repeatable @@ -185,7 +188,7 @@ public Set split(StepExecution stepExecution, int gridSize) throw StepExecution currentStepExecution = jobExecution.createStepExecution(stepName); - boolean startable = isStartable(currentStepExecution, context.getValue()); + boolean startable = isStartable(currentStepExecution, context.getValue(), allPriorStepExecutions); if (startable) { set.add(currentStepExecution); @@ -245,9 +248,29 @@ private Map getContexts(StepExecution stepExecution, i * @param context the execution context of the step * @return true if the step execution is startable, false otherwise * @throws JobExecutionException if unable to check if the step execution is startable + * @deprecated This method is less performant than and deprecated in favor of + * {@link SimpleStepExecutionSplitter#isStartable(StepExecution, ExecutionContext, Collection)} */ + @Deprecated protected boolean isStartable(StepExecution stepExecution, ExecutionContext context) throws JobExecutionException { - return getStartable(stepExecution, context); + JobInstance jobInstance = stepExecution.getJobExecution().getJobInstance(); + String stepName = stepExecution.getStepName(); + StepExecution lastStepExecution = jobRepository.getLastStepExecution(jobInstance, stepName); + return isStartable(stepExecution, context, lastStepExecution); + } + + /** + * Check if a step execution is startable. + * @param stepExecution the step execution to check + * @param context the execution context of the step + * @param allPriorStepExecutions all the step executions in the job repository at this moment to compare against + * @return true if the step execution is startable, false otherwise + * @throws JobExecutionException if unable to check if the step execution is startable + */ + protected boolean isStartable(StepExecution stepExecution, ExecutionContext context, Collection allPriorStepExecutions) throws JobExecutionException { + String stepName = stepExecution.getStepName(); + StepExecution lastStepExecution = jobRepository.getLastStepExecution(allPriorStepExecutions, stepName); + return isStartable(stepExecution, context, lastStepExecution); } /** @@ -262,11 +285,10 @@ protected boolean isStartable(StepExecution stepExecution, ExecutionContext cont */ @Deprecated protected boolean getStartable(StepExecution stepExecution, ExecutionContext context) throws JobExecutionException { + return isStartable(stepExecution, context); + } - JobInstance jobInstance = stepExecution.getJobExecution().getJobInstance(); - String stepName = stepExecution.getStepName(); - StepExecution lastStepExecution = jobRepository.getLastStepExecution(jobInstance, stepName); - + private boolean isStartable(StepExecution stepExecution, ExecutionContext context, StepExecution lastStepExecution) throws JobExecutionException { boolean isRestart = (lastStepExecution != null && lastStepExecution.getStatus() != BatchStatus.COMPLETED); if (isRestart) { @@ -277,7 +299,6 @@ protected boolean getStartable(StepExecution stepExecution, ExecutionContext con } return shouldStart(allowStartIfComplete, stepExecution, lastStepExecution) || isRestart; - } private boolean shouldStart(boolean allowStartIfComplete, StepExecution stepExecution, StepExecution lastStepExecution) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java index a65a7a51b9..3e4cbda00c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/JobRepository.java @@ -178,14 +178,28 @@ JobExecution createJobExecution(String jobName, JobParameters jobParameters) */ void updateExecutionContext(JobExecution jobExecution); + /** + * @return all step executions, from all job executions, for the given job instance. + */ + Collection getStepExecutions(JobInstance jobInstance); + /** * @param jobInstance {@link JobInstance} instance containing the step executions. * @param stepName the name of the step execution that might have run. - * @return the last execution of step for the given job instance. + * @return the last execution of step for the given job instance, null otherwise. */ @Nullable StepExecution getLastStepExecution(JobInstance jobInstance, String stepName); + /** + * + * @param stepExecutions all step executions to filter through + * @param stepName the name of the step execution that might have run. + * @return the last execution of step inside the list, null otherwise. + */ + @Nullable + StepExecution getLastStepExecution(Collection stepExecutions, String stepName); + /** * @param jobInstance {@link JobInstance} instance containing the step executions. * @param stepName the name of the step execution that might have run. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java index 05ef5127f9..959510d30a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java @@ -217,17 +217,31 @@ public void updateExecutionContext(JobExecution jobExecution) { } @Override - @Nullable - public StepExecution getLastStepExecution(JobInstance jobInstance, String stepName) { + public Collection getStepExecutions(JobInstance jobInstance) { List jobExecutions = jobExecutionDao.findJobExecutions(jobInstance); List stepExecutions = new ArrayList<>(jobExecutions.size()); for (JobExecution jobExecution : jobExecutions) { stepExecutionDao.addStepExecutions(jobExecution); - for (StepExecution stepExecution : jobExecution.getStepExecutions()) { - if (stepName.equals(stepExecution.getStepName())) { - stepExecutions.add(stepExecution); - } + stepExecutions.addAll(jobExecution.getStepExecutions()); + } + return stepExecutions; + } + + @Override + public StepExecution getLastStepExecution(JobInstance jobInstance, String stepName) { + Collection stepExecutions = getStepExecutions(jobInstance); + return getLastStepExecution(stepExecutions, stepName); + } + + @Override + public StepExecution getLastStepExecution(Collection allStepExecutions, String stepName) { + + List stepExecutions = new ArrayList(); + + for (StepExecution stepExecution : allStepExecutions) { + if (stepName.equals(stepExecution.getStepName())) { + stepExecutions.add(stepExecution); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/DummyJobRepository.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/DummyJobRepository.java index 485f265aa4..0cffb69759 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/DummyJobRepository.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/DummyJobRepository.java @@ -65,6 +65,11 @@ public StepExecution getLastStepExecution(JobInstance jobInstance, String stepNa return null; } + @Override + public StepExecution getLastStepExecution(Collection stepExecutions, String stepName) { + return null; + } + @Override public int getStepExecutionCount(JobInstance jobInstance, String stepName) { return 0; @@ -91,6 +96,11 @@ public void updateExecutionContext(StepExecution stepExecution) { public void updateExecutionContext(JobExecution jobExecution) { } + @Override + public Collection getStepExecutions(JobInstance jobInstance) { + return null; + } + @Override public void addAll(Collection stepExecutions) { } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java index 55339d9f83..6da04cf44d 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/JobRepositorySupport.java @@ -57,6 +57,11 @@ public StepExecution getLastStepExecution(JobInstance jobInstance, String stepNa return null; } + @Override + public StepExecution getLastStepExecution(Collection stepExecutions, String stepName) { + return null; + } + @Override public int getStepExecutionCount(JobInstance jobInstance, String stepName) { return 0; @@ -99,6 +104,11 @@ public JobExecution getLastJobExecution(String jobName, JobParameters jobParamet public void updateExecutionContext(JobExecution jobExecution) { } + @Override + public Collection getStepExecutions(JobInstance jobInstance) { + return null; + } + @Override public void addAll(Collection stepExecutions) { } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java index 9ac4ca1210..c60d5a019d 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java @@ -510,6 +510,11 @@ public StepExecution getLastStepExecution(JobInstance jobInstance, String stepNa return null; } + @Override + public StepExecution getLastStepExecution(Collection stepExecutions, String stepName) { + return null; + } + @Override public int getStepExecutionCount(JobInstance jobInstance, String stepName) { return 0; @@ -557,6 +562,11 @@ public JobExecution getLastJobExecution(String jobName, JobParameters jobParamet public void updateExecutionContext(JobExecution jobExecution) { } + @Override + public Collection getStepExecutions(JobInstance jobInstance) { + return null; + } + @Override public void addAll(Collection stepExecutions) { } diff --git a/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java b/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java index dedffa1951..155c0c4875 100644 --- a/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java +++ b/spring-batch-integration/src/test/java/org/springframework/batch/integration/JobRepositorySupport.java @@ -47,6 +47,11 @@ public StepExecution getLastStepExecution(JobInstance jobInstance, String stepNa return null; } + @Override + public StepExecution getLastStepExecution(Collection stepExecutions, String stepName) { + return null; + } + /* (non-Javadoc) * @see org.springframework.batch.core.repository.JobRepository#getStepExecutionCount(org.springframework.batch.core.JobInstance, org.springframework.batch.core.Step) */ @@ -75,6 +80,11 @@ public void updateExecutionContext(StepExecution stepExecution) { public void updateExecutionContext(JobExecution jobExecution) { } + @Override + public Collection getStepExecutions(JobInstance jobInstance) { + return null; + } + public void add(StepExecution stepExecution) { }