diff --git a/CHANGES.txt b/CHANGES.txt index 032652cd313..d9a0bbb0252 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -139,6 +139,8 @@ Release 0.91.0 - Unreleased (Doug Meil via Stack) HBASE-3738 Book.xml - expanding Architecture Client section (Doug Meil via Stack) + HBASE-3587 Eliminate use of read-write lock to guard loaded + coprocessor collection TASK HBASE-3559 Move report of split to master OFF the heartbeat channel diff --git a/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java b/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index cec146db7e4..3a4946cb009 100644 --- a/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ b/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.client.*; import org.apache.hadoop.hbase.client.coprocessor.Batch; import org.apache.hadoop.hbase.ipc.CoprocessorProtocol; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.SortedCopyOnWriteSet; import org.apache.hadoop.hbase.util.VersionInfo; import java.io.File; @@ -38,7 +39,6 @@ import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; import java.util.*; -import java.util.concurrent.locks.ReentrantReadWriteLock; /** * Provides the common setup framework and runtime services for coprocessor @@ -56,9 +56,9 @@ public abstract class CoprocessorHost { private static final Log LOG = LogFactory.getLog(CoprocessorHost.class); /** Ordered set of loaded coprocessors with lock */ - protected final ReentrantReadWriteLock coprocessorLock = new ReentrantReadWriteLock(); - protected Set coprocessors = - new TreeSet(new EnvironmentPriorityComparator()); + protected SortedSet coprocessors = + new SortedCopyOnWriteSet(new EnvironmentPriorityComparator()); + protected Configuration conf; // unique file prefix to use for local copies of jars when classloading protected String pathPrefix; @@ -79,6 +79,7 @@ public abstract class CoprocessorHost { return; StringTokenizer st = new StringTokenizer(defaultCPClasses, ","); int priority = Coprocessor.Priority.SYSTEM.intValue(); + List configured = new ArrayList(); while (st.hasMoreTokens()) { String className = st.nextToken(); if (findCoprocessor(className) != null) { @@ -88,7 +89,7 @@ public abstract class CoprocessorHost { Thread.currentThread().setContextClassLoader(cl); try { implClass = cl.loadClass(className); - load(implClass, Coprocessor.Priority.SYSTEM); + configured.add(loadInstance(implClass, Coprocessor.Priority.SYSTEM)); LOG.info("System coprocessor " + className + " was loaded " + "successfully with priority (" + priority++ + ")."); } catch (ClassNotFoundException e) { @@ -99,6 +100,9 @@ public abstract class CoprocessorHost { e.getMessage()); } } + + // add entire set to the collection for COW efficiency + coprocessors.addAll(configured); } /** @@ -109,7 +113,7 @@ public abstract class CoprocessorHost { * @throws java.io.IOException Exception */ @SuppressWarnings("deprecation") - public void load(Path path, String className, Coprocessor.Priority priority) + public E load(Path path, String className, Coprocessor.Priority priority) throws IOException { Class implClass = null; @@ -163,7 +167,7 @@ public abstract class CoprocessorHost { } } - load(implClass, priority); + return loadInstance(implClass, priority); } /** @@ -173,6 +177,12 @@ public abstract class CoprocessorHost { */ public void load(Class implClass, Coprocessor.Priority priority) throws IOException { + E env = loadInstance(implClass, priority); + coprocessors.add(env); + } + + public E loadInstance(Class implClass, Coprocessor.Priority priority) + throws IOException { // create the instance Coprocessor impl; Object o = null; @@ -189,13 +199,7 @@ public abstract class CoprocessorHost { if (env instanceof Environment) { ((Environment)env).startup(); } - - try { - coprocessorLock.writeLock().lock(); - coprocessors.add(env); - } finally { - coprocessorLock.writeLock().unlock(); - } + return env; } /** @@ -220,18 +224,13 @@ public abstract class CoprocessorHost { */ public Coprocessor findCoprocessor(String className) { // initialize the coprocessors - try { - coprocessorLock.readLock().lock(); - for (E env: coprocessors) { - if (env.getInstance().getClass().getName().equals(className) || - env.getInstance().getClass().getSimpleName().equals(className)) { - return env.getInstance(); - } + for (E env: coprocessors) { + if (env.getInstance().getClass().getName().equals(className) || + env.getInstance().getClass().getSimpleName().equals(className)) { + return env.getInstance(); } - return null; - } finally { - coprocessorLock.readLock().unlock(); } + return null; } /** diff --git a/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index d4c98725466..c0e941781e8 100644 --- a/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -70,483 +70,343 @@ public class MasterCoprocessorHost /* Implementation of hooks for invoking MasterObservers */ void preCreateTable(HTableDescriptor desc, byte[][] splitKeys) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preCreateTable(env, desc, splitKeys); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preCreateTable(env, desc, splitKeys); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postCreateTable(HRegionInfo[] regions, boolean sync) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postCreateTable(env, regions, sync); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postCreateTable(env, regions, sync); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preDeleteTable(byte[] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preDeleteTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preDeleteTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postDeleteTable(byte[] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postDeleteTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postDeleteTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preModifyTable(final byte[] tableName, HTableDescriptor htd) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preModifyTable(env, tableName, htd); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preModifyTable(env, tableName, htd); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postModifyTable(final byte[] tableName, HTableDescriptor htd) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postModifyTable(env, tableName, htd); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postModifyTable(env, tableName, htd); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preAddColumn(byte [] tableName, HColumnDescriptor column) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preAddColumn(env, tableName, column); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preAddColumn(env, tableName, column); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postAddColumn(byte [] tableName, HColumnDescriptor column) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postAddColumn(env, tableName, column); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postAddColumn(env, tableName, column); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preModifyColumn(byte [] tableName, HColumnDescriptor descriptor) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preModifyColumn( - env, tableName, descriptor); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preModifyColumn( + env, tableName, descriptor); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postModifyColumn(byte [] tableName, HColumnDescriptor descriptor) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postModifyColumn( - env, tableName, descriptor); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postModifyColumn( + env, tableName, descriptor); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preDeleteColumn(final byte [] tableName, final byte [] c) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preDeleteColumn(env, tableName, c); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preDeleteColumn(env, tableName, c); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postDeleteColumn(final byte [] tableName, final byte [] c) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postDeleteColumn(env, tableName, c); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postDeleteColumn(env, tableName, c); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preEnableTable(final byte [] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preEnableTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preEnableTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postEnableTable(final byte [] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postEnableTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postEnableTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preDisableTable(final byte [] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preDisableTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preDisableTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postDisableTable(final byte [] tableName) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postDisableTable(env, tableName); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postDisableTable(env, tableName); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preMove(final HRegionInfo region, final HServerInfo srcServer, final HServerInfo destServer) throws UnknownRegionException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preMove( - env, region, srcServer, destServer); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preMove( + env, region, srcServer, destServer); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void postMove(final HRegionInfo region, final HServerInfo srcServer, final HServerInfo destServer) throws UnknownRegionException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postMove( - env, region, srcServer, destServer); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postMove( + env, region, srcServer, destServer); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } boolean preAssign(final byte [] regionName, final boolean force) throws IOException { boolean bypass = false; - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preAssign(env, regionName, force); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preAssign(env, regionName, force); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } return bypass; } void postAssign(final HRegionInfo regionInfo) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postAssign(env, regionInfo); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postAssign(env, regionInfo); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } boolean preUnassign(final byte [] regionName, final boolean force) throws IOException { boolean bypass = false; - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preUnassign( - env, regionName, force); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preUnassign( + env, regionName, force); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } return bypass; } void postUnassign(final HRegionInfo regionInfo, final boolean force) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postUnassign( - env, regionInfo, force); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postUnassign( + env, regionInfo, force); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } boolean preBalance() throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preBalance(env); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preBalance(env); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } void postBalance() throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postBalance(env); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postBalance(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } boolean preBalanceSwitch(final boolean b) throws IOException { boolean balance = b; - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - balance = ((MasterObserver)env.getInstance()).preBalanceSwitch( - env, balance); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + balance = ((MasterObserver)env.getInstance()).preBalanceSwitch( + env, balance); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } return balance; } void postBalanceSwitch(final boolean oldValue, final boolean newValue) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).postBalanceSwitch( - env, oldValue, newValue); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).postBalanceSwitch( + env, oldValue, newValue); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preShutdown() throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preShutdown(env); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preShutdown(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } void preStopMaster() throws IOException { - try { - coprocessorLock.readLock().lock(); - for (MasterEnvironment env: coprocessors) { - if (env.getInstance() instanceof MasterObserver) { - ((MasterObserver)env.getInstance()).preStopMaster(env); - if (env.shouldComplete()) { - break; - } + for (MasterEnvironment env: coprocessors) { + if (env.getInstance() instanceof MasterObserver) { + ((MasterObserver)env.getInstance()).preStopMaster(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java b/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java index 66fb32104a0..4d766585942 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java @@ -120,6 +120,7 @@ public class RegionCoprocessorHost void loadTableCoprocessors () { // scan the table attributes for coprocessor load specifications // initialize the coprocessors + List configured = new ArrayList(); for (Map.Entry e: region.getTableDesc().getValues().entrySet()) { String key = Bytes.toString(e.getKey().get()); @@ -133,7 +134,7 @@ public class RegionCoprocessorHost String className = matcher.group(2); Coprocessor.Priority priority = Coprocessor.Priority.valueOf(matcher.group(3)); - load(path, className, priority); + configured.add(load(path, className, priority)); LOG.info("Load coprocessor " + className + " from HTD of " + Bytes.toString(region.getTableDesc().getName()) + " successfully."); @@ -145,6 +146,8 @@ public class RegionCoprocessorHost } } } + // add together to coprocessor set for COW efficiency + coprocessors.addAll(configured); } @Override @@ -170,18 +173,13 @@ public class RegionCoprocessorHost */ public void preOpen() { loadTableCoprocessors(); - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preOpen(env); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preOpen(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -189,18 +187,13 @@ public class RegionCoprocessorHost * Invoked after a region open */ public void postOpen() { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postOpen(env); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postOpen(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -209,15 +202,10 @@ public class RegionCoprocessorHost * @param abortRequested true if the server is aborting */ public void preClose(boolean abortRequested) { - try { - coprocessorLock.writeLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preClose(env, abortRequested); - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preClose(env, abortRequested); } - } finally { - coprocessorLock.writeLock().unlock(); } } @@ -226,16 +214,11 @@ public class RegionCoprocessorHost * @param abortRequested true if the server is aborting */ public void postClose(boolean abortRequested) { - try { - coprocessorLock.writeLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postClose(env, abortRequested); - } - shutdown(env); + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postClose(env, abortRequested); } - } finally { - coprocessorLock.writeLock().unlock(); + shutdown(env); } } @@ -244,18 +227,13 @@ public class RegionCoprocessorHost * @param willSplit true if the compaction is about to trigger a split */ public void preCompact(boolean willSplit) { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preCompact(env, willSplit); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preCompact(env, willSplit); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -264,18 +242,13 @@ public class RegionCoprocessorHost * @param willSplit true if the compaction is about to trigger a split */ public void postCompact(boolean willSplit) { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postCompact(env, willSplit); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postCompact(env, willSplit); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -283,18 +256,13 @@ public class RegionCoprocessorHost * Invoked before a memstore flush */ public void preFlush() { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preFlush(env); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preFlush(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -302,18 +270,13 @@ public class RegionCoprocessorHost * Invoked after a memstore flush */ public void postFlush() { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postFlush(env); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postFlush(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -321,18 +284,13 @@ public class RegionCoprocessorHost * Invoked just before a split */ public void preSplit() { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preSplit(env); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preSplit(env); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -342,18 +300,13 @@ public class RegionCoprocessorHost * @param r the new right-hand daughter region */ public void postSplit(HRegion l, HRegion r) { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postSplit(env, l, r); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postSplit(env, l, r); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -368,23 +321,18 @@ public class RegionCoprocessorHost */ public boolean preGetClosestRowBefore(final byte[] row, final byte[] family, final Result result) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preGetClosestRowBefore(env, row, family, - result); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preGetClosestRowBefore(env, row, family, + result); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -395,19 +343,14 @@ public class RegionCoprocessorHost */ public void postGetClosestRowBefore(final byte[] row, final byte[] family, final Result result) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postGetClosestRowBefore(env, row, family, - result); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postGetClosestRowBefore(env, row, family, + result); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -418,22 +361,17 @@ public class RegionCoprocessorHost */ public boolean preGet(final Get get, final List results) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preGet(env, get, results); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preGet(env, get, results); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -444,18 +382,13 @@ public class RegionCoprocessorHost */ public void postGet(final Get get, final List results) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postGet(env, get, results); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postGet(env, get, results); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -466,23 +399,18 @@ public class RegionCoprocessorHost * @exception IOException Exception */ public Boolean preExists(final Get get) throws IOException { - try { - boolean bypass = false; - boolean exists = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - exists = ((RegionObserver)env.getInstance()).preExists(env, get, exists); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; + boolean bypass = false; + boolean exists = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + exists = ((RegionObserver)env.getInstance()).preExists(env, get, exists); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - } - return bypass ? exists : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? exists : null; } /** @@ -493,20 +421,15 @@ public class RegionCoprocessorHost */ public boolean postExists(final Get get, boolean exists) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - exists = ((RegionObserver)env.getInstance()).postExists(env, get, exists); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + exists = ((RegionObserver)env.getInstance()).postExists(env, get, exists); + if (env.shouldComplete()) { + break; } } - return exists; - } finally { - coprocessorLock.readLock().unlock(); } + return exists; } /** @@ -517,22 +440,17 @@ public class RegionCoprocessorHost */ public boolean prePut(final Map> familyMap, final boolean writeToWAL) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).prePut(env, familyMap, writeToWAL); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).prePut(env, familyMap, writeToWAL); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -542,18 +460,13 @@ public class RegionCoprocessorHost */ public void postPut(final Map> familyMap, final boolean writeToWAL) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postPut(env, familyMap, writeToWAL); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postPut(env, familyMap, writeToWAL); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -565,22 +478,17 @@ public class RegionCoprocessorHost */ public boolean preDelete(final Map> familyMap, final boolean writeToWAL) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preDelete(env, familyMap, writeToWAL); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preDelete(env, familyMap, writeToWAL); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -590,18 +498,13 @@ public class RegionCoprocessorHost */ public void postDelete(final Map> familyMap, final boolean writeToWAL) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postDelete(env, familyMap, writeToWAL); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postDelete(env, familyMap, writeToWAL); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -619,26 +522,20 @@ public class RegionCoprocessorHost public Boolean preCheckAndPut(final byte [] row, final byte [] family, final byte [] qualifier, final CompareOp compareOp, final WritableByteArrayComparable comparator, Put put) - throws IOException - { - try { - boolean bypass = false; - boolean result = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - result = ((RegionObserver)env.getInstance()).preCheckAndPut(env, row, family, - qualifier, compareOp, comparator, put, result); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + throws IOException { + boolean bypass = false; + boolean result = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + result = ((RegionObserver)env.getInstance()).preCheckAndPut(env, row, family, + qualifier, compareOp, comparator, put, result); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? result : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? result : null; } /** @@ -654,23 +551,17 @@ public class RegionCoprocessorHost final byte [] qualifier, final CompareOp compareOp, final WritableByteArrayComparable comparator, final Put put, boolean result) - throws IOException - { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - result = ((RegionObserver)env.getInstance()).postCheckAndPut(env, row, - family, qualifier, compareOp, comparator, put, result); - if (env.shouldComplete()) { - break; - } + throws IOException { + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + result = ((RegionObserver)env.getInstance()).postCheckAndPut(env, row, + family, qualifier, compareOp, comparator, put, result); + if (env.shouldComplete()) { + break; } } - return result; - } finally { - coprocessorLock.readLock().unlock(); } + return result; } /** @@ -688,24 +579,19 @@ public class RegionCoprocessorHost final byte [] qualifier, final CompareOp compareOp, final WritableByteArrayComparable comparator, Delete delete) throws IOException { - try { - boolean bypass = false; - boolean result = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - result = ((RegionObserver)env.getInstance()).preCheckAndDelete(env, row, - family, qualifier, compareOp, comparator, delete, result); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + boolean result = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + result = ((RegionObserver)env.getInstance()).preCheckAndDelete(env, row, + family, qualifier, compareOp, comparator, delete, result); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? result : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? result : null; } /** @@ -721,24 +607,18 @@ public class RegionCoprocessorHost final byte [] qualifier, final CompareOp compareOp, final WritableByteArrayComparable comparator, final Delete delete, boolean result) - throws IOException - { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - result = ((RegionObserver)env.getInstance()) - .postCheckAndDelete(env, row, family, qualifier, compareOp, - comparator, delete, result); - if (env.shouldComplete()) { - break; + throws IOException { + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + result = ((RegionObserver)env.getInstance()) + .postCheckAndDelete(env, row, family, qualifier, compareOp, + comparator, delete, result); + if (env.shouldComplete()) { + break; } } - } - return result; - } finally { - coprocessorLock.readLock().unlock(); } + return result; } /** @@ -754,23 +634,18 @@ public class RegionCoprocessorHost public Long preIncrementColumnValue(final byte [] row, final byte [] family, final byte [] qualifier, long amount, final boolean writeToWAL) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - amount = ((RegionObserver)env.getInstance()).preIncrementColumnValue(env, - row, family, qualifier, amount, writeToWAL); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + amount = ((RegionObserver)env.getInstance()).preIncrementColumnValue(env, + row, family, qualifier, amount, writeToWAL); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? amount : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? amount : null; } /** @@ -786,19 +661,14 @@ public class RegionCoprocessorHost public long postIncrementColumnValue(final byte [] row, final byte [] family, final byte [] qualifier, final long amount, final boolean writeToWAL, long result) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - result = ((RegionObserver)env.getInstance()).postIncrementColumnValue(env, - row, family, qualifier, amount, writeToWAL, result); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + result = ((RegionObserver)env.getInstance()).postIncrementColumnValue(env, + row, family, qualifier, amount, writeToWAL, result); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } return result; } @@ -811,23 +681,18 @@ public class RegionCoprocessorHost */ public Result preIncrement(Increment increment) throws IOException { - try { - boolean bypass = false; - Result result = new Result(); - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preIncrement(env, increment, result); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + Result result = new Result(); + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preIncrement(env, increment, result); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? result : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? result : null; } /** @@ -837,18 +702,13 @@ public class RegionCoprocessorHost */ public void postIncrement(final Increment increment, Result result) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postIncrement(env, increment, result); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postIncrement(env, increment, result); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -859,23 +719,18 @@ public class RegionCoprocessorHost * @exception IOException Exception */ public InternalScanner preScannerOpen(Scan scan) throws IOException { - try { - boolean bypass = false; - InternalScanner s = null; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - s = ((RegionObserver)env.getInstance()).preScannerOpen(env, scan, s); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + InternalScanner s = null; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + s = ((RegionObserver)env.getInstance()).preScannerOpen(env, scan, s); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? s : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? s : null; } /** @@ -886,20 +741,15 @@ public class RegionCoprocessorHost */ public InternalScanner postScannerOpen(final Scan scan, InternalScanner s) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - s = ((RegionObserver)env.getInstance()).postScannerOpen(env, scan, s); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + s = ((RegionObserver)env.getInstance()).postScannerOpen(env, scan, s); + if (env.shouldComplete()) { + break; } } - return s; - } finally { - coprocessorLock.readLock().unlock(); } + return s; } /** @@ -912,24 +762,19 @@ public class RegionCoprocessorHost */ public Boolean preScannerNext(final InternalScanner s, final List results, int limit) throws IOException { - try { - boolean bypass = false; - boolean hasNext = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - hasNext = ((RegionObserver)env.getInstance()).preScannerNext(env, s, results, - limit, hasNext); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + boolean hasNext = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + hasNext = ((RegionObserver)env.getInstance()).preScannerNext(env, s, results, + limit, hasNext); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass ? hasNext : null; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass ? hasNext : null; } /** @@ -943,21 +788,16 @@ public class RegionCoprocessorHost public boolean postScannerNext(final InternalScanner s, final List results, final int limit, boolean hasMore) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - hasMore = ((RegionObserver)env.getInstance()).postScannerNext(env, s, - results, limit, hasMore); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + hasMore = ((RegionObserver)env.getInstance()).postScannerNext(env, s, + results, limit, hasMore); + if (env.shouldComplete()) { + break; } } - return hasMore; - } finally { - coprocessorLock.readLock().unlock(); } + return hasMore; } /** @@ -967,22 +807,17 @@ public class RegionCoprocessorHost */ public boolean preScannerClose(final InternalScanner s) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preScannerClose(env, s); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preScannerClose(env, s); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -991,18 +826,13 @@ public class RegionCoprocessorHost */ public void postScannerClose(final InternalScanner s) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postScannerClose(env, s); - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postScannerClose(env, s); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } @@ -1015,23 +845,18 @@ public class RegionCoprocessorHost */ public boolean preWALRestore(HRegionInfo info, HLogKey logKey, WALEdit logEdit) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).preWALRestore(env, info, logKey, - logEdit); - } - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).preWALRestore(env, info, logKey, + logEdit); + } + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -1042,19 +867,14 @@ public class RegionCoprocessorHost */ public void postWALRestore(HRegionInfo info, HLogKey logKey, WALEdit logEdit) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (RegionEnvironment env: coprocessors) { - if (env.getInstance() instanceof RegionObserver) { - ((RegionObserver)env.getInstance()).postWALRestore(env, info, - logKey, logEdit); - } - if (env.shouldComplete()) { - break; - } + for (RegionEnvironment env: coprocessors) { + if (env.getInstance() instanceof RegionObserver) { + ((RegionObserver)env.getInstance()).postWALRestore(env, info, + logKey, logEdit); + } + if (env.shouldComplete()) { + break; } - } finally { - coprocessorLock.readLock().unlock(); } } } diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java b/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java index 6885bc0afe2..32b0e3dc27e 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java @@ -95,24 +95,19 @@ public class WALCoprocessorHost */ public boolean preWALWrite(HRegionInfo info, HLogKey logKey, WALEdit logEdit) throws IOException { - try { - boolean bypass = false; - coprocessorLock.readLock().lock(); - for (WALEnvironment env: coprocessors) { - if (env.getInstance() instanceof - org.apache.hadoop.hbase.coprocessor.WALObserver) { - ((org.apache.hadoop.hbase.coprocessor.WALObserver)env.getInstance()). - preWALWrite(env, info, logKey, logEdit); - bypass |= env.shouldBypass(); - if (env.shouldComplete()) { - break; - } + boolean bypass = false; + for (WALEnvironment env: coprocessors) { + if (env.getInstance() instanceof + org.apache.hadoop.hbase.coprocessor.WALObserver) { + ((org.apache.hadoop.hbase.coprocessor.WALObserver)env.getInstance()). + preWALWrite(env, info, logKey, logEdit); + bypass |= env.shouldBypass(); + if (env.shouldComplete()) { + break; } } - return bypass; - } finally { - coprocessorLock.readLock().unlock(); } + return bypass; } /** @@ -123,20 +118,15 @@ public class WALCoprocessorHost */ public void postWALWrite(HRegionInfo info, HLogKey logKey, WALEdit logEdit) throws IOException { - try { - coprocessorLock.readLock().lock(); - for (WALEnvironment env: coprocessors) { - if (env.getInstance() instanceof - org.apache.hadoop.hbase.coprocessor.WALObserver) { - ((org.apache.hadoop.hbase.coprocessor.WALObserver)env.getInstance()). - postWALWrite(env, info, logKey, logEdit); - if (env.shouldComplete()) { - break; - } + for (WALEnvironment env: coprocessors) { + if (env.getInstance() instanceof + org.apache.hadoop.hbase.coprocessor.WALObserver) { + ((org.apache.hadoop.hbase.coprocessor.WALObserver)env.getInstance()). + postWALWrite(env, info, logKey, logEdit); + if (env.shouldComplete()) { + break; } } - } finally { - coprocessorLock.readLock().unlock(); } } } diff --git a/src/main/java/org/apache/hadoop/hbase/util/SortedCopyOnWriteSet.java b/src/main/java/org/apache/hadoop/hbase/util/SortedCopyOnWriteSet.java new file mode 100644 index 00000000000..f04f306e812 --- /dev/null +++ b/src/main/java/org/apache/hadoop/hbase/util/SortedCopyOnWriteSet.java @@ -0,0 +1,175 @@ +/* + * Copyright 2011 The Apache Software Foundation + * + * 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.util; + +import java.util.Collection; +import java.util.Comparator; +import java.util.Iterator; +import java.util.SortedSet; +import java.util.TreeSet; + +/** + * Simple {@link java.util.SortedSet} implementation that uses an internal + * {@link java.util.TreeSet} to provide ordering. All mutation operations + * create a new copy of the TreeSet instance, so are very + * expensive. This class is only intended for use on small, very rarely + * written collections that expect highly concurrent reads. Read operations + * are performed on a reference to the internal TreeSet at the + * time of invocation, so will not see any mutations to the collection during + * their operation. + * + *

