From b115dd28e2d1070c59ccca4dcf828e36f6a4f18a Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Tue, 9 Dec 2014 21:36:40 +0530 Subject: [PATCH] HBASE-12630 Provide capability for dropping malfunctioning ConfigurationObserver automatically --- .../java/org/apache/hadoop/hbase/HConstants.java | 11 ++++++-- .../hadoop/hbase/conf/ConfigurationManager.java | 33 +++++++++++++++++++--- .../hbase/conf/TestConfigurationManager.java | 30 ++++++++++++++++++++ 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java index 6001767..12418f6 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java @@ -388,7 +388,7 @@ public final class HConstants { /** * The hbase:meta table's name. - * + * */ @Deprecated // for compat from 0.94 -> 0.96. public static final byte[] META_TABLE_NAME = TableName.META_TABLE_NAME.getName(); @@ -1086,7 +1086,7 @@ public final class HConstants { public static final String HBASE_CLIENT_FAST_FAIL_THREASHOLD_MS = "hbase.client.fastfail.threshold"; - + public static final long HBASE_CLIENT_FAST_FAIL_THREASHOLD_MS_DEFAULT = 60000; @@ -1097,7 +1097,12 @@ public final class HConstants { 600000; public static final String HBASE_CLIENT_FAST_FAIL_INTERCEPTOR_IMPL = - "hbase.client.fast.fail.interceptor.impl"; + "hbase.client.fast.fail.interceptor.impl"; + + public static final String HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES = + "hbase.configuration.observer.max.exception.retries"; + + public static final int HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES_DEFAULT = 10; private HConstants() { // Can't be instantiated with this ctor. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/conf/ConfigurationManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/conf/ConfigurationManager.java index 76edbf4..2c19f97 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/conf/ConfigurationManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/conf/ConfigurationManager.java @@ -17,16 +17,18 @@ */ package org.apache.hadoop.hbase.conf; +import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.WeakHashMap; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; -import java.util.Collections; -import java.util.Set; -import java.util.WeakHashMap; - /** * Maintains the set of all the classes which would like to get notified * when the Configuration is reloaded from the disk in the Online Configuration @@ -82,6 +84,8 @@ public class ConfigurationManager { private Set configurationObservers = Collections.newSetFromMap(new WeakHashMap()); + private Map configurationObserverExceptionTracker = + new WeakHashMap(); /** * Register an observer class @@ -90,6 +94,7 @@ public class ConfigurationManager { public void registerObserver(ConfigurationObserver observer) { synchronized (configurationObservers) { configurationObservers.add(observer); + configurationObserverExceptionTracker.put(observer, 0); if (observer instanceof PropagatingConfigurationObserver) { ((PropagatingConfigurationObserver) observer).registerChildren(this); } @@ -114,20 +119,40 @@ public class ConfigurationManager { * all the observers that are expressed interest to do that. */ public void notifyAllObservers(Configuration conf) { + int maxRetries = + conf.getInt(HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES, + HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES_DEFAULT); synchronized (configurationObservers) { for (ConfigurationObserver observer : configurationObservers) { try { if (observer != null) { observer.onConfigurationChange(conf); + configurationObserverExceptionTracker.put(observer, 0); } } catch (Throwable t) { LOG.error("Encountered a throwable while notifying observers: " + " of type : " + observer.getClass().getCanonicalName() + "(" + observer + ")", t); + + handleObserverException(conf, observer, maxRetries); } } } } + private void handleObserverException(Configuration conf, ConfigurationObserver observer, + int maxRetries) { + Integer observerExceptionCount = configurationObserverExceptionTracker.get(observer); + if (observerExceptionCount > maxRetries) { + LOG.error("(" + observer + + ") failed in updating the configuration continuously for more than " + maxRetries + + " times. Hence deregistering the observer from ConfigurationManager."); + this.deregisterObserver(observer); + } else { + configurationObserverExceptionTracker.put(observer, observerExceptionCount + 1); + } + } + + /** * @return the number of observers. */ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/conf/TestConfigurationManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/conf/TestConfigurationManager.java index fe56344..bf5c433 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/conf/TestConfigurationManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/conf/TestConfigurationManager.java @@ -20,10 +20,12 @@ package org.apache.hadoop.hbase.conf; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.Test; @@ -32,6 +34,9 @@ import org.junit.experimental.categories.Category; @Category({SmallTests.class, ClientTests.class}) public class TestConfigurationManager { public static final Log LOG = LogFactory.getLog(TestConfigurationManager.class); + String HBASE_CONFIGURATION_OBSERVER_UPDATE_CONF_FAIL_TEST = + "hbase.configuration.observer.update.conf.fail.test"; + class DummyConfigurationObserver implements ConfigurationObserver { private boolean notifiedOnChange = false; @@ -44,6 +49,10 @@ public class TestConfigurationManager { public void onConfigurationChange(Configuration conf) { notifiedOnChange = true; + if (conf.getBoolean(HBASE_CONFIGURATION_OBSERVER_UPDATE_CONF_FAIL_TEST, false)) { + throw new RuntimeException("Throwing exception to test HBASE-12630."); + } + } // Was the observer notified on Configuration change? @@ -134,4 +143,25 @@ public class TestConfigurationManager { } assertTrue(outOfScopeObserversDeregistered); } + + @Test + public void testObserversDeregisterationAfterContinuosConfiguredTimesFailure() { + Configuration conf = new Configuration(); + conf.setInt(HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES, 2); + conf.setBoolean(HBASE_CONFIGURATION_OBSERVER_UPDATE_CONF_FAIL_TEST, true); + ConfigurationManager cm = new ConfigurationManager(); + DummyConfigurationObserver d1 = new DummyConfigurationObserver(cm); + assertEquals(1, cm.getNumObservers()); + + int maxRetries = + conf.getInt(HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES, + HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES_DEFAULT) + 1; + for (int i = 0; i <= maxRetries; i++) { + cm.notifyAllObservers(conf); + assertTrue(d1.wasNotifiedOnChange()); + d1.resetNotifiedOnChange(); + } + + assertEquals(0, cm.getNumObservers()); + } } -- 1.9.2.msysgit.0