diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/impl/GlobalMetricRegistriesAdapter.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/impl/GlobalMetricRegistriesAdapter.java index 40a358fa2fcc..0f2f81ced9cf 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/impl/GlobalMetricRegistriesAdapter.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/impl/GlobalMetricRegistriesAdapter.java @@ -18,17 +18,13 @@ package org.apache.hadoop.hbase.metrics.impl; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.Collection; import java.util.HashMap; import java.util.Iterator; -import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.commons.lang.reflect.FieldUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.metrics.MetricRegistries; @@ -37,11 +33,8 @@ import org.apache.hadoop.metrics2.MetricsCollector; import org.apache.hadoop.metrics2.MetricsExecutor; import org.apache.hadoop.metrics2.MetricsSource; -import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.impl.JmxCacheBuster; -import org.apache.hadoop.metrics2.impl.MetricsSystemImpl; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; -import org.apache.hadoop.metrics2.lib.DefaultMetricsSystemHelper; import org.apache.hadoop.metrics2.lib.MetricsExecutorImpl; import com.google.common.annotations.VisibleForTesting; @@ -87,7 +80,6 @@ public void getMetrics(MetricsCollector collector, boolean all) { private final MetricsExecutor executor; private final AtomicBoolean stopped; - private final DefaultMetricsSystemHelper helper; private final HBaseMetrics2HadoopMetricsAdapter metricsAdapter; private final HashMap registeredSources; @@ -96,7 +88,6 @@ private GlobalMetricRegistriesAdapter() { this.stopped = new AtomicBoolean(false); this.metricsAdapter = new HBaseMetrics2HadoopMetricsAdapter(); this.registeredSources = new HashMap<>(); - this.helper = new DefaultMetricsSystemHelper(); executor.getExecutor().scheduleAtFixedRate(new Runnable(){ @Override public void run() { @@ -167,67 +158,13 @@ private void doRun() { if (LOG.isDebugEnabled()) { LOG.debug("Removing adapter for the MetricRegistry: " + info.getMetricsJmxContext()); } - synchronized(DefaultMetricsSystem.instance()) { - unregisterSource(info); - helper.removeSourceName(info.getMetricsJmxContext()); - helper.removeObjectName(info.getMetricsJmxContext()); - it.remove(); - removed = true; - } + DefaultMetricsSystem.instance().unregisterSource(info.getMetricsJmxContext()); + it.remove(); + removed = true; } } if (removed) { JmxCacheBuster.clearJmxCache(); } } - - /** - * Use reflection to unregister the Hadoop metric source, since MetricsSystem#unregisterSource() - * is only available in Hadoop 2.6+ (HADOOP-10839) - */ - @VisibleForTesting - protected void unregisterSource(MetricRegistryInfo info) { - // unregisterSource is only available in Hadoop 2.6+ (HADOOP-10839). Don't unregister for now - MetricsSystem metricsSystem = DefaultMetricsSystem.instance(); - if (metricsSystem instanceof MetricsSystemImpl) { - try { - // it's actually a Map , but MetricsSourceAdapter isn't - // accessible - @SuppressWarnings("unchecked") - Map sources = - (Map) FieldUtils.readField(metricsSystem, "sources", true); - String sourceName = info.getMetricsJmxContext(); - if (sources.containsKey(sourceName)) { - Object sourceAdapter = sources.get(sourceName); - Method method = null; - try { - method = sourceAdapter.getClass().getDeclaredMethod("stop"); - } catch (NoSuchMethodException e) { - LOG.info("Stop method not found on MetricsSourceAdapter"); - } catch (SecurityException e) { - LOG.info("Don't have access to call stop method not found on MetricsSourceAdapter", e); - } - if (method != null) { - method.setAccessible(true); - try { - method.invoke(sourceAdapter); - } catch (IllegalArgumentException | InvocationTargetException e) { - LOG.warn("Couldn't invoke stop on metrics source adapter: " + sourceName); - e.printStackTrace(); - } - } - sources.remove(sourceName); - - } - @SuppressWarnings("unchecked") - Map allSources = - (Map) FieldUtils.readField(metricsSystem, "allSources", true); - if (allSources.containsKey(sourceName)) { - allSources.remove(sourceName); - } - } catch (IllegalAccessException e) { - LOG.warn("Error unregistering metric source " + info.getMetricsJmxContext()); - } - } - } } diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystemHelper.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystemHelper.java index eb465c38c8b5..71da1d9ae5b3 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystemHelper.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib/DefaultMetricsSystemHelper.java @@ -17,9 +17,7 @@ */ package org.apache.hadoop.metrics2.lib; -import java.lang.reflect.Field; import java.lang.reflect.Method; -import java.util.HashMap; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -28,8 +26,6 @@ public class DefaultMetricsSystemHelper { private static final Log LOG = LogFactory.getLog(DefaultMetricsSystemHelper.class); private final Method removeObjectMethod; - private final Field sourceNamesField; - private final Field mapField; public DefaultMetricsSystemHelper() { Class clazz = DefaultMetricsSystem.INSTANCE.getClass(); @@ -41,20 +37,6 @@ public DefaultMetricsSystemHelper() { m = null; } removeObjectMethod = m; - - Field f1, f2; - try { - f1 = clazz.getDeclaredField("sourceNames"); - f1.setAccessible(true); - f2 = UniqueNames.class.getDeclaredField("map"); - f2.setAccessible(true); - } catch (NoSuchFieldException e) { - LOG.trace(e); - f1 = null; - f2 = null; - } - sourceNamesField = f1; - mapField = f2; } public boolean removeObjectName(final String name) { @@ -70,30 +52,4 @@ public boolean removeObjectName(final String name) { } return false; } - - /** - * Unfortunately Hadoop tries to be too-clever and permanently keeps track of all names registered - * so far as a Source, thus preventing further re-registration of the source with the same name. - * In case of dynamic metrics tied to region-lifecycles, this becomes a problem because we would - * like to be able to re-register and remove with the same name. Otherwise, it is resource leak. - * This ugly code manually removes the name from the UniqueNames map. - * TODO: May not be needed for Hadoop versions after YARN-5190. - */ - public void removeSourceName(String name) { - if (sourceNamesField == null || mapField == null) { - return; - } - try { - Object sourceNames = sourceNamesField.get(DefaultMetricsSystem.INSTANCE); - HashMap map = (HashMap) mapField.get(sourceNames); - synchronized (sourceNames) { - map.remove(name); - } - } catch (Exception ex) { - if (LOG.isTraceEnabled()) { - LOG.trace("Received exception while trying to access Hadoop Metrics classes via reflection.", - ex); - } - } - } } diff --git a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestGlobalMetricRegistriesAdapter.java b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestGlobalMetricRegistriesAdapter.java deleted file mode 100644 index e20a1f3ea362..000000000000 --- a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/metrics/impl/TestGlobalMetricRegistriesAdapter.java +++ /dev/null @@ -1,86 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.metrics.impl; - -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - -import org.apache.hadoop.hbase.metrics.MetricRegistryInfo; -import org.apache.hadoop.hbase.testclassification.MetricsTests; -import org.apache.hadoop.hbase.testclassification.SmallTests; -import org.apache.hadoop.metrics2.MetricsSource; -import org.apache.hadoop.metrics2.MetricsSystem; -import org.apache.hadoop.metrics2.annotation.Metric; -import org.apache.hadoop.metrics2.annotation.Metrics; -import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; -import org.apache.hadoop.metrics2.lib.MetricsRegistry; -import org.apache.hadoop.metrics2.lib.MutableCounterLong; -import org.apache.hadoop.metrics2.lib.MutableGaugeLong; -import org.apache.hadoop.metrics2.lib.MutableRate; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.mockito.Mockito; - -@Category({ MetricsTests.class, SmallTests.class }) -public class TestGlobalMetricRegistriesAdapter { - - /** - * Tests that using reflection to unregister the Hadoop metrics source works properly - */ - @Test - public void testUnregisterSource() { - GlobalMetricRegistriesAdapter adapter = GlobalMetricRegistriesAdapter.init(); - // we'll configure the sources manually, so disable the executor - adapter.stop(); - TestSource ts1 = new TestSource("ts1"); - TestSource ts2 = new TestSource("ts2"); - MetricsSystem metricsSystem = DefaultMetricsSystem.instance(); - metricsSystem.register("ts1", "", ts1); - metricsSystem.register("ts2", "", ts2); - MetricsSource s1 = metricsSystem.getSource("ts1"); - assertNotNull(s1); - MetricRegistryInfo mockRegistryInfo = Mockito.mock(MetricRegistryInfo.class); - Mockito.when(mockRegistryInfo.getMetricsJmxContext()).thenReturn("ts1"); - adapter.unregisterSource(mockRegistryInfo); - s1 = metricsSystem.getSource("ts1"); - assertNull(s1); - MetricsSource s2 = metricsSystem.getSource("ts2"); - assertNotNull(s2); - } - - @Metrics(context = "test") - private static class TestSource { - @Metric("C1 desc") - MutableCounterLong c1; - @Metric("XXX desc") - MutableCounterLong xxx; - @Metric("G1 desc") - MutableGaugeLong g1; - @Metric("YYY desc") - MutableGaugeLong yyy; - @Metric - MutableRate s1; - @SuppressWarnings("unused") - final MetricsRegistry registry; - - TestSource(String recName) { - registry = new MetricsRegistry(recName); - } - } - -}