Repository: hbase
Updated Branches:
  refs/heads/master 278625312 -> c8e9a295c


HBASE-16855 Avoid NPE in MetricsConnection’s construction (ChiaPing Tsai)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/c8e9a295
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/c8e9a295
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/c8e9a295

Branch: refs/heads/master
Commit: c8e9a295c133ef9507a84ab9c70d18563e2c22ad
Parents: 2786253
Author: tedyu <yuzhih...@gmail.com>
Authored: Mon Oct 17 09:34:21 2016 -0700
Committer: tedyu <yuzhih...@gmail.com>
Committed: Mon Oct 17 09:34:21 2016 -0700

----------------------------------------------------------------------
 .../hadoop/hbase/client/MetricsConnection.java  | 31 ++++++++++++++++----
 .../hbase/client/TestMetricsConnection.java     | 24 +++++++++++----
 2 files changed, 44 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/c8e9a295/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
----------------------------------------------------------------------
diff --git 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
index 22a5561..36627bd 100644
--- 
a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
+++ 
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java
@@ -298,23 +298,29 @@ public class MetricsConnection implements 
StatisticTrackable {
   private final ConcurrentMap<String, Counter> cacheDroppingExceptions =
     new ConcurrentHashMap<>(CAPACITY, LOAD_FACTOR, CONCURRENCY_LEVEL);
 
-  public MetricsConnection(final ConnectionImplementation conn) {
+  MetricsConnection(final ConnectionImplementation conn) {
     this.scope = conn.toString();
     this.registry = new MetricRegistry();
-    final ThreadPoolExecutor batchPool = (ThreadPoolExecutor) 
conn.getCurrentBatchPool();
-    final ThreadPoolExecutor metaPool = (ThreadPoolExecutor) 
conn.getCurrentMetaLookupPool();
 
-    this.registry.register(name(this.getClass(), "executorPoolActiveThreads", 
scope),
+    this.registry.register(getExecutorPoolName(),
         new RatioGauge() {
           @Override
           protected Ratio getRatio() {
+            ThreadPoolExecutor batchPool = (ThreadPoolExecutor) 
conn.getCurrentBatchPool();
+            if (batchPool == null) {
+              return Ratio.of(0, 0);
+            }
             return Ratio.of(batchPool.getActiveCount(), 
batchPool.getMaximumPoolSize());
           }
         });
-    this.registry.register(name(this.getClass(), "metaPoolActiveThreads", 
scope),
+    this.registry.register(getMetaPoolName(),
         new RatioGauge() {
           @Override
           protected Ratio getRatio() {
+            ThreadPoolExecutor metaPool = (ThreadPoolExecutor) 
conn.getCurrentMetaLookupPool();
+            if (metaPool == null) {
+              return Ratio.of(0, 0);
+            }
             return Ratio.of(metaPool.getActiveCount(), 
metaPool.getMaximumPoolSize());
           }
         });
@@ -337,6 +343,21 @@ public class MetricsConnection implements 
StatisticTrackable {
     this.reporter.start();
   }
 
+  @VisibleForTesting
+  final String getExecutorPoolName() {
+    return name(getClass(), "executorPoolActiveThreads", scope);
+  }
+
+  @VisibleForTesting
+  final String getMetaPoolName() {
+    return name(getClass(), "metaPoolActiveThreads", scope);
+  }
+
+  @VisibleForTesting
+  MetricRegistry getMetricRegistry() {
+    return registry;
+  }
+
   public void shutdown() {
     this.reporter.stop();
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/c8e9a295/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
----------------------------------------------------------------------
diff --git 
a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
 
b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
index d17dd7f..854ecc5 100644
--- 
a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
+++ 
b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestMetricsConnection.java
@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hbase.client;
 
+import com.codahale.metrics.RatioGauge;
+import com.codahale.metrics.RatioGauge.Ratio;
 import org.apache.hadoop.hbase.shaded.com.google.protobuf.ByteString;
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import 
org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.ClientService;
@@ -32,24 +34,28 @@ import 
org.apache.hadoop.hbase.testclassification.MetricsTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.AfterClass;
-import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Mockito;
 
 import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicBoolean;
+import static org.junit.Assert.assertEquals;
 
 @Category({ClientTests.class, MetricsTests.class, SmallTests.class})
 public class TestMetricsConnection {
 
   private static MetricsConnection METRICS;
-
+  private static final ExecutorService BATCH_POOL = 
Executors.newFixedThreadPool(2);
   @BeforeClass
   public static void beforeClass() {
     ConnectionImplementation mocked = 
Mockito.mock(ConnectionImplementation.class);
     Mockito.when(mocked.toString()).thenReturn("mocked-connection");
-    METRICS = new 
MetricsConnection(Mockito.mock(ConnectionImplementation.class));
+    Mockito.when(mocked.getCurrentBatchPool()).thenReturn(BATCH_POOL);
+    METRICS = new MetricsConnection(mocked);
   }
 
   @AfterClass
@@ -112,9 +118,15 @@ public class TestMetricsConnection {
         METRICS.getTracker, METRICS.scanTracker, METRICS.multiTracker, 
METRICS.appendTracker,
         METRICS.deleteTracker, METRICS.incrementTracker, METRICS.putTracker
     }) {
-      Assert.assertEquals("Failed to invoke callTimer on " + t, loop, 
t.callTimer.getCount());
-      Assert.assertEquals("Failed to invoke reqHist on " + t, loop, 
t.reqHist.getCount());
-      Assert.assertEquals("Failed to invoke respHist on " + t, loop, 
t.respHist.getCount());
+      assertEquals("Failed to invoke callTimer on " + t, loop, 
t.callTimer.getCount());
+      assertEquals("Failed to invoke reqHist on " + t, loop, 
t.reqHist.getCount());
+      assertEquals("Failed to invoke respHist on " + t, loop, 
t.respHist.getCount());
     }
+    RatioGauge executorMetrics = (RatioGauge) METRICS.getMetricRegistry()
+            .getMetrics().get(METRICS.getExecutorPoolName());
+    RatioGauge metaMetrics = (RatioGauge) METRICS.getMetricRegistry()
+            .getMetrics().get(METRICS.getMetaPoolName());
+    assertEquals(Ratio.of(0, 3).getValue(), executorMetrics.getValue(), 0);
+    assertEquals(Double.NaN, metaMetrics.getValue(), 0);
   }
 }

Reply via email to