hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From xu...@apache.org
Subject [24/61] [abbrv] hive git commit: HIVE-10944 : Fix HS2 for Metrics (Szehon, reviewed by Sergey Shelukhin and Lenni Kuff)
Date Sun, 21 Jun 2015 05:25:49 GMT
HIVE-10944 : Fix HS2 for Metrics (Szehon, reviewed by Sergey Shelukhin and Lenni Kuff)


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

Branch: refs/heads/spark
Commit: 5fd11679a6184ec5abdc9be00b328c310ddfe62c
Parents: 97802f7
Author: Szehon Ho <szehon@cloudera.com>
Authored: Fri Jun 12 16:32:26 2015 -0700
Committer: Szehon Ho <szehon@cloudera.com>
Committed: Fri Jun 12 16:33:38 2015 -0700

----------------------------------------------------------------------
 .../hadoop/hive/common/JvmPauseMonitor.java     | 12 +++--
 .../hive/common/metrics/LegacyMetrics.java      | 51 ++++----------------
 .../hive/common/metrics/common/Metrics.java     |  8 +--
 .../common/metrics/common/MetricsFactory.java   | 30 +++++++++---
 .../metrics/metrics2/CodahaleMetrics.java       | 41 +++++-----------
 .../org/apache/hadoop/hive/conf/HiveConf.java   |  2 +-
 .../hive/common/metrics/TestLegacyMetrics.java  |  6 +--
 .../metrics/metrics2/TestCodahaleMetrics.java   | 16 +++---
 .../hadoop/hive/metastore/HiveMetaStore.java    | 10 ++--
 .../apache/hive/service/server/HiveServer2.java |  6 +--
 10 files changed, 72 insertions(+), 110 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java b/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
index c3949f2..ec5ac4a 100644
--- a/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
+++ b/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java
@@ -26,6 +26,7 @@ import com.google.common.collect.Sets;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.common.metrics.common.Metrics;
 import org.apache.hadoop.hive.common.metrics.common.MetricsFactory;
 import org.apache.hadoop.util.Daemon;
 
