zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-2641: AvgRequestLatency metric improves to be more accurate
Date Mon, 07 Jan 2019 13:39:29 GMT
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 <maoling199210191@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 <maoling199210191@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();


Mime
View raw message