From fac04aeecee8ab8d46ad23ff4406d3a30e64ad3c Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Wed, 29 Mar 2017 13:00:21 -0400 Subject: [PATCH] INT-4250: Fix Statistics Mean Decay with Time JIRA: https://jira.spring.io/browse/INT-4250 When retrieving the mean from `ExponentialMovingAverageRate` or `ExponentialMovingAverageRatio` via `getStatistics()` the mean did not decay over time. The mean did decay when using `getMean()`. This was caused by the statistics performance refactoring. --- .../ExponentialMovingAverageRate.java | 25 +++++++++++++------ .../ExponentialMovingAverageRatio.java | 24 ++++++++++++------ .../support/management/Statistics.java | 9 +++++-- .../ExponentialMovingAverageRateTests.java | 4 ++- .../ExponentialMovingAverageRatioTests.java | 8 ++++++ 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRate.java b/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRate.java index 036c40d75a..44afa0e4a1 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRate.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRate.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2016 the original author or authors. + * Copyright 2009-2017 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. @@ -118,7 +118,7 @@ public class ExponentialMovingAverageRate { this.count++; //NOSONAR - false positive, we're synchronized } - private Statistics calc() { + private Statistics calcStatic() { List copy; long count; synchronized (this) { @@ -197,6 +197,15 @@ public class ExponentialMovingAverageRate { * @return the mean value */ public double getMean() { + return recalcMean(calcStatic()); + } + + /** + * Decay the mean using the current time. + * @param staticStats the static statistics. + * @return the new mean. + */ + private double recalcMean(Statistics staticStats) { long count = this.count; count = count > this.retention ? this.retention : count; if (count == 0) { @@ -205,7 +214,7 @@ public class ExponentialMovingAverageRate { double t0 = lastTime(); double t = System.nanoTime() / this.factor; double value = t > t0 ? (t - t0) / this.period : 0; - return count / (count / calc().getMean() + value); + return count / (count / staticStats.getMean() + value); } private synchronized double lastTime() { @@ -221,14 +230,14 @@ public class ExponentialMovingAverageRate { * @return the approximate standard deviation */ public double getStandardDeviation() { - return calc().getStandardDeviation(); + return calcStatic().getStandardDeviation(); } /** * @return the maximum value recorded (not weighted) */ public double getMax() { - double min = calc().getMin(); + double min = calcStatic().getMin(); return min > 0 ? 1 / min : 0; } @@ -236,7 +245,7 @@ public class ExponentialMovingAverageRate { * @return the minimum value recorded (not weighted) */ public double getMin() { - double max = calc().getMax(); + double max = calcStatic().getMax(); return max > 0 ? 1 / max : 0; } @@ -244,7 +253,9 @@ public class ExponentialMovingAverageRate { * @return summary statistics (count, mean, standard deviation etc.) */ public Statistics getStatistics() { - return calc(); + Statistics staticStats = calcStatic(); + staticStats.setMean(recalcMean(staticStats)); + return staticStats; } @Override diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRatio.java b/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRatio.java index 1637fd8563..6d5d7cccd6 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRatio.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/management/ExponentialMovingAverageRatio.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2016 the original author or authors. + * Copyright 2009-2017 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. @@ -137,7 +137,7 @@ public class ExponentialMovingAverageRatio { this.count++; //NOSONAR - false positive, we're synchronized } - private Statistics calc() { + private Statistics calcStatic() { List copyTimes; List copyValues; long count; @@ -219,7 +219,15 @@ public class ExponentialMovingAverageRatio { // Optimistic to start: success rate is 100% return 1; } - Statistics statistics = calc(); + return decayMean(calcStatic()); + } + + /** + * Decay the mean using the current time. + * @param staticStats the static statistics. + * @return the new mean. + */ + private double decayMean(Statistics statistics) { double t = System.nanoTime() / this.factor; double mean = statistics.getMean(); double alpha = Math.exp((lastTime() / this.factor - t) * this.lapse); @@ -239,28 +247,30 @@ public class ExponentialMovingAverageRatio { * @return the approximate standard deviation of the success rate measurements */ public double getStandardDeviation() { - return calc().getStandardDeviation(); + return calcStatic().getStandardDeviation(); } /** * @return the maximum value recorded of the exponential weighted average (per measurement) success rate */ public double getMax() { - return calc().getMax(); + return calcStatic().getMax(); } /** * @return the minimum value recorded of the exponential weighted average (per measurement) success rate */ public double getMin() { - return calc().getMin(); + return calcStatic().getMin(); } /** * @return summary statistics (count, mean, standard deviation etc.) */ public Statistics getStatistics() { - return calc(); + Statistics staticStats = calcStatic(); + staticStats.setMean(decayMean(staticStats)); + return staticStats; } @Override diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/management/Statistics.java b/spring-integration-core/src/main/java/org/springframework/integration/support/management/Statistics.java index e7f57cc615..92fe7d1444 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/management/Statistics.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/management/Statistics.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 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. @@ -18,6 +18,7 @@ package org.springframework.integration.support.management; /** * @author Dave Syer + * @author Gary Russell * @since 2.0 */ public class Statistics { @@ -28,7 +29,7 @@ public class Statistics { private final double max; - private final double mean; + private double mean; private final double standardDeviation; @@ -62,6 +63,10 @@ public class Statistics { return this.mean; } + public void setMean(double mean) { + this.mean = mean; + } + public double getStandardDeviation() { return this.standardDeviation; } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRateTests.java b/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRateTests.java index ae4f9bd9f0..c46bfcaf0b 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRateTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -106,6 +106,7 @@ public class ExponentialMovingAverageRateTests { Thread.sleep(20L); history.increment(); double before = history.getMean(); + Statistics statisticsBefore = history.getStatistics(); long elapsed = System.currentTimeMillis() - t0; if (elapsed < 50L) { assertTrue(before > 10); @@ -113,6 +114,7 @@ public class ExponentialMovingAverageRateTests { elapsed = System.currentTimeMillis() - t0; if (elapsed < 80L) { assertThat(history.getMean(), lessThan(before)); + assertThat(history.getStatistics().getMean(), lessThan(statisticsBefore.getMean())); } else { logger.warn("Test took too long to verify mean"); diff --git a/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRatioTests.java b/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRatioTests.java index 652d6b6b84..eac2396346 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRatioTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/support/management/ExponentialMovingAverageRatioTests.java @@ -17,6 +17,7 @@ package org.springframework.integration.support.management; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -96,6 +97,13 @@ public class ExponentialMovingAverageRatioTests { public void testDecayedMean() throws Exception { history.failure(System.nanoTime() - 200000000); assertEquals(average(0, Math.exp(-0.4)), history.getMean(), 0.01); + history.success(); + history.failure(); + double mean = history.getMean(); + Statistics statistics = history.getStatistics(); + Thread.sleep(50); + assertThat(history.getMean(), greaterThan(mean)); + assertThat(history.getStatistics().getMean(), greaterThan(statistics.getMean())); } @Test