You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by GitBox <gi...@apache.org> on 2019/01/07 13:39:46 UTC
[GitHub] asfgit closed pull request #748: ZOOKEEPER-2641:AvgRequestLatency
metric improves to be more accurate
asfgit closed pull request #748: ZOOKEEPER-2641:AvgRequestLatency metric improves to be more accurate
URL: https://github.com/apache/zookeeper/pull/748
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 0e92cf9737..2b68e38268 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 203d0f6976..c5d82deebb 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 void reset() {
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 e52ef8e004..bb5cbb4b75 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 long getMinLatency() {
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 6ac76021b3..cf84b2f9e5 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 String getVersion() {
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 d7e50d3ccf..feb6875870 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 @@
/**
* @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 b89d557473..e3ac230cc7 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 void commandRun() {
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 499c9a0e2d..3029f9485c 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 AvgMinMaxCounter(String name) {
this.name = name;
}
-
+
public void addDataPoint(long value) {
total.addAndGet(value);
count.incrementAndGet();
@@ -58,13 +59,15 @@ private void setMin(long value) {
;
}
- 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 void add(long value) {
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 c475055d9c..bc4d1667fb 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 @@
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 4bf8046192..7755247d79 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 long getCount() {
}
@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 80f850b4c9..8140724623 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 @@ private void testAvgMinMaxCounter(AvgMinMaxCounter metric, int size) {
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 @@ private void testSimpleCounter(SimpleCounter metric, int size) {
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 d28dc8ca2f..357b73e18e 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 void testLatencyMetrics() {
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 @@ private void assertAllPacketsZero(ServerStats serverStats) {
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 fedbe0fe87..9b30c555dc 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 void testIsReadOnly() throws IOException, InterruptedException {
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 void testMonitor() throws IOException, InterruptedException {
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 ad71eabb3b..8330da23c6 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 void testValidateStatOutput() throws Exception {
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();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services