From 9582b9b34daa9e6f0b25f7d6bbef85b5e2c6ad7d Mon Sep 17 00:00:00 2001
From: jxiang <jxiang@unknown>
Date: Mon, 24 Feb 2014 17:10:03 +0000
Subject: [PATCH] HBASE-10587 Master metrics clusterRequests is wrong

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1571354 13f79535-47bb-0310-9956-ffa450edef68
---
 .../apache/hadoop/hbase/master/HMaster.java   |  7 +++--
 .../hbase/master/TestMasterMetrics.java       | 29 ++++++++++++++-----
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index 0104c01965d..eae4040e7c6 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -1351,10 +1351,13 @@ MasterServices, Server {
       RpcController controller, RegionServerReportRequest request) throws ServiceException {
     try {
       ClusterStatusProtos.ServerLoad sl = request.getLoad();
-      this.serverManager.regionServerReport(ProtobufUtil.toServerName(request.getServer()), new ServerLoad(sl));
+      ServerName serverName = ProtobufUtil.toServerName(request.getServer());
+      ServerLoad oldLoad = serverManager.getLoad(serverName);
+      this.serverManager.regionServerReport(serverName, new ServerLoad(sl));
       if (sl != null && this.metricsMaster != null) {
         // Up our metrics.
-        this.metricsMaster.incrementRequests(sl.getTotalNumberOfRequests());
+        this.metricsMaster.incrementRequests(sl.getTotalNumberOfRequests()
+          - (oldLoad != null ? oldLoad.getTotalNumberOfRequests() : 0));
       }
     } catch (IOException ioe) {
       throw new ServiceException(ioe);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
index 0c648962f4e..e1c6e8295c9 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
@@ -19,14 +19,15 @@ package org.apache.hadoop.hbase.master;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CompatibilityFactory;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.MediumTests;
 import org.apache.hadoop.hbase.MiniHBaseCluster;
+import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.ClusterStatusProtos;
 import org.apache.hadoop.hbase.protobuf.generated.RegionServerStatusProtos;
-import org.apache.hadoop.hbase.regionserver.HRegionServer;
 import org.apache.hadoop.hbase.test.MetricsAssertHelper;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -49,6 +50,9 @@ public class TestMasterMetrics {
   public static void startCluster() throws Exception {
     LOG.info("Starting cluster");
     TEST_UTIL = new HBaseTestingUtility();
+    Configuration conf = TEST_UTIL.getConfiguration();
+    // Prevent region server report any load during the test
+    conf.setInt("hbase.regionserver.msginterval", 3000000);
     TEST_UTIL.startMiniCluster(1, 1);
     cluster = TEST_UTIL.getHBaseCluster();
     LOG.info("Waiting for active/ready master");
@@ -69,18 +73,30 @@ public class TestMasterMetrics {
     // sending fake request to master to see how metric value has changed
     RegionServerStatusProtos.RegionServerReportRequest.Builder request =
         RegionServerStatusProtos.RegionServerReportRequest.newBuilder();
-    HRegionServer rs = cluster.getRegionServer(0);
-    request.setServer(ProtobufUtil.toServerName(rs.getServerName()));
+    ServerName serverName = cluster.getRegionServer(0).getServerName();
+    request.setServer(ProtobufUtil.toServerName(serverName));
 
+    MetricsMasterSource masterSource = master.getMetrics().getMetricsSource();
     ClusterStatusProtos.ServerLoad sl = ClusterStatusProtos.ServerLoad.newBuilder()
                                            .setTotalNumberOfRequests(10000)
                                            .build();
-    master.getMetrics().getMetricsSource().init();
+    masterSource.init();
     request.setLoad(sl);
     master.regionServerReport(null, request.build());
 
-    metricsHelper.assertCounter("cluster_requests", 10000,
-        master.getMetrics().getMetricsSource());
+    metricsHelper.assertCounter("cluster_requests", 10000, masterSource);
+
+    sl = ClusterStatusProtos.ServerLoad.newBuilder()
+        .setTotalNumberOfRequests(15000)
+        .build();
+    request.setLoad(sl);
+    master.regionServerReport(null, request.build());
+
+    metricsHelper.assertCounter("cluster_requests", 15000, masterSource);
+
+    master.regionServerReport(null, request.build());
+
+    metricsHelper.assertCounter("cluster_requests", 15000, masterSource);
     master.stopMaster();
   }
 
@@ -98,6 +114,5 @@ public class TestMasterMetrics {
     metricsHelper.assertTag("serverName", master.getServerName().toString(), masterSource);
     metricsHelper.assertTag("clusterId", master.getClusterId(), masterSource);
     metricsHelper.assertTag("zookeeperQuorum", master.getZooKeeper().getQuorum(), masterSource);
-
   }
 }