From 1b9e735a1668cb6d986757c7eb70159a41a12a82 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Tue, 9 Dec 2014 21:26:06 +0530 Subject: [PATCH] HBASE-12630 Provide capability for dropping malfunctioning ConfigurationObserver automatically --- .../java/org/apache/hadoop/hbase/HConstants.java | 5 ++++ .../hadoop/hbase/conf/ConfigurationManager.java | 32 +++++++++++++++++++--- .../hbase/conf/TestConfigurationManager.java | 29 ++++++++++++++++++++ 3 files changed, 62 insertions(+), 4 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..0871bbf 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 @@ -1099,6 +1099,11 @@ public final class HConstants { public static final String 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..5a5341a 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); } @@ -119,15 +124,34 @@ public class ConfigurationManager { 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); } } } } + private void handleObserverException(Configuration conf, ConfigurationObserver observer) { + Integer observerExceptionCount = configurationObserverExceptionTracker.get(observer); + int maxRetries = + conf.getInt(HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES, + HConstants.HBASE_CONFIGURATION_OBSERVER_MAX_EXCEPTION_RETRIES_DEFAULT); + 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..2e9e1b4 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,8 @@ 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 +48,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 +142,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