@@ -199,10 +200,13 @@ public class JvmPauseMonitor {
     }
 
     private void incrementMetricsCounter(String name, long count) {
-      try {
-        MetricsFactory.getMetricsInstance().incrementCounter(name, count);
-      } catch (Exception e) {
-        LOG.warn("Error Reporting JvmPauseMonitor to Metrics system", e);
+      Metrics metrics = MetricsFactory.getInstance();
+      if (metrics != null) {
+        try {
+          metrics.incrementCounter(name, count);
+        } catch (Exception e) {
+          LOG.warn("Error Reporting JvmPauseMonitor to Metrics system", e);
+        }
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java b/common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java
index 14f7afb..e811339 100644
--- a/common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java
+++ b/common/src/java/org/apache/hadoop/hive/common/metrics/LegacyMetrics.java
@@ -149,7 +149,6 @@ public class LegacyMetrics implements Metrics {
     }
   }
 
-
   private static final ThreadLocal<HashMap<String, MetricsScope>> threadLocalScopes
     = new ThreadLocal<HashMap<String,MetricsScope>>() {
     @Override
@@ -158,31 +157,16 @@ public class LegacyMetrics implements Metrics {
     }
   };
 
-  private boolean initialized = false;
-
-  public void init(HiveConf conf) throws Exception {
-    if (!initialized) {
-      MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
-      mbs.registerMBean(metrics, oname);
-      initialized = true;
-    }
-  }
-
-  public boolean isInitialized() {
-    return initialized;
+  public LegacyMetrics(HiveConf conf) throws Exception {
+    MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+    mbs.registerMBean(metrics, oname);
   }
 
   public Long incrementCounter(String name) throws IOException{
-    if (!initialized) {
-      return null;
-    }
     return incrementCounter(name,Long.valueOf(1));
   }
 
   public Long incrementCounter(String name, long increment) throws IOException{
-    if (!initialized) {
-      return null;
-    }
     Long value;
     synchronized(metrics) {
       if (!metrics.hasKey(name)) {
@@ -197,23 +181,14 @@ public class LegacyMetrics implements Metrics {
   }
 
   public void set(String name, Object value) throws IOException{
-    if (!initialized) {
-      return;
-    }
     metrics.put(name,value);
   }
 
   public Object get(String name) throws IOException{
-    if (!initialized) {
-      return null;
-    }
     return metrics.get(name);
   }
 
   public void startScope(String name) throws IOException{
-    if (!initialized) {
-      return;
-    }
     if (threadLocalScopes.get().containsKey(name)) {
       threadLocalScopes.get().get(name).open();
     } else {
@@ -222,9 +197,6 @@ public class LegacyMetrics implements Metrics {
   }
 
   public MetricsScope getScope(String name) throws IOException {
-    if (!initialized) {
-      return null;
-    }
     if (threadLocalScopes.get().containsKey(name)) {
       return threadLocalScopes.get().get(name);
     } else {
@@ -233,9 +205,6 @@ public class LegacyMetrics implements Metrics {
   }
 
   public void endScope(String name) throws IOException{
-    if (!initialized) {
-      return;
-    }
     if (threadLocalScopes.get().containsKey(name)) {
       threadLocalScopes.get().get(name).close();
     }
@@ -247,16 +216,14 @@ public class LegacyMetrics implements Metrics {
    *
    * Note that threadLocalScopes ThreadLocal is *not* cleared in this call.
    */
-  public void deInit() throws Exception {
+  public void close() throws Exception {
     synchronized (metrics) {
-      if (initialized) {
-        MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
-        if (mbs.isRegistered(oname)) {
-          mbs.unregisterMBean(oname);
-        }
-        metrics.clear();
-        initialized = false;
+      MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+      if (mbs.isRegistered(oname)) {
+        mbs.unregisterMBean(oname);
       }
+      metrics.clear();
+      threadLocalScopes.remove();
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java b/common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java
index 13a5336..27b69cc 100644
--- a/common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java
+++ b/common/src/java/org/apache/hadoop/hive/common/metrics/common/Metrics.java
@@ -28,16 +28,12 @@ import java.io.IOException;
  */
 public interface Metrics {
 
-  /**
-   * Initialize Metrics system with given Hive configuration.
-   * @param conf
-   */
-  public void init(HiveConf conf) throws Exception;
+  //Must declare CTOR taking in HiveConf.
 
   /**
    * Deinitializes the Metrics system.
    */
-  public void deInit() throws Exception;
+  public void close() throws Exception;
 
   /**
    * @param name

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
b/common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
index 12a309d..8769d68 100644
--- a/common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
+++ b/common/src/java/org/apache/hadoop/hive/common/metrics/common/MetricsFactory.java
@@ -20,29 +20,43 @@ package org.apache.hadoop.hive.common.metrics.common;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.util.ReflectionUtils;
 
+import java.lang.reflect.Constructor;
+
 /**
  * Class that manages a static Metric instance for this process.
  */
 public class MetricsFactory {
 
-  private static Metrics metrics;
-  private static Object initLock = new Object();
+  //Volatile ensures that static access returns Metrics instance in fully-initialized state.
+  //Alternative is to synchronize static access, which has performance penalties.
+  private volatile static Metrics metrics;
 
+  /**
+   * Initializes static Metrics instance.
+   */
   public synchronized static void init(HiveConf conf) throws Exception {
     if (metrics == null) {
-      metrics = (Metrics) ReflectionUtils.newInstance(conf.getClassByName(
-        conf.getVar(HiveConf.ConfVars.HIVE_METRICS_CLASS)), conf);
+      Class metricsClass = conf.getClassByName(
+        conf.getVar(HiveConf.ConfVars.HIVE_METRICS_CLASS));
+      Constructor constructor = metricsClass.getConstructor(HiveConf.class);
+      metrics = (Metrics) constructor.newInstance(conf);
     }
-    metrics.init(conf);
   }
 
-  public synchronized static Metrics getMetricsInstance() {
+  /**
+   * Returns static Metrics instance, null if not initialized or closed.
+   */
+  public static Metrics getInstance() {
     return metrics;
   }
 
-  public synchronized static void deInit() throws Exception {
+  /**
+   * Closes and removes static Metrics instance.
+   */
+  public synchronized static void close() throws Exception {
     if (metrics != null) {
-      metrics.deInit();
+      metrics.close();
+      metrics = null;
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
b/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
index e59da99..ae353d0 100644
--- a/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
+++ b/common/src/java/org/apache/hadoop/hive/common/metrics/metrics2/CodahaleMetrics.java
@@ -77,7 +77,6 @@ public class CodahaleMetrics implements org.apache.hadoop.hive.common.metrics.co
   private LoadingCache<String, Timer> timers;
   private LoadingCache<String, Counter> counters;
 
-  private boolean initialized = false;
   private HiveConf conf;
   private final Set<Closeable> reporters = new HashSet<Closeable>();
 
@@ -139,11 +138,7 @@ public class CodahaleMetrics implements org.apache.hadoop.hive.common.metrics.co
     }
   }
 
-  public synchronized void init(HiveConf conf) throws Exception {
-    if (initialized) {
-      return;
-    }
-
+  public CodahaleMetrics(HiveConf conf) throws Exception {
     this.conf = conf;
     //Codahale artifacts are lazily-created.
     timers = CacheBuilder.newBuilder().build(
@@ -190,32 +185,23 @@ public class CodahaleMetrics implements org.apache.hadoop.hive.common.metrics.co
       }
     }
     initReporting(finalReporterList);
-    initialized = true;
   }
 
 
-  public synchronized void deInit() throws Exception {
-    if (initialized) {
-      if (reporters != null) {
-        for (Closeable reporter : reporters) {
-          reporter.close();
-        }
+  public void close() throws Exception {
+    if (reporters != null) {
+      for (Closeable reporter : reporters) {
+        reporter.close();
       }
-      for (Map.Entry<String, Metric> metric : metricRegistry.getMetrics().entrySet())
{
-        metricRegistry.remove(metric.getKey());
-      }
-      timers.invalidateAll();
-      counters.invalidateAll();
-      initialized = false;
     }
+    for (Map.Entry<String, Metric> metric : metricRegistry.getMetrics().entrySet())
{
+      metricRegistry.remove(metric.getKey());
+    }
+    timers.invalidateAll();
+    counters.invalidateAll();
   }
 
   public void startScope(String name) throws IOException {
-    synchronized (this) {
-      if (!initialized) {
-        return;
-      }
-    }
     name = API_PREFIX + name;
     if (threadLocalScopes.get().containsKey(name)) {
       threadLocalScopes.get().get(name).open();
@@ -224,12 +210,7 @@ public class CodahaleMetrics implements org.apache.hadoop.hive.common.metrics.co
     }
   }
 
-  public void endScope(String name) throws IOException{
-    synchronized (this) {
-      if (!initialized) {
-        return;
-      }
-    }
+  public void endScope(String name) throws IOException {
     name = API_PREFIX + name;
     if (threadLocalScopes.get().containsKey(name)) {
       threadLocalScopes.get().get(name).close();

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index 4e36bae..27f68df 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -1725,7 +1725,7 @@ public class HiveConf extends Configuration {
         "Hive metrics subsystem implementation class."),
     HIVE_METRICS_REPORTER("hive.service.metrics.reporter", "JSON_FILE, JMX",
         "Reporter type for metric class org.apache.hadoop.hive.common.metrics.metrics2.CodahaleMetrics,
comma separated list of JMX, CONSOLE, JSON_FILE"),
-    HIVE_METRICS_JSON_FILE_LOCATION("hive.service.metrics.file.location", "file:///tmp/my-logging.properties",
+    HIVE_METRICS_JSON_FILE_LOCATION("hive.service.metrics.file.location", "file:///tmp/report.json",
         "For metric class org.apache.hadoop.hive.common.metrics.metrics2.CodahaleMetrics
JSON_FILE reporter, the location of JSON metrics file.  " +
         "This file will get overwritten at every interval."),
     HIVE_METRICS_JSON_FILE_INTERVAL("hive.service.metrics.file.frequency", "5s",

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java
----------------------------------------------------------------------
diff --git a/common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java
b/common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java
index c14c7ee..c3e8282 100644
--- a/common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java
+++ b/common/src/test/org/apache/hadoop/hive/common/metrics/TestLegacyMetrics.java
@@ -47,16 +47,16 @@ public class TestLegacyMetrics {
 
   @Before
   public void before() throws Exception {
-    MetricsFactory.deInit();
+    MetricsFactory.close();
     HiveConf conf = new HiveConf();
     conf.setVar(HiveConf.ConfVars.HIVE_METRICS_CLASS, LegacyMetrics.class.getCanonicalName());
     MetricsFactory.init(conf);
-    metrics = (LegacyMetrics) MetricsFactory.getMetricsInstance();
+    metrics = (LegacyMetrics) MetricsFactory.getInstance();
   }
 
   @After
   public void after() throws Exception {
-    MetricsFactory.deInit();
+    MetricsFactory.close();
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
----------------------------------------------------------------------
diff --git a/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
b/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
index 8749349..954b388 100644
--- a/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
+++ b/common/src/test/org/apache/hadoop/hive/common/metrics/metrics2/TestCodahaleMetrics.java
@@ -63,20 +63,20 @@ public class TestCodahaleMetrics {
     conf.setVar(HiveConf.ConfVars.HIVE_METRICS_JSON_FILE_INTERVAL, "100ms");
 
     MetricsFactory.init(conf);
-    metricRegistry = ((CodahaleMetrics) MetricsFactory.getMetricsInstance()).getMetricRegistry();
+    metricRegistry = ((CodahaleMetrics) MetricsFactory.getInstance()).getMetricRegistry();
   }
 
   @After
   public void after() throws Exception {
-    MetricsFactory.deInit();
+    MetricsFactory.close();
   }
 
   @Test
   public void testScope() throws Exception {
     int runs = 5;
     for (int i = 0; i < runs; i++) {
-      MetricsFactory.getMetricsInstance().startScope("method1");
-      MetricsFactory.getMetricsInstance().endScope("method1");
+      MetricsFactory.getInstance().startScope("method1");
+      MetricsFactory.getInstance().endScope("method1");
     }
 
     Timer timer = metricRegistry.getTimers().get("api_method1");
@@ -89,7 +89,7 @@ public class TestCodahaleMetrics {
   public void testCount() throws Exception {
     int runs = 5;
     for (int i = 0; i < runs; i++) {
-      MetricsFactory.getMetricsInstance().incrementCounter("count1");
+      MetricsFactory.getInstance().incrementCounter("count1");
     }
     Counter counter = metricRegistry.getCounters().get("count1");
     Assert.assertEquals(5L, counter.getCount());
@@ -104,8 +104,8 @@ public class TestCodahaleMetrics {
       executorService.submit(new Callable<Void>() {
         @Override
         public Void call() throws Exception {
-          MetricsFactory.getMetricsInstance().startScope("method2");
-          MetricsFactory.getMetricsInstance().endScope("method2");
+          MetricsFactory.getInstance().startScope("method2");
+          MetricsFactory.getInstance().endScope("method2");
           return null;
         }
       });
@@ -121,7 +121,7 @@ public class TestCodahaleMetrics {
   public void testFileReporting() throws Exception {
     int runs = 5;
     for (int i = 0; i < runs; i++) {
-      MetricsFactory.getMetricsInstance().incrementCounter("count2");
+      MetricsFactory.getInstance().incrementCounter("count2");
       Thread.sleep(100);
     }
 

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 914f2e7..ae9c119 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -748,9 +748,9 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       incrementCounter(function);
       logInfo((getIpAddress() == null ? "" : "source:" + getIpAddress() + " ") +
           function + extraLogInfo);
-      if (hiveConf.getBoolVar(ConfVars.METASTORE_METRICS)) {
+      if (MetricsFactory.getInstance() != null) {
         try {
-          MetricsFactory.getMetricsInstance().startScope(function);
+          MetricsFactory.getInstance().startScope(function);
         } catch (IOException e) {
           LOG.debug("Exception when starting metrics scope"
             + e.getClass().getName() + " " + e.getMessage(), e);
@@ -792,9 +792,9 @@ public class HiveMetaStore extends ThriftHiveMetastore {
     }
 
     private void endFunction(String function, MetaStoreEndFunctionContext context) {
-      if (hiveConf.getBoolVar(ConfVars.METASTORE_METRICS)) {
+      if (MetricsFactory.getInstance() != null) {
         try {
-          MetricsFactory.getMetricsInstance().endScope(function);
+          MetricsFactory.getInstance().endScope(function);
         } catch (IOException e) {
           LOG.debug("Exception when closing metrics scope" + e);
         }
@@ -823,7 +823,7 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       }
       if (hiveConf.getBoolVar(ConfVars.METASTORE_METRICS)) {
         try {
-          MetricsFactory.deInit();
+          MetricsFactory.close();
         } catch (Exception e) {
           LOG.error("error in Metrics deinit: " + e.getClass().getName() + " "
             + e.getMessage(), e);

http://git-wip-us.apache.org/repos/asf/hive/blob/5fd11679/service/src/java/org/apache/hive/service/server/HiveServer2.java
----------------------------------------------------------------------
diff --git a/service/src/java/org/apache/hive/service/server/HiveServer2.java b/service/src/java/org/apache/hive/service/server/HiveServer2.java
index 0959cba..00ab75f 100644
--- a/service/src/java/org/apache/hive/service/server/HiveServer2.java
+++ b/service/src/java/org/apache/hive/service/server/HiveServer2.java
@@ -308,9 +308,9 @@ public class HiveServer2 extends CompositeService {
     HiveConf hiveConf = this.getHiveConf();
     super.stop();
     // Shutdown Metrics
-    if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_METRICS_ENABLED)) {
+    if (MetricsFactory.getInstance() != null) {
       try {
-        MetricsFactory.getMetricsInstance().deInit();
+        MetricsFactory.close();
       } catch (Exception e) {
         LOG.error("error in Metrics deinit: " + e.getClass().getName() + " "
           + e.getMessage(), e);
@@ -359,7 +359,7 @@ public class HiveServer2 extends CompositeService {
         server.start();
 
         if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_METRICS_ENABLED)) {
-          MetricsFactory.getMetricsInstance().init(hiveConf);
+          MetricsFactory.init(hiveConf);
         }
         try {
           JvmPauseMonitor pauseMonitor = new JvmPauseMonitor(hiveConf);


Mime
View raw message