You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by an...@apache.org on 2019/01/07 13:39:29 UTC
[zookeeper] branch master updated: ZOOKEEPER-2641:
AvgRequestLatency metric improves to be more accurate
This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new bc99248 ZOOKEEPER-2641: AvgRequestLatency metric improves to be more accurate
bc99248 is described below
commit bc992480ec938a3fad4b90f75a52dd186e1b968a
Author: maoling <ma...@sina.com>
AuthorDate: Mon Jan 7 14:39:13 2019 +0100
ZOOKEEPER-2641: AvgRequestLatency metric improves to be more accurate
- some original review historys were included [here ](https://github.com/apache/zookeeper/pull/629)
- more details in [ZOOKEEPER-2641](https://issues.apache.org/jira/browse/ZOOKEEPER-2641)
Author: maoling <ma...@sina.com>
Reviewers: andor@apache.org
Closes #748 from maoling/ZOOKEEPER-2641 and squashes the following commits:
e0d4fc890 [maoling] fix the flaky test in the FourLetterWordsTest.testValidateStatOutput
1739dbf1c [maoling] fix the flaky test in the CommandsTest.testMonitor
01af4002e [maoling] ZOOKEEPER-2641:AvgRequestLatency metric improves to be more accurate
---
.../src/main/resources/markdown/zookeeperAdmin.md | 2 +-
.../java/org/apache/zookeeper/server/ServerMetrics.java | 6 +++---
.../main/java/org/apache/zookeeper/server/ServerStats.java | 2 +-
.../org/apache/zookeeper/server/ZooKeeperServerBean.java | 2 +-
.../org/apache/zookeeper/server/ZooKeeperServerMXBean.java | 2 +-
.../org/apache/zookeeper/server/command/MonitorCommand.java | 4 ++++
.../apache/zookeeper/server/metric/AvgMinMaxCounter.java | 13 ++++++++-----
.../java/org/apache/zookeeper/server/metric/Metric.java | 2 +-
.../org/apache/zookeeper/server/metric/SimpleCounter.java | 4 ++--
.../java/org/apache/zookeeper/server/ServerMetricsTest.java | 10 +++++-----
.../java/org/apache/zookeeper/server/ServerStatsTest.java | 6 ++----
.../org/apache/zookeeper/server/admin/CommandsTest.java | 8 ++++++--
.../java/org/apache/zookeeper/test/FourLetterWordsTest.java | 2 +-
13 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 0e92cf9..2b68e38 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1270,7 +1270,7 @@ Moving forward, Four Letter Words will be deprecated, please use
$ echo mntr | nc localhost 2185
zk_version 3.4.0
- zk_avg_latency 0
+ zk_avg_latency 0.7561 - be account to four decimal places
zk_max_latency 0
zk_min_latency 0
zk_packets_received 70
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
index 203d0f6..c5d82de 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
@@ -83,12 +83,12 @@ public enum ServerMetrics {
metric.reset();
}
- Map<String, Long> getValues() {
+ Map<String, Object> getValues() {
return metric.values();
}
- static public Map<String, Long> getAllValues() {
- LinkedHashMap<String, Long> m = new LinkedHashMap<>();
+ static public Map<String, Object> getAllValues() {
+ LinkedHashMap<String, Object> m = new LinkedHashMap<>();
for (ServerMetrics metric : ServerMetrics.values()) {
m.putAll(metric.getValues());
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerStats.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerStats.java
index e52ef8e..bb5cbb4 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerStats.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerStats.java
@@ -64,7 +64,7 @@ public class ServerStats {
return requestLatency.getMin();
}
- public long getAvgLatency() {
+ public double getAvgLatency() {
return requestLatency.getAvg();
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerBean.java
index 6ac7602..cf84b2f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerBean.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerBean.java
@@ -59,7 +59,7 @@ public class ZooKeeperServerBean implements ZooKeeperServerMXBean, ZKMBeanInfo {
return Version.getFullVersion();
}
- public long getAvgRequestLatency() {
+ public double getAvgRequestLatency() {
return zks.serverStats().getAvgLatency();
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
index d7e50d3..feb6875 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java
@@ -41,7 +41,7 @@ public interface ZooKeeperServerMXBean {
/**
* @return average request latency in ms
*/
- public long getAvgRequestLatency();
+ public double getAvgRequestLatency();
/**
* @return max request latency in ms
*/
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/command/MonitorCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/command/MonitorCommand.java
index b89d557..e3ac230 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/command/MonitorCommand.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/command/MonitorCommand.java
@@ -84,6 +84,10 @@ public class MonitorCommand extends AbstractFourLetterCommand {
private void print(String key, long number) {
print(key, "" + number);
}
+
+ private void print(String key, double number) {
+ print(key, "" + number);
+ }
private void print(String key, String value) {
pw.print("zk_");
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
index 499c9a0..3029f94 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java
@@ -18,6 +18,7 @@
package org.apache.zookeeper.server.metric;
+import java.math.BigDecimal;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
@@ -36,7 +37,7 @@ public class AvgMinMaxCounter implements Metric {
public AvgMinMaxCounter(String name) {
this.name = name;
}
-
+
public void addDataPoint(long value) {
total.addAndGet(value);
count.incrementAndGet();
@@ -58,13 +59,15 @@ public class AvgMinMaxCounter implements Metric {
;
}
- public long getAvg() {
+ public double getAvg() {
// There is possible race-condition but we don't need the stats to be
// extremely accurate.
long currentCount = count.get();
long currentTotal = total.get();
if (currentCount > 0) {
- return currentTotal / currentCount;
+ double avgLatency = currentTotal / (double)currentCount;
+ BigDecimal bg = new BigDecimal(avgLatency);
+ return bg.setScale(4, BigDecimal.ROUND_HALF_UP).doubleValue();
}
return 0;
}
@@ -102,8 +105,8 @@ public class AvgMinMaxCounter implements Metric {
addDataPoint(value);
}
- public Map<String, Long> values() {
- Map<String, Long> m = new LinkedHashMap<String, Long>();
+ public Map<String, Object> values() {
+ Map<String, Object> m = new LinkedHashMap<String, Object>();
m.put("avg_" + name, this.getAvg());
m.put("min_" + name, this.getMin());
m.put("max_" + name, this.getMax());
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/Metric.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/Metric.java
index c475055..bc4d166 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/Metric.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/Metric.java
@@ -23,5 +23,5 @@ import java.util.Map;
public interface Metric {
void add(long value);
void reset();
- Map<String, Long> values();
+ Map<String, Object> values();
}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounter.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounter.java
index 4bf8046..7755247 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounter.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounter.java
@@ -45,8 +45,8 @@ public class SimpleCounter implements Metric {
}
@Override
- public Map<String, Long> values() {
- Map<String, Long> m = new LinkedHashMap<String, Long>();
+ public Map<String, Object> values() {
+ Map<String, Object> m = new LinkedHashMap<String, Object>();
m.put(name, this.getCount());
return m;
}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerMetricsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerMetricsTest.java
index 80f850b..8140724 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerMetricsTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerMetricsTest.java
@@ -65,19 +65,19 @@ public class ServerMetricsTest extends ZKTestCase {
long expectedMax = Arrays.stream(values).max().orElse(0);
long expectedSum = Arrays.stream(values).sum();
long expectedCnt = values.length;
- long expectedAvg = expectedSum / Math.max(1, expectedCnt);
+ double expectedAvg = expectedSum / Math.max(1, expectedCnt);
- Assert.assertEquals(expectedAvg, metric.getAvg());
+ Assert.assertEquals(expectedAvg, metric.getAvg(), (double)200);
Assert.assertEquals(expectedMin, metric.getMin());
Assert.assertEquals(expectedMax, metric.getMax());
Assert.assertEquals(expectedCnt, metric.getCount());
Assert.assertEquals(expectedSum, metric.getTotal());
- final Map<String, Long> results = metric.values();
+ final Map<String, Object> results = metric.values();
Assert.assertEquals(expectedMax, (long)results.get("max_test"));
Assert.assertEquals(expectedMin, (long)results.get("min_test"));
Assert.assertEquals(expectedCnt, (long)results.get("cnt_test"));
- Assert.assertEquals(expectedAvg, (long)results.get("avg_test"));
+ Assert.assertEquals(expectedAvg, (double)results.get("avg_test"), (double)200);
metric.reset();
}
@@ -101,7 +101,7 @@ public class ServerMetricsTest extends ZKTestCase {
long expectedCount = Arrays.stream(values).sum();
Assert.assertEquals(expectedCount, metric.getCount());
- final Map<String, Long> results = metric.values();
+ final Map<String, Object> results = metric.values();
Assert.assertEquals(expectedCount, (long)results.get("test"));
metric.reset();
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerStatsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerStatsTest.java
index d28dc8c..357b73e 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerStatsTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerStatsTest.java
@@ -19,7 +19,6 @@
package org.apache.zookeeper.server;
import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.common.Time;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -77,8 +76,7 @@ public class ServerStatsTest extends ZKTestCase {
lessThanOrEqualTo(serverStats.getMaxLatency()));
assertThat("Min latency check", 1000L,
lessThanOrEqualTo(serverStats.getMinLatency()));
- assertThat("Avg latency check", 1500L,
- lessThanOrEqualTo(serverStats.getAvgLatency()));
+ Assert.assertEquals((double)1500, serverStats.getAvgLatency(), (double)200);
// When reset...
serverStats.resetLatency();
@@ -138,7 +136,7 @@ public class ServerStatsTest extends ZKTestCase {
private void assertAllLatencyZero(ServerStats serverStats) {
Assert.assertEquals(0L, serverStats.getMaxLatency());
Assert.assertEquals(0L, serverStats.getMinLatency());
- Assert.assertEquals(0L, serverStats.getAvgLatency());
+ Assert.assertEquals((double)0, serverStats.getAvgLatency(), (double)0.00001);
}
private void assertFsyncThresholdExceedCountZero(ServerStats serverStats) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
index fedbe0f..9b30c55 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
@@ -171,7 +171,7 @@ public class CommandsTest extends ClientBase {
public void testMonitor() throws IOException, InterruptedException {
ArrayList<Field> fields = new ArrayList<>(Arrays.asList(
new Field("version", String.class),
- new Field("avg_latency", Long.class),
+ new Field("avg_latency", Double.class),
new Field("max_latency", Long.class),
new Field("min_latency", Long.class),
new Field("packets_received", Long.class),
@@ -193,7 +193,11 @@ public class CommandsTest extends ClientBase {
new Field("local_sessions", Long.class)
));
for (String metric : ServerMetrics.getAllValues().keySet()) {
- fields.add(new Field(metric, Long.class));
+ if (metric.startsWith("avg_")) {
+ fields.add(new Field(metric, Double.class));
+ } else {
+ fields.add(new Field(metric, Long.class));
+ }
}
Field fieldsArray[] = fields.toArray(new Field[0]);
testCommand("monitor", fieldsArray);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java
index ad71eab..8330da2 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java
@@ -153,7 +153,7 @@ public class FourLetterWordsTest extends ClientBase {
Assert.assertTrue(count >= 2);
line = in.readLine();
- Assert.assertTrue(Pattern.matches("^Latency min/avg/max: \\d+/\\d+/\\d+$", line));
+ Assert.assertTrue(Pattern.matches("^Latency min/avg/max: \\d+/-?[0-9]*.?[0-9]*/\\d+$", line));
line = in.readLine();
Assert.assertTrue(Pattern.matches("^Received: \\d+$", line));
line = in.readLine();