diff --git a/src/main/java/org/springframework/retry/policy/CircuitBreakerRetryPolicy.java b/src/main/java/org/springframework/retry/policy/CircuitBreakerRetryPolicy.java index b4f8b0f..4c92d2e 100644 --- a/src/main/java/org/springframework/retry/policy/CircuitBreakerRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/CircuitBreakerRetryPolicy.java @@ -16,6 +16,8 @@ package org.springframework.retry.policy; +import java.util.concurrent.atomic.AtomicInteger; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.retry.RetryContext; @@ -28,7 +30,9 @@ import org.springframework.retry.context.RetryContextSupport; */ public class CircuitBreakerRetryPolicy implements RetryPolicy { - public static final String CIRCUIT_OPEN = "circuite.open"; + public static final String CIRCUIT_OPEN = "circuit.open"; + + public static final String CIRCUIT_SHORT_COUNT = "circuit.shortCount"; private static Log logger = LogFactory.getLog(CircuitBreakerRetryPolicy.class); @@ -65,8 +69,12 @@ public class CircuitBreakerRetryPolicy implements RetryPolicy { public boolean canRetry(RetryContext context) { CircuitBreakerRetryContext circuit = (CircuitBreakerRetryContext) context; if (circuit.isOpen()) { + circuit.incrementShortCircuitCount(); return false; } + else { + circuit.reset(); + } return this.delegate.canRetry(circuit.context); } @@ -96,6 +104,7 @@ public class CircuitBreakerRetryPolicy implements RetryPolicy { private volatile long start = System.currentTimeMillis(); private final long timeout; private final long openWindow; + private final AtomicInteger shortCircuitCount = new AtomicInteger(); public CircuitBreakerRetryContext(RetryContext parent, RetryPolicy policy, long timeout, long openWindow) { @@ -107,9 +116,19 @@ public class CircuitBreakerRetryPolicy implements RetryPolicy { setAttribute("state.global", true); } + public void reset() { + shortCircuitCount.set(0); + } + + public void incrementShortCircuitCount() { + shortCircuitCount.incrementAndGet(); + setAttribute(CIRCUIT_SHORT_COUNT, shortCircuitCount.get()); + } + private RetryContext createDelegateContext(RetryPolicy policy, RetryContext parent) { RetryContext context = policy.open(parent); + reset(); return context; } diff --git a/src/main/java/org/springframework/retry/stats/DefaultRetryStatistics.java b/src/main/java/org/springframework/retry/stats/DefaultRetryStatistics.java index 47b24ef..ca8a12f 100644 --- a/src/main/java/org/springframework/retry/stats/DefaultRetryStatistics.java +++ b/src/main/java/org/springframework/retry/stats/DefaultRetryStatistics.java @@ -86,26 +86,6 @@ public class DefaultRetryStatistics extends AttributeAccessorSupport implements this.name = name; } - public void setStartedCount(int startedCount) { - this.startedCount.set(startedCount); - } - - public void setCompleteCount(int completeCount) { - this.completeCount.set(completeCount); - } - - public void setRecoveryCount(int recoveryCount) { - this.recoveryCount.set(recoveryCount); - } - - public void setErrorCount(int errorCount) { - this.errorCount.set(errorCount); - } - - public void setAbortCount(int abortCount) { - this.abortCount.set(abortCount); - } - @Override public void incrementStartedCount() { this.startedCount.incrementAndGet(); diff --git a/src/main/java/org/springframework/retry/stats/StatisticsListener.java b/src/main/java/org/springframework/retry/stats/StatisticsListener.java index 0181664..4221dce 100644 --- a/src/main/java/org/springframework/retry/stats/StatisticsListener.java +++ b/src/main/java/org/springframework/retry/stats/StatisticsListener.java @@ -57,10 +57,12 @@ public class StatisticsListener extends RetryListenerSupport { } RetryStatistics stats = repository.findOne(name); if (stats instanceof AttributeAccessor) { - if (context.hasAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)) { - ((AttributeAccessor) stats).setAttribute( - CircuitBreakerRetryPolicy.CIRCUIT_OPEN, - context.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); + AttributeAccessor accessor = (AttributeAccessor) stats; + for (String key : new String[] { CircuitBreakerRetryPolicy.CIRCUIT_OPEN, + CircuitBreakerRetryPolicy.CIRCUIT_SHORT_COUNT }) { + if (context.hasAttribute(key)) { + accessor.setAttribute(key, context.getAttribute(key)); + } } } } diff --git a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java index 8e6f6d6..a1eca9e 100644 --- a/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java +++ b/src/test/java/org/springframework/retry/annotation/CircuitBreakerTests.java @@ -27,6 +27,7 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.retry.RetryContext; +import org.springframework.retry.policy.CircuitBreakerRetryPolicy; import org.springframework.retry.support.RetrySynchronizationManager; /** @@ -47,21 +48,21 @@ public class CircuitBreakerTests { } catch (Exception e) { } - assertFalse((Boolean)service.getContext().getAttribute("open")); + assertFalse((Boolean)service.getContext().getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); try { service.service(); fail("Expected exception"); } catch (Exception e) { } - assertFalse((Boolean)service.getContext().getAttribute("open")); + assertFalse((Boolean)service.getContext().getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); try { service.service(); fail("Expected exception"); } catch (Exception e) { } - assertTrue((Boolean)service.getContext().getAttribute("open")); + assertTrue((Boolean)service.getContext().getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); assertEquals(3, service.getCount()); try { service.service(); diff --git a/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java b/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java index e0671b2..b64d59d 100644 --- a/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java +++ b/src/test/java/org/springframework/retry/stats/CircuitBreakerStatisticsTests.java @@ -25,7 +25,6 @@ import org.springframework.retry.RecoveryCallback; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; import org.springframework.retry.RetryListener; -import org.springframework.retry.RetryStatistics; import org.springframework.retry.policy.CircuitBreakerRetryPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.DefaultRetryState; @@ -69,7 +68,7 @@ public class CircuitBreakerStatisticsTests { .setRetryPolicy(new CircuitBreakerRetryPolicy(new NeverRetryPolicy())); Object result = this.retryTemplate.execute(this.callback, this.recovery, this.state); - RetryStatistics stats = repository.findOne("test"); + MutableRetryStatistics stats = (MutableRetryStatistics) repository.findOne("test"); // System.err.println(stats); assertEquals(1, stats.getStartedCount()); assertEquals(RECOVERED, result); @@ -77,6 +76,9 @@ public class CircuitBreakerStatisticsTests { assertEquals(RECOVERED, result); assertEquals("There should be two recoveries", 2, stats.getRecoveryCount()); assertEquals("There should only be one error because the circuit is now open", 1, stats.getErrorCount()); + assertEquals(true, stats.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); + // Both recoveries are through a short circuit because we used NeverRetryPolicy + assertEquals(2, stats.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_SHORT_COUNT)); } @Test @@ -92,9 +94,10 @@ public class CircuitBreakerStatisticsTests { this.retryTemplate.execute(this.callback, this.state); } catch (Exception e) { } - RetryStatistics stats = repository.findOne("test"); + MutableRetryStatistics stats = (MutableRetryStatistics) repository.findOne("test"); assertEquals("There should be two recoveries", 2, stats.getAbortCount()); assertEquals("There should only be one error because the circuit is now open", 1, stats.getErrorCount()); + assertEquals(true, stats.getAttribute(CircuitBreakerRetryPolicy.CIRCUIT_OPEN)); } protected static class MockRetryCallback implements RetryCallback { diff --git a/src/test/java/org/springframework/retry/stats/ExponentialAverageRetryStatisticsTests.java b/src/test/java/org/springframework/retry/stats/ExponentialAverageRetryStatisticsTests.java new file mode 100644 index 0000000..6a1bb49 --- /dev/null +++ b/src/test/java/org/springframework/retry/stats/ExponentialAverageRetryStatisticsTests.java @@ -0,0 +1,94 @@ +/* + * Copyright 2012-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.retry.stats; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; + +import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + +/** + * @author Dave Syer + * + */ +public class ExponentialAverageRetryStatisticsTests { + + private ExponentialAverageRetryStatistics stats = new ExponentialAverageRetryStatistics( + "test"); + + @Test + public void attributes() throws Exception { + stats.setAttribute("foo", "bar");; + assertEquals("bar", stats.getAttribute("foo")); + assertTrue(Arrays.asList(stats.attributeNames()).contains("foo")); + } + + @Test + public void abortCount() throws Exception { + stats.incrementAbortCount(); + assertEquals(1, stats.getAbortCount()); + // rounds up to 1 + assertEquals(1, stats.getRollingAbortCount()); + } + + @Test + public void errorCount() throws Exception { + stats.incrementErrorCount(); + assertEquals(1, stats.getErrorCount()); + // rounds up to 1 + assertEquals(1, stats.getRollingErrorCount()); + } + + @Test + public void startedCount() throws Exception { + stats.incrementStartedCount(); + assertEquals(1, stats.getStartedCount()); + // rounds up to 1 + assertEquals(1, stats.getRollingStartedCount()); + } + + @Test + public void completeCount() throws Exception { + stats.incrementCompleteCount(); + assertEquals(1, stats.getCompleteCount()); + // rounds up to 1 + assertEquals(1, stats.getRollingCompleteCount()); + } + + @Test + public void recoveryCount() throws Exception { + stats.incrementRecoveryCount(); + assertEquals(1, stats.getRecoveryCount()); + // rounds up to 1 + assertEquals(1, stats.getRollingRecoveryCount()); + } + + @Test + public void oldValuesDecay() throws Exception { + stats.incrementAbortCount(); + assertEquals(1, stats.getAbortCount()); + // Wind back time to epoch 0 + ReflectionTestUtils.setField(ReflectionTestUtils.getField(stats, "abort"), + "lastTime", 0); + // rounds down to 1 + assertEquals(0, stats.getRollingAbortCount()); + } + +}