Переглянути джерело

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
maoling 6 роки тому
батько
коміт
bc992480ec

+ 1 - 1
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
     $ echo mntr | nc localhost 2185
                   zk_version  3.4.0
                   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_max_latency  0
                   zk_min_latency  0
                   zk_min_latency  0
                   zk_packets_received 70
                   zk_packets_received 70

+ 3 - 3
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java

@@ -83,12 +83,12 @@ public enum ServerMetrics {
         metric.reset();
         metric.reset();
     }
     }
 
 
-    Map<String, Long> getValues() {
+    Map<String, Object> getValues() {
         return metric.values();
         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()) {
         for (ServerMetrics metric : ServerMetrics.values()) {
             m.putAll(metric.getValues());
             m.putAll(metric.getValues());
         }
         }

+ 1 - 1
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerStats.java

@@ -64,7 +64,7 @@ public class ServerStats {
         return requestLatency.getMin();
         return requestLatency.getMin();
     }
     }
 
 
-    public long getAvgLatency() {
+    public double getAvgLatency() {
         return requestLatency.getAvg();
         return requestLatency.getAvg();
     }
     }
 
 

+ 1 - 1
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerBean.java

@@ -59,7 +59,7 @@ public class ZooKeeperServerBean implements ZooKeeperServerMXBean, ZKMBeanInfo {
         return Version.getFullVersion();
         return Version.getFullVersion();
     }
     }
     
     
-    public long getAvgRequestLatency() {
+    public double getAvgRequestLatency() {
         return zks.serverStats().getAvgLatency();
         return zks.serverStats().getAvgLatency();
     }
     }
     
     

+ 1 - 1
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMXBean.java

@@ -41,7 +41,7 @@ public interface ZooKeeperServerMXBean {
     /**
     /**
      * @return average request latency in ms
      * @return average request latency in ms
      */
      */
-    public long getAvgRequestLatency();
+    public double getAvgRequestLatency();
     /**
     /**
      * @return max request latency in ms
      * @return max request latency in ms
      */
      */

+ 4 - 0
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) {
     private void print(String key, long number) {
         print(key, "" + number);
         print(key, "" + number);
     }
     }
