diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java index 7f3ae6acb..b8de6175b 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/TaskletStep.java @@ -306,6 +306,8 @@ public class TaskletStep extends AbstractStep { private boolean rolledBack = false; + private boolean stepExecutionUpdated = false; + private StepExecution oldVersion; private boolean locked = false; @@ -319,11 +321,15 @@ public class TaskletStep extends AbstractStep { public void afterCompletion(int status) { try { if (status != TransactionSynchronization.STATUS_COMMITTED) { - if (oldVersion != null) { + if (stepExecutionUpdated) { // Wah! the commit failed. We need to rescue the step // execution data. + logger.info("Commit failed while step execution data was already updated. " + + "Reverting to old version."); copy(oldVersion, stepExecution); - stepExecution.incrementRollbackCount(); + if (status == TransactionSynchronization.STATUS_ROLLED_BACK) { + rollback(stepExecution); + } } } if (status == TransactionSynchronization.STATUS_UNKNOWN) { @@ -353,6 +359,11 @@ public class TaskletStep extends AbstractStep { chunkListener.beforeChunk(); + // In case we need to push it back to its old value + // after a commit fails... + oldVersion = new StepExecution(stepExecution.getStepName(), stepExecution.getJobExecution()); + copy(stepExecution, oldVersion); + try { try { @@ -386,11 +397,6 @@ public class TaskletStep extends AbstractStep { Thread.currentThread().interrupt(); } - // In case we need to push it back to its old value - // after a commit fails... - oldVersion = new StepExecution(stepExecution.getStepName(), stepExecution.getJobExecution()); - copy(stepExecution, oldVersion); - // Apply the contribution to the step // even if unsuccessful logger.debug("Applying contribution: " + contribution); @@ -398,6 +404,8 @@ public class TaskletStep extends AbstractStep { } + stepExecutionUpdated = true; + stream.update(stepExecution.getExecutionContext()); try { @@ -446,12 +454,13 @@ public class TaskletStep extends AbstractStep { rolledBack = true; } } - + private void copy(final StepExecution source, final StepExecution target) { target.setVersion(source.getVersion()); target.setWriteCount(source.getWriteCount()); target.setFilterCount(source.getFilterCount()); - target.setRollbackCount(source.getRollbackCount()); + target.setCommitCount(source.getCommitCount()); + target.setExecutionContext(new ExecutionContext(source.getExecutionContext())); } } 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 2c41e2b9c..2a6ed338a 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 @@ -36,6 +36,7 @@ import org.springframework.batch.item.ItemStreamSupport; import org.springframework.batch.repeat.RepeatStatus; import org.springframework.batch.support.transaction.ResourcelessTransactionManager; import org.springframework.transaction.TransactionException; +import org.springframework.transaction.UnexpectedRollbackException; import org.springframework.transaction.support.DefaultTransactionStatus; /** @@ -210,6 +211,7 @@ public class TaskletStepExceptionTests { taskletStep.setTasklet(new Tasklet() { public RepeatStatus execute(StepContribution contribution, ChunkContext attributes) throws Exception { + attributes.getStepContext().getStepExecution().getExecutionContext().putString("foo", "bar"); return RepeatStatus.FINISHED; } @@ -219,6 +221,38 @@ public class TaskletStepExceptionTests { assertEquals(UNKNOWN, stepExecution.getStatus()); Throwable e = stepExecution.getFailureExceptions().get(0); assertEquals("foo", e.getMessage()); + assertEquals(0, stepExecution.getCommitCount()); + assertEquals(1, stepExecution.getRollbackCount()); // Failed transaction counts as rollback + assertEquals(0, stepExecution.getExecutionContext().size()); + } + + @Test + public void testUnexpectedRollback() throws Exception { + + taskletStep.setTransactionManager(new ResourcelessTransactionManager() { + @Override + protected void doCommit(DefaultTransactionStatus status) throws TransactionException { + super.doRollback(status); + throw new UnexpectedRollbackException("bar"); + } + }); + + taskletStep.setTasklet(new Tasklet() { + + public RepeatStatus execute(StepContribution contribution, ChunkContext attributes) throws Exception { + attributes.getStepContext().getStepExecution().getExecutionContext().putString("foo", "bar"); + return RepeatStatus.FINISHED; + } + + }); + + taskletStep.execute(stepExecution); + assertEquals(FAILED, stepExecution.getStatus()); + Throwable e = stepExecution.getFailureExceptions().get(0); + assertEquals("bar", e.getMessage()); + assertEquals(0, stepExecution.getCommitCount()); + assertEquals(1, stepExecution.getRollbackCount()); // Failed transaction counts as rollback + assertEquals(0, stepExecution.getExecutionContext().size()); } @Test