From b33878f6fcf760f1956e34c88957909a1b1d40cd Mon Sep 17 00:00:00 2001 From: eclark Date: Mon, 29 Apr 2013 18:17:37 +0000 Subject: [PATCH] HBASE-8426 Opening a region failed on Metrics source RegionServer,sub=Regions already exists git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1477247 13f79535-47bb-0310-9956-ffa450edef68 --- .../hbase/CompatibilitySingletonFactory.java | 67 ++++++++++--------- .../MetricsRegionServerSourceFactoryImpl.java | 22 +++--- .../MetricsRegionServerSourceFactoryImpl.java | 22 +++--- 3 files changed, 65 insertions(+), 46 deletions(-) diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java index 2b2c53d7000..c3f89703e41 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/CompatibilitySingletonFactory.java @@ -31,8 +31,12 @@ import java.util.ServiceLoader; * created. */ public class CompatibilitySingletonFactory extends CompatibilityFactory { + public static enum SingletonStorage { + INSTANCE; + Object lock = new Object(); + private static final Map instances = new HashMap(); + } private static final Log LOG = LogFactory.getLog(CompatibilitySingletonFactory.class); - private static final Map instances = new HashMap(); /** * This is a static only class don't let anyone create an instance. @@ -45,37 +49,40 @@ public class CompatibilitySingletonFactory extends CompatibilityFactory { * @return the singleton */ @SuppressWarnings("unchecked") - public static synchronized T getInstance(Class klass) { - T instance = (T) instances.get(klass); - if (instance == null) { - try { - ServiceLoader loader = ServiceLoader.load(klass); - Iterator it = loader.iterator(); - instance = it.next(); - if (it.hasNext()) { - StringBuilder msg = new StringBuilder(); - msg.append("ServiceLoader provided more than one implementation for class: ") - .append(klass) - .append(", using implementation: ").append(instance.getClass()) - .append(", other implementations: {"); - while (it.hasNext()) { - msg.append(it.next()).append(" "); - } - msg.append("}"); - LOG.warn(msg); - } - } catch (Exception e) { - throw new RuntimeException(createExceptionString(klass), e); - } catch (Error e) { - throw new RuntimeException(createExceptionString(klass), e); - } - - // If there was nothing returned and no exception then throw an exception. + public static T getInstance(Class klass) { + synchronized (SingletonStorage.INSTANCE.lock) { + T instance = (T) SingletonStorage.INSTANCE.instances.get(klass); if (instance == null) { - throw new RuntimeException(createExceptionString(klass)); + try { + ServiceLoader loader = ServiceLoader.load(klass); + Iterator it = loader.iterator(); + instance = it.next(); + if (it.hasNext()) { + StringBuilder msg = new StringBuilder(); + msg.append("ServiceLoader provided more than one implementation for class: ") + .append(klass) + .append(", using implementation: ").append(instance.getClass()) + .append(", other implementations: {"); + while (it.hasNext()) { + msg.append(it.next()).append(" "); + } + msg.append("}"); + LOG.warn(msg); + } + } catch (Exception e) { + throw new RuntimeException(createExceptionString(klass), e); + } catch (Error e) { + throw new RuntimeException(createExceptionString(klass), e); + } + + // If there was nothing returned and no exception then throw an exception. + if (instance == null) { + throw new RuntimeException(createExceptionString(klass)); + } + SingletonStorage.INSTANCE.instances.put(klass, instance); } - instances.put(klass, instance); + return instance; } - return instance; + } } diff --git a/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java b/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java index dc4ae6abc7c..f6cb79bf427 100644 --- a/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java +++ b/hbase-hadoop1-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java @@ -22,27 +22,33 @@ package org.apache.hadoop.hbase.regionserver; * Factory to create MetricsRegionServerSource when given a MetricsRegionServerWrapper */ public class MetricsRegionServerSourceFactoryImpl implements MetricsRegionServerSourceFactory { - private static enum FactoryStorage { + public static enum FactoryStorage { INSTANCE; + private Object aggLock = new Object(); + private Object serverLock = new Object(); private MetricsRegionServerSource serverSource; private MetricsRegionAggregateSourceImpl aggImpl; } private synchronized MetricsRegionAggregateSourceImpl getAggregate() { - if (FactoryStorage.INSTANCE.aggImpl == null) { - FactoryStorage.INSTANCE.aggImpl = new MetricsRegionAggregateSourceImpl(); + synchronized (FactoryStorage.INSTANCE.aggLock) { + if (FactoryStorage.INSTANCE.aggImpl == null) { + FactoryStorage.INSTANCE.aggImpl = new MetricsRegionAggregateSourceImpl(); + } + return FactoryStorage.INSTANCE.aggImpl; } - return FactoryStorage.INSTANCE.aggImpl; } @Override public synchronized MetricsRegionServerSource createServer(MetricsRegionServerWrapper regionServerWrapper) { - if (FactoryStorage.INSTANCE.serverSource == null) { - FactoryStorage.INSTANCE.serverSource = new MetricsRegionServerSourceImpl( - regionServerWrapper); + synchronized (FactoryStorage.INSTANCE.serverLock) { + if (FactoryStorage.INSTANCE.serverSource == null) { + FactoryStorage.INSTANCE.serverSource = new MetricsRegionServerSourceImpl( + regionServerWrapper); + } + return FactoryStorage.INSTANCE.serverSource; } - return FactoryStorage.INSTANCE.serverSource; } @Override diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java index dc4ae6abc7c..f6cb79bf427 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceFactoryImpl.java @@ -22,27 +22,33 @@ package org.apache.hadoop.hbase.regionserver; * Factory to create MetricsRegionServerSource when given a MetricsRegionServerWrapper */ public class MetricsRegionServerSourceFactoryImpl implements MetricsRegionServerSourceFactory { - private static enum FactoryStorage { + public static enum FactoryStorage { INSTANCE; + private Object aggLock = new Object(); + private Object serverLock = new Object(); private MetricsRegionServerSource serverSource; private MetricsRegionAggregateSourceImpl aggImpl; } private synchronized MetricsRegionAggregateSourceImpl getAggregate() { - if (FactoryStorage.INSTANCE.aggImpl == null) { - FactoryStorage.INSTANCE.aggImpl = new MetricsRegionAggregateSourceImpl(); + synchronized (FactoryStorage.INSTANCE.aggLock) { + if (FactoryStorage.INSTANCE.aggImpl == null) { + FactoryStorage.INSTANCE.aggImpl = new MetricsRegionAggregateSourceImpl(); + } + return FactoryStorage.INSTANCE.aggImpl; } - return FactoryStorage.INSTANCE.aggImpl; } @Override public synchronized MetricsRegionServerSource createServer(MetricsRegionServerWrapper regionServerWrapper) { - if (FactoryStorage.INSTANCE.serverSource == null) { - FactoryStorage.INSTANCE.serverSource = new MetricsRegionServerSourceImpl( - regionServerWrapper); + synchronized (FactoryStorage.INSTANCE.serverLock) { + if (FactoryStorage.INSTANCE.serverSource == null) { + FactoryStorage.INSTANCE.serverSource = new MetricsRegionServerSourceImpl( + regionServerWrapper); + } + return FactoryStorage.INSTANCE.serverSource; } - return FactoryStorage.INSTANCE.serverSource; } @Override