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..4de3bb9 100644 --- a/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java +++ b/common/src/java/org/apache/hadoop/hive/common/JvmPauseMonitor.java @@ -199,10 +199,12 @@ public void run() { } 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); + if (MetricsFactory.getMetricsInstance() != null) { + try { + MetricsFactory.getMetricsInstance().incrementCounter(name, count); + } catch (Exception e) { + LOG.warn("Error Reporting JvmPauseMonitor to Metrics system", e); + } } } } 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..9ef1676 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 void reopen() throws IOException { } } - private static final ThreadLocal> threadLocalScopes = new ThreadLocal>() { @Override @@ -158,31 +157,16 @@ public void reopen() throws IOException { } }; - 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 Long incrementCounter(String name, long increment) throws IOException{ } 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 void startScope(String name) throws IOException{ } 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 MetricsScope getScope(String name) throws IOException { } public void endScope(String name) throws IOException{ - if (!initialized) { - return; - } if (threadLocalScopes.get().containsKey(name)) { threadLocalScopes.get().get(name).close(); } @@ -249,14 +218,12 @@ public void endScope(String name) throws IOException{ */ public void deInit() 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(); } } } 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..c089887 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 @@ -25,14 +25,11 @@ /** * Generic Metics interface. + * Implementations do not have to be MT-safe, that is taken care of by the MetricsFactory. */ 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. 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..0504860 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,32 @@ 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(); + private volatile static Metrics metrics; 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() { + public static Metrics getMetricsInstance() { return metrics; } public synchronized static void deInit() throws Exception { if (metrics != null) { metrics.deInit(); + metrics = null; } } } 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..a2d7d31 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 @@ private LoadingCache timers; private LoadingCache counters; - private boolean initialized = false; private HiveConf conf; private final Set reporters = new HashSet(); @@ -139,11 +138,7 @@ public void close() throws IOException { } } - 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 Counter load(String key) throws Exception { } } initReporting(finalReporterList); - initialized = true; } - public synchronized void deInit() throws Exception { - if (initialized) { - if (reporters != null) { - for (Closeable reporter : reporters) { - reporter.close(); - } + public void deInit() throws Exception { + if (reporters != null) { + for (Closeable reporter : reporters) { + reporter.close(); } - for (Map.Entry metric : metricRegistry.getMetrics().entrySet()) { - metricRegistry.remove(metric.getKey()); - } - timers.invalidateAll(); - counters.invalidateAll(); - initialized = false; } + for (Map.Entry 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 void startScope(String name) throws IOException { } } - 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(); 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 85a734c..d407df3 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java @@ -748,7 +748,7 @@ private String startFunction(String function, String extraLogInfo) { incrementCounter(function); logInfo((getIpAddress() == null ? "" : "source:" + getIpAddress() + " ") + function + extraLogInfo); - if (hiveConf.getBoolVar(ConfVars.METASTORE_METRICS)) { + if (MetricsFactory.getMetricsInstance() != null) { try { MetricsFactory.getMetricsInstance().startScope(function); } catch (IOException e) { @@ -792,7 +792,7 @@ private void endFunction(String function, boolean successful, Exception e, } private void endFunction(String function, MetaStoreEndFunctionContext context) { - if (hiveConf.getBoolVar(ConfVars.METASTORE_METRICS)) { + if (MetricsFactory.getMetricsInstance() != null) { try { MetricsFactory.getMetricsInstance().endScope(function); } catch (IOException e) { 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 7820ed5..3de623b 100644 --- a/service/src/java/org/apache/hive/service/server/HiveServer2.java +++ b/service/src/java/org/apache/hive/service/server/HiveServer2.java @@ -307,9 +307,9 @@ public synchronized void stop() { HiveConf hiveConf = this.getHiveConf(); super.stop(); // Shutdown Metrics - if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_METRICS_ENABLED)) { + if (MetricsFactory.getMetricsInstance() != null) { try { - MetricsFactory.getMetricsInstance().deInit(); + MetricsFactory.deInit(); } catch (Exception e) { LOG.error("error in Metrics deinit: " + e.getClass().getName() + " " + e.getMessage(), e); @@ -355,8 +355,8 @@ private static void startHiveServer2() throws Throwable { server.init(hiveConf); server.start(); - if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_METRICS_ENABLED)) { - MetricsFactory.getMetricsInstance().init(hiveConf); + if (hiveConf.getBoolVar(ConfVars.HIVE_SERVER2_METRICS_ENABLED) && MetricsFactory.getMetricsInstance() == null) { + MetricsFactory.init(hiveConf); } try { JvmPauseMonitor pauseMonitor = new JvmPauseMonitor(hiveConf);