+    
+    private void print(String key, double number) {
+        print(key, "" + number);
+    }
 
 
     private void print(String key, String value) {
     private void print(String key, String value) {
         pw.print("zk_");
         pw.print("zk_");

+ 8 - 5
zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/AvgMinMaxCounter.java

@@ -18,6 +18,7 @@
 
 
 package org.apache.zookeeper.server.metric;
 package org.apache.zookeeper.server.metric;
 
 
+import java.math.BigDecimal;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicLong;
@@ -36,7 +37,7 @@ public class AvgMinMaxCounter implements Metric {
     public AvgMinMaxCounter(String name) {
     public AvgMinMaxCounter(String name) {
         this.name = name;
         this.name = name;
     }
     }
-
+    
     public void addDataPoint(long value) {
     public void addDataPoint(long value) {
         total.addAndGet(value);
         total.addAndGet(value);
         count.incrementAndGet();
         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
         // There is possible race-condition but we don't need the stats to be
         // extremely accurate.
         // extremely accurate.
         long currentCount = count.get();
         long currentCount = count.get();
         long currentTotal = total.get();
         long currentTotal = total.get();
         if (currentCount > 0) {
         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;
         return 0;
     }
     }
@@ -102,8 +105,8 @@ public class AvgMinMaxCounter implements Metric {
         addDataPoint(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("avg_" + name, this.getAvg());
         m.put("min_" + name, this.getMin());
         m.put("min_" + name, this.getMin());
         m.put("max_" + name, this.getMax());
         m.put("max_" + name, this.getMax());

+ 1 - 1
zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/Metric.java

@@ -23,5 +23,5 @@ import java.util.Map;
 public interface Metric {
 public interface Metric {
     void add(long value);
     void add(long value);
     void reset();
     void reset();
-    Map<String, Long> values();
+    Map<String, Object> values();
 }
 }

+ 2 - 2
zookeeper-server/src/main/java/org/apache/zookeeper/server/metric/SimpleCounter.java

@@ -45,8 +45,8 @@ public class SimpleCounter implements Metric {
     }
     }
 
 
     @Override
     @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());
         m.put(name, this.getCount());
         return m;
         return m;
     }
     }

+ 5 - 5
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 expectedMax = Arrays.stream(values).max().orElse(0);
         long expectedSum = Arrays.stream(values).sum();
         long expectedSum = Arrays.stream(values).sum();
         long expectedCnt = values.length;
         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(expectedMin, metric.getMin());
         Assert.assertEquals(expectedMax, metric.getMax());
         Assert.assertEquals(expectedMax, metric.getMax());
         Assert.assertEquals(expectedCnt, metric.getCount());
         Assert.assertEquals(expectedCnt, metric.getCount());
         Assert.assertEquals(expectedSum, metric.getTotal());
         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(expectedMax, (long)results.get("max_test"));
         Assert.assertEquals(expectedMin, (long)results.get("min_test"));
         Assert.assertEquals(expectedMin, (long)results.get("min_test"));
         Assert.assertEquals(expectedCnt, (long)results.get("cnt_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();
         metric.reset();
     }
     }
@@ -101,7 +101,7 @@ public class ServerMetricsTest extends ZKTestCase {
         long expectedCount = Arrays.stream(values).sum();
         long expectedCount = Arrays.stream(values).sum();
         Assert.assertEquals(expectedCount, metric.getCount());
         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"));
         Assert.assertEquals(expectedCount, (long)results.get("test"));
 
 
         metric.reset();
         metric.reset();

+ 2 - 4
zookeeper-server/src/test/java/org/apache/zookeeper/server/ServerStatsTest.java

@@ -19,7 +19,6 @@
 package org.apache.zookeeper.server;
 package org.apache.zookeeper.server;
 
 
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZKTestCase;
-import org.apache.zookeeper.common.Time;
 import org.junit.Assert;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Test;
@@ -77,8 +76,7 @@ public class ServerStatsTest extends ZKTestCase {
                 lessThanOrEqualTo(serverStats.getMaxLatency()));
                 lessThanOrEqualTo(serverStats.getMaxLatency()));
         assertThat("Min latency check", 1000L,
         assertThat("Min latency check", 1000L,
                 lessThanOrEqualTo(serverStats.getMinLatency()));
                 lessThanOrEqualTo(serverStats.getMinLatency()));
-        assertThat("Avg latency check", 1500L,
-                lessThanOrEqualTo(serverStats.getAvgLatency()));
+        Assert.assertEquals((double)1500, serverStats.getAvgLatency(), (double)200);
 
 
         // When reset...
         // When reset...
         serverStats.resetLatency();
         serverStats.resetLatency();
@@ -138,7 +136,7 @@ public class ServerStatsTest extends ZKTestCase {
     private void assertAllLatencyZero(ServerStats serverStats) {
     private void assertAllLatencyZero(ServerStats serverStats) {
         Assert.assertEquals(0L, serverStats.getMaxLatency());
         Assert.assertEquals(0L, serverStats.getMaxLatency());
         Assert.assertEquals(0L, serverStats.getMinLatency());
         Assert.assertEquals(0L, serverStats.getMinLatency());
-        Assert.assertEquals(0L, serverStats.getAvgLatency());
+        Assert.assertEquals((double)0, serverStats.getAvgLatency(), (double)0.00001);
     }
     }
 
 
     private void assertFsyncThresholdExceedCountZero(ServerStats serverStats) {
     private void assertFsyncThresholdExceedCountZero(ServerStats serverStats) {

+ 6 - 2
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 {
     public void testMonitor() throws IOException, InterruptedException {
         ArrayList<Field> fields = new ArrayList<>(Arrays.asList(
         ArrayList<Field> fields = new ArrayList<>(Arrays.asList(
                 new Field("version", String.class),
                 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("max_latency", Long.class),
                 new Field("min_latency", Long.class),
                 new Field("min_latency", Long.class),
                 new Field("packets_received", Long.class),
                 new Field("packets_received", Long.class),
@@ -193,7 +193,11 @@ public class CommandsTest extends ClientBase {
                 new Field("local_sessions", Long.class)
                 new Field("local_sessions", Long.class)
         ));
         ));
         for (String metric : ServerMetrics.getAllValues().keySet()) {
         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]);
         Field fieldsArray[] = fields.toArray(new Field[0]);
         testCommand("monitor", fieldsArray);
         testCommand("monitor", fieldsArray);

+ 1 - 1
zookeeper-server/src/test/java/org/apache/zookeeper/test/FourLetterWordsTest.java

@@ -153,7 +153,7 @@ public class FourLetterWordsTest extends ClientBase {
         Assert.assertTrue(count >= 2);
         Assert.assertTrue(count >= 2);
 
 
         line = in.readLine();
         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();
         line = in.readLine();
         Assert.assertTrue(Pattern.matches("^Received: \\d+$", line));
         Assert.assertTrue(Pattern.matches("^Received: \\d+$", line));
         line = in.readLine();
         line = in.readLine();