Note that due to the use of a {@link java.util.TreeSet} internally, + * a {@link java.util.Comparator} instance must be provided, or collection + * elements must implement {@link java.lang.Comparable}. + *

+ * @param A class implementing {@link java.lang.Comparable} or able to be + * compared by a provided comparator. + */ +public class SortedCopyOnWriteSet implements SortedSet { + private SortedSet internalSet; + + public SortedCopyOnWriteSet() { + this.internalSet = new TreeSet(); + } + + public SortedCopyOnWriteSet(Collection c) { + this.internalSet = new TreeSet(c); + } + + public SortedCopyOnWriteSet(Comparator comparator) { + this.internalSet = new TreeSet(comparator); + } + + @Override + public int size() { + return internalSet.size(); + } + + @Override + public boolean isEmpty() { + return internalSet.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return internalSet.contains(o); + } + + @Override + public Iterator iterator() { + return internalSet.iterator(); + } + + @Override + public Object[] toArray() { + return internalSet.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return internalSet.toArray(a); + } + + @Override + public synchronized boolean add(E e) { + SortedSet newSet = new TreeSet(internalSet); + boolean added = newSet.add(e); + internalSet = newSet; + return added; + } + + @Override + public synchronized boolean remove(Object o) { + SortedSet newSet = new TreeSet(internalSet); + boolean removed = newSet.remove(o); + internalSet = newSet; + return removed; + } + + @Override + public boolean containsAll(Collection c) { + return internalSet.containsAll(c); + } + + @Override + public synchronized boolean addAll(Collection c) { + SortedSet newSet = new TreeSet(internalSet); + boolean changed = newSet.addAll(c); + internalSet = newSet; + return changed; + } + + @Override + public synchronized boolean retainAll(Collection c) { + SortedSet newSet = new TreeSet(internalSet); + boolean changed = newSet.retainAll(c); + internalSet = newSet; + return changed; + } + + @Override + public synchronized boolean removeAll(Collection c) { + SortedSet newSet = new TreeSet(internalSet); + boolean changed = newSet.removeAll(c); + internalSet = newSet; + return changed; + } + + @Override + public synchronized void clear() { + Comparator comparator = internalSet.comparator(); + if (comparator != null) { + internalSet = new TreeSet(comparator); + } else { + internalSet = new TreeSet(); + } + } + + @Override + public Comparator comparator() { + return internalSet.comparator(); + } + + @Override + public SortedSet subSet(E fromElement, E toElement) { + return internalSet.subSet(fromElement, toElement); + } + + @Override + public SortedSet headSet(E toElement) { + return internalSet.headSet(toElement); + } + + @Override + public SortedSet tailSet(E fromElement) { + return internalSet.tailSet(fromElement); + } + + @Override + public E first() { + return internalSet.first(); + } + + @Override + public E last() { + return internalSet.last(); + } +} diff --git a/src/test/java/org/apache/hadoop/hbase/util/TestSortedCopyOnWriteSet.java b/src/test/java/org/apache/hadoop/hbase/util/TestSortedCopyOnWriteSet.java new file mode 100644 index 00000000000..f53507c825a --- /dev/null +++ b/src/test/java/org/apache/hadoop/hbase/util/TestSortedCopyOnWriteSet.java @@ -0,0 +1,103 @@ +/* + * Copyright 2011 The Apache Software Foundation + * + * 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.util; + +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.Iterator; + +import com.google.common.collect.Lists; +import org.junit.Test; + +public class TestSortedCopyOnWriteSet { + + @Test + public void testSorting() throws Exception { + SortedCopyOnWriteSet set = new SortedCopyOnWriteSet(); + set.add("c"); + set.add("d"); + set.add("a"); + set.add("b"); + + String[] expected = new String[]{"a", "b", "c", "d"}; + String[] stored = set.toArray(new String[4]); + assertArrayEquals(expected, stored); + + set.add("c"); + assertEquals(4, set.size()); + stored = set.toArray(new String[4]); + assertArrayEquals(expected, stored); + } + + @Test + public void testIteratorIsolation() throws Exception { + SortedCopyOnWriteSet set = new SortedCopyOnWriteSet( + Lists.newArrayList("a", "b", "c", "d", "e")); + + // isolation of remove() + Iterator iter = set.iterator(); + set.remove("c"); + boolean found = false; + while (iter.hasNext() && !found) { + found = "c".equals(iter.next()); + } + assertTrue(found); + + iter = set.iterator(); + found = false; + while (iter.hasNext() && !found) { + found = "c".equals(iter.next()); + } + assertFalse(found); + + // isolation of add() + iter = set.iterator(); + set.add("f"); + found = false; + while (iter.hasNext() && !found) { + String next = iter.next(); + found = "f".equals(next); + } + assertFalse(found); + + // isolation of addAll() + iter = set.iterator(); + set.addAll(Lists.newArrayList("g", "h", "i")); + found = false; + while (iter.hasNext() && !found) { + String next = iter.next(); + found = "g".equals(next) || "h".equals(next) || "i".equals(next); + } + assertFalse(found); + + // isolation of clear() + iter = set.iterator(); + set.clear(); + assertEquals(0, set.size()); + int size = 0; + while (iter.hasNext()) { + iter.next(); + size++; + } + assertTrue(size > 0); + } +}