From 39ba53995e252ce0203d5e71e83e0dc1478d9e3a Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Mon, 8 Dec 2014 16:24:45 +0530 Subject: [PATCH] HBASE-12360 Provide capability for dropping malfunctioning ConfigurationObserver automatically --- .../java/org/apache/hadoop/hbase/HConstants.java | 11 ++++-- .../hadoop/hbase/conf/ConfigurationManager.java | 40 ++++++++++++++++++++-- .../hbase/conf/TestConfigurationManager.java | 26 ++++++++++++++ 3 files changed, 71 insertions(+), 6 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..99bf626 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 @@ -20,10 +20,13 @@ package org.apache.hadoop.hbase.conf; 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.HashMap; +import java.util.Map; import java.util.Set; import java.util.WeakHashMap; @@ -83,6 +86,9 @@ public class ConfigurationManager { Collections.newSetFromMap(new WeakHashMap()); + private Map trackConfigurationObserversException = + new HashMap(); + /** * Register an observer class * @param observer @@ -90,6 +96,7 @@ public class ConfigurationManager { public void registerObserver(ConfigurationObserver observer) { synchronized (configurationObservers) { configurationObservers.add(observer); + trackConfigurationObserversException.put(observer, 0); if (observer instanceof PropagatingConfigurationObserver) { ((PropagatingConfigurationObserver) observer).registerChildren(this); } @@ -119,21 +126,48 @@ public class ConfigurationManager { try { if (observer != null) { observer.onConfigurationChange(conf); + trackConfigurationObserversException.put(observer, 0); } } catch (Throwable t) { - LOG.error("Encountered a throwable while notifying observers: " + " of type : " + - observer.getClass().getCanonicalName() + "(" + observer + ")", 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 = trackConfigurationObserversException.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 continuosly for more than " + maxRetries + + " times. Hence deregistering the observer from ConfigurationManager."); + this.deregisterObserver(observer); + } else { + trackConfigurationObserversException.put(observer, observerExceptionCount + 1); + } + } + /** - * @return the number of observers. + * @return the number of observers. */ public int getNumObservers() { synchronized (configurationObservers) { return configurationObservers.size(); } } + + public Map getTrackConfigurationObserversException() { + return trackConfigurationObserversException; + } + + public void setTrackConfigurationObserversException( + Map trackConfigurationObserversException) { + this.trackConfigurationObserversException = trackConfigurationObserversException; + } } 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 51552a5..e7806b5 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 @@ -23,6 +23,7 @@ import junit.framework.TestCase; 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.experimental.categories.Category; @@ -30,6 +31,8 @@ import org.junit.experimental.categories.Category; @Category({SmallTests.class, ClientTests.class}) public class TestConfigurationManager extends TestCase { 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; @@ -42,6 +45,9 @@ public class TestConfigurationManager extends TestCase { 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? @@ -92,6 +98,26 @@ public class TestConfigurationManager extends TestCase { assertFalse(d2.wasNotifiedOnChange()); } + 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()); + } + // Register an observer that will go out of scope immediately, allowing // us to test that out of scope observers are deregistered. private void registerLocalObserver(ConfigurationManager cm) { -- 1.9.2.msysgit.0