Skip to content

Commit c9bb895

Browse files
WIP: Make EWMR allow zero times
1 parent fb5c212 commit c9bb895

File tree

2 files changed

+7
-15
lines changed

2 files changed

+7
-15
lines changed

server/src/main/java/org/elasticsearch/common/metrics/ExponentiallyWeightedMovingRate.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ public class ExponentiallyWeightedMovingRate {
4141
private final double lambda;
4242
private final long startTime;
4343
private double rate;
44-
long lastTime;
44+
private long lastTime;
45+
private boolean waitingForFirstIncrement;
4546

4647
/**
4748
* Constructor.
@@ -57,14 +58,12 @@ public ExponentiallyWeightedMovingRate(double lambda, long startTime) {
5758
if (lambda < 0.0) {
5859
throw new IllegalArgumentException("lambda must be non-negative but was " + lambda);
5960
}
60-
if (startTime <= 0.0) {
61-
throw new IllegalArgumentException("startTime must be non-negative but was " + startTime);
62-
}
6361
synchronized (this) {
6462
this.lambda = lambda;
6563
this.rate = Double.NaN; // should never be used
6664
this.startTime = startTime;
67-
this.lastTime = 0; // after an increment, this must be positive, so a zero value indicates we're waiting for the first
65+
this.lastTime = 0; // should never be used
66+
this.waitingForFirstIncrement = true;
6867
}
6968
}
7069

@@ -80,7 +79,7 @@ public ExponentiallyWeightedMovingRate(double lambda, long startTime) {
8079
*/
8180
public double getRate(long time) {
8281
synchronized (this) {
83-
if (lastTime == 0) { // indicates that no increment has happened yet
82+
if (waitingForFirstIncrement) {
8483
return 0.0;
8584
} else if (time <= lastTime) {
8685
return rate;
@@ -130,12 +129,13 @@ public double calculateRateSince(long currentTime, double currentRate, long oldT
130129
*/
131130
public void addIncrement(double increment, long time) {
132131
synchronized (this) {
133-
if (lastTime == 0) { // indicates that this is the first increment
132+
if (waitingForFirstIncrement) {
134133
if (time <= startTime) {
135134
time = startTime + 1;
136135
}
137136
// This is the formula for R(t_1) given in subsection 2.6 of the document referenced above:
138137
rate = increment / expHelper(time - startTime);
138+
waitingForFirstIncrement = false;
139139
} else {
140140
if (time < lastTime) {
141141
time = lastTime;

server/src/test/java/org/elasticsearch/common/metrics/ExponentiallyWeightedMovingRateTests.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,6 @@ public void testEwmr_negativeLambdaThrowsOnConstruction() {
214214
assertThrows(IllegalArgumentException.class, () -> new ExponentiallyWeightedMovingRate(-1.0e-6, START_TIME_IN_MILLIS));
215215
}
216216

217-
public void testEwmr_zeroStartTimeInMillis() {
218-
assertThrows(IllegalArgumentException.class, () -> new ExponentiallyWeightedMovingRate(LAMBDA, 0));
219-
}
220-
221-
public void testEwmr_negativeStartTimeInMillis() {
222-
assertThrows(IllegalArgumentException.class, () -> new ExponentiallyWeightedMovingRate(LAMBDA, -1));
223-
}
224-
225217
// N.B. This test is not guaranteed to fail even if the implementation is not thread-safe. The operations are fast enough that there is
226218
// a chance each thread will complete before the next one has started. We use a high thread count to try to get a decent change of
227219
// hitting a race condition if there is one. This should be run with e.g. -Dtests.iters=20 to test thoroughly.

0 commit comments

Comments
 (0)