diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java index 68fa1b42c..5a1de1ce3 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java @@ -180,7 +180,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw } catch (Throwable e) { - logger.error("Encountered an error executing the step"); + logger.error("Encountered an error executing the step: "+e.getClass()+": "+e.getMessage()); stepExecution.setStatus(determineBatchStatus(e)); exitStatus = getDefaultExitStatusForFailure(e); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleStepFactoryBean.java index cfdacfc43..f2a5ca336 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleStepFactoryBean.java @@ -40,7 +40,6 @@ import org.springframework.util.Assert; * appropriate subclass of this factory bean to configure skip or retry. * * @see SkipLimitStepFactoryBean - * @see StatefulRetryStepFactoryBean * * @author Dave Syer * diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java index 466ad9a30..9181ecbee 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java @@ -23,6 +23,7 @@ import org.springframework.batch.retry.RetryOperations; import org.springframework.batch.retry.backoff.BackOffPolicy; import org.springframework.batch.retry.callback.RecoveryRetryCallback; import org.springframework.batch.retry.policy.ExceptionClassifierRetryPolicy; +import org.springframework.batch.retry.policy.MapRetryContextCache; import org.springframework.batch.retry.policy.NeverRetryPolicy; import org.springframework.batch.retry.policy.RecoveryCallbackRetryPolicy; import org.springframework.batch.retry.policy.SimpleRetryPolicy; @@ -43,7 +44,6 @@ import org.springframework.batch.support.SubclassExceptionClassifier; * {@link #setNoRollbackForExceptionClasses(Class[])} list. * * @see SimpleStepFactoryBean - * @see StatefulRetryStepFactoryBean * * @author Dave Syer * @author Robert Kasanicky @@ -61,8 +61,7 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean { private ItemKeyGenerator itemKeyGenerator; - // TODO: build this into the retry policy - private int skipCacheCapacity = 1024; + private int cacheCapacity = 0; private int retryLimit; @@ -81,6 +80,27 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean { this.retryLimit = retryLimit; } + /** + * Public setter for the capacity of the cache in the retry policy. If more + * items than this fail without being skipped or recovered an exception will + * be thrown. This is to guard against inadvertent infinite loops generated + * by item identity problems. If a large number of items are failing and not + * being recognized as skipped, it usually signals a problem with the key + * generation (often equals and hashCode in the item itself). So it is + * better to enforce a strict limit than have weird looking errors, where a + * skip limit is reached without anything being skipped.
+ * + * The default value should be high enough and more for most purposes. To + * breach the limit in a single-threaded step typically you have to have + * this many failures in a single transaction. Defaults to the value in the + * {@link MapRetryContextCache}. + * + * @param cacheCapacity the cacheCapacity to set + */ + public void setCacheCapacity(int cacheCapacity) { + this.cacheCapacity = cacheCapacity; + } + /** * Public setter for the Class[]. * @param retryableExceptionClasses the retryableExceptionClasses to set @@ -149,24 +169,6 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean { this.itemKeyGenerator = itemKeyGenerator; } - /** - * Public setter for the capacity of the skipped item cache. If a large - * number of items are failing and not being recognized as skipped, it - * usually signals a problem with the key generation (often equals and - * hashCode in the item itself). So it is better to enforce a strict limit - * than have weird looking errors, where a skip limit is reached without - * anything being skipped.
- * - * The default value is 1024 which should be high enough and more for most - * purposes. To breach the limit in a single-threaded step typically you - * have to have this many failures in a single transaction. - * - * @param skipCacheCapacity the capacity to set - */ - public void setSkipCacheCapacity(int skipCacheCapacity) { - this.skipCacheCapacity = skipCacheCapacity; - } - /** * Skippable noRollbackForExceptionClasses will *not* cause transaction * rollback. @@ -192,7 +194,7 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean { addFatalExceptionIfMissing(RetryException.class); SimpleRetryPolicy simpleRetryPolicy = new SimpleRetryPolicy(retryLimit); - if (retryableExceptionClasses.length>0) { // otherwise we retry + if (retryableExceptionClasses.length > 0) { // otherwise we retry // all exceptions simpleRetryPolicy.setRetryableExceptionClasses(retryableExceptionClasses); } @@ -218,6 +220,9 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean { RecoveryCallbackRetryPolicy recoveryCallbackRetryPolicy = new RecoveryCallbackRetryPolicy(retryPolicy); recoveryCallbackRetryPolicy.setRecoverableExceptionClasses(noRollbackForExceptionClasses); + if (cacheCapacity > 0) { + recoveryCallbackRetryPolicy.setRetryContextCache(new MapRetryContextCache(cacheCapacity)); + } RetryTemplate retryTemplate = new RetryTemplate(); if (retryListeners != null) { diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBean.java index 22691c8ac..0e6fe9d34 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBean.java @@ -39,6 +39,9 @@ import org.springframework.batch.retry.RetryPolicy; * * @author Dave Syer * + * @deprecated All the features of this factory bean were moved to + * {@link SkipLimitStepFactoryBean} in version 1.1.0. + * */ public class StatefulRetryStepFactoryBean extends SkipLimitStepFactoryBean { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBeanTests.java index 381dcca87..95b032cc4 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/StatefulRetryStepFactoryBeanTests.java @@ -37,11 +37,14 @@ import org.springframework.batch.core.repository.dao.MapJobInstanceDao; import org.springframework.batch.core.repository.dao.MapStepExecutionDao; import org.springframework.batch.core.repository.support.SimpleJobRepository; import org.springframework.batch.core.step.AbstractStep; +import org.springframework.batch.core.step.skip.SkipLimitExceededException; +import org.springframework.batch.item.AbstractItemReader; import org.springframework.batch.item.AbstractItemWriter; import org.springframework.batch.item.ItemReader; import org.springframework.batch.item.ItemWriter; import org.springframework.batch.item.support.ListItemReader; import org.springframework.batch.retry.RetryException; +import org.springframework.batch.retry.policy.RetryCacheCapacityExceededException; import org.springframework.batch.support.transaction.ResourcelessTransactionManager; import org.springframework.batch.support.transaction.TransactionAwareProxyFactory; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -54,7 +57,7 @@ public class StatefulRetryStepFactoryBeanTests extends TestCase { protected final Log logger = LogFactory.getLog(getClass()); - private StatefulRetryStepFactoryBean factory = new StatefulRetryStepFactoryBean(); + private SkipLimitStepFactoryBean factory = new SkipLimitStepFactoryBean(); private List recovered = new ArrayList(); @@ -215,4 +218,78 @@ public class StatefulRetryStepFactoryBeanTests extends TestCase { // terminator assertEquals(17, count); } + + public void testRetryWithNoSkip() throws Exception { + factory.setRetryableExceptionClasses(new Class[] { Exception.class }); + factory.setRetryLimit(4); + factory.setSkipLimit(0); + List items = TransactionAwareProxyFactory.createTransactionalList(); + items.addAll(Arrays.asList(new String[] { "b" })); + ItemReader provider = new ListItemReader(items) { + public Object read() { + Object item = super.read(); + count++; + return item; + } + }; + ItemWriter itemWriter = new AbstractItemWriter() { + public void write(Object item) throws Exception { + logger.debug("Write Called! Item: [" + item + "]"); + throw new RuntimeException("Write error - planned but retryable."); + } + }; + factory.setItemReader(provider); + factory.setItemWriter(itemWriter); + AbstractStep step = (AbstractStep) factory.getObject(); + + StepExecution stepExecution = new StepExecution(step.getName(), jobExecution); + try { + step.execute(stepExecution); + fail("Expected SkipLimitExceededException"); + } + catch (SkipLimitExceededException e) { + // expected + } + + assertEquals(0, stepExecution.getSkipCount()); + // b is processed 4 times plus the null at end + assertEquals(5, count); + } + + public void testCacheLimitWithRetry() throws Exception { + factory.setRetryableExceptionClasses(new Class[] { Exception.class }); + factory.setRetryLimit(2); + // set the cache limit lower than the number of unique un-recovered + // errors expected + factory.setCacheCapacity(2); + ItemReader provider = new AbstractItemReader() { + public Object read() { + Object item = new Object(); + count++; + return item; + } + }; + ItemWriter itemWriter = new AbstractItemWriter() { + public void write(Object item) throws Exception { + logger.debug("Write Called! Item: [" + item + "]"); + throw new RuntimeException("Write error - planned but retryable."); + } + }; + factory.setItemReader(provider); + factory.setItemWriter(itemWriter); + AbstractStep step = (AbstractStep) factory.getObject(); + + StepExecution stepExecution = new StepExecution(step.getName(), jobExecution); + try { + step.execute(stepExecution); + fail("Expected RetryCacheCapacityExceededException"); + } + catch (RetryCacheCapacityExceededException e) { + // expected + } + + assertEquals(0, stepExecution.getSkipCount()); + // 2 processed and cached, 3rd barfed because cache was full + assertEquals(3, count); + } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/RetryContextCache.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/RetryContextCache.java index ceb21c7b7..9fa243fa0 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/RetryContextCache.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/RetryContextCache.java @@ -19,7 +19,7 @@ package org.springframework.batch.retry.policy; import org.springframework.batch.retry.RetryContext; /** - * Simple map-like bstraction for stateful retry policies to use when storing + * Simple map-like abstraction for stateful retry policies to use when storing * and retrieving {@link RetryContext} instances. * * @author Dave Syer