Ethanlm commented on a change in pull request #3323:
URL: https://github.com/apache/storm/pull/3323#discussion_r477432315
##########
File path:
storm-client/src/jvm/org/apache/storm/metrics2/RollingAverageGauge.java
##########
@@ -14,36 +14,22 @@
import com.codahale.metrics.Gauge;
-public class ResettingAverageGauge implements Gauge<Long> {
- private long total = 0L;
- private long samples = 0L;
+public class RollingAverageGauge implements Gauge<Double> {
+ private long[] samples = new long[3];
+ private int index = 0;
- public ResettingAverageGauge() {
- }
-
- public void addValue(long value) {
+ @Override
+ public Double getValue() {
synchronized (this) {
- total++;
- samples += value;
+ long total = samples[0] + samples[1] + samples[2];
Review comment:
At beginning, the values are all 0. So it is not accurate if there are
not enough samples (less than 3).
For example, if there is only one value, x, the result will be `x / 3.0`,
while the result should have been `x`.
But this is not very likely to happen since number of values will quickly
increase to more than 3. I think it is acceptable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
