diff --git src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index 0a1fb2a..ada94d6 100644 --- src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -27,6 +27,7 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.client.*; import org.apache.hadoop.hbase.client.coprocessor.Batch; import org.apache.hadoop.hbase.ipc.CoprocessorProtocol; @@ -537,4 +538,28 @@ public abstract class CoprocessorHost { return new HTableWrapper(tableName); } } + + // for HBASE-4014: dump list of loaded coprocessors for crash analysis. + protected String coprocessorSetAsString() { + String coprocessorSetAsString = "{"; + + for (E env: coprocessors) { + coprocessorSetAsString += env.getInstance().getClass().getName(); + coprocessorSetAsString += ","; + } + + if (coprocessorSetAsString.indexOf(',') != -1) { + // remove trailing comma. + coprocessorSetAsString = coprocessorSetAsString.substring(0,coprocessorSetAsString.length()-1); + } + coprocessorSetAsString += "}"; + return coprocessorSetAsString; + } + + protected void abortServiceWithCoprocessorInfo(final String service, final Server server,final CoprocessorEnvironment environment,final Throwable e) { + String coprocessorName = (environment.getInstance()).getClass().getName(); + server.abort("Aborting service: " + service + " running on : " + server.getServerName() + " because coprocessor: " + coprocessorName + " threw an exception: " + + e + ". Loaded coprocessors are: " + coprocessorSetAsString(),e); + } + } diff --git src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 4800bea..c99248e 100644 --- src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -67,6 +67,42 @@ public class MasterCoprocessorHost return new MasterEnvironment(implClass, instance, priority, seq, masterServices); } + private void abortWithCoprocessorInfo(final CoprocessorEnvironment env, final Throwable e) { + abortServiceWithCoprocessorInfo("master", masterServices, env, e); + } + + private void handleCoprocessorThrowable(final CoprocessorEnvironment env, final Throwable e) + throws IOException { + if (e instanceof IOException) { + // Throwable objects which are instances of IOException should be passed + // on to the client. This is in conformance with the HBase idiom regarding + // IOException: that it represents a circumstance that should be passed along to the client + // for its own handling. For example, a coprocessor that implements access controls would + // throw a subclass of IOException, such as AccessDeniedException, in its preGet() method + // to prevent an unauthorized client's performing a Get on a particular table. + throw (IOException)e; + } + else { + // e is not an IOException: master should abort. + abortWithCoprocessorInfo(env, e); + } + } + + private void handleCoprocessorThrowableAsUnknownRegionException(final CoprocessorEnvironment env, final Throwable e) + // Similar to the handleCoprocessorThrowable(), but for those Master coprocessor hooks that throw specifically + // a UnknownRegionException (rather than the more general IOException). In this case, treat any IOException that is + // not an UnknownRegionException as we would a non-IOException Throwable: abort the master. + throws UnknownRegionException { + if (e instanceof UnknownRegionException) { + // The coprocessor threw an UnknownRegionException, which should be passed back to the client. + throw (UnknownRegionException)e; + } + else { + // e is not an UnknownRegionException: master should abort. + abortWithCoprocessorInfo(env, e); + } + } + /* Implementation of hooks for invoking MasterObservers */ void preCreateTable(HTableDescriptor desc, byte[][] splitKeys) throws IOException { @@ -74,7 +110,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preCreateTable(ctx, desc, splitKeys); + try { + ((MasterObserver)env.getInstance()).preCreateTable(ctx, desc, splitKeys); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -87,7 +128,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postCreateTable(ctx, regions, sync); + try { + ((MasterObserver)env.getInstance()).postCreateTable(ctx, regions, sync); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -100,7 +146,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preDeleteTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).preDeleteTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -113,7 +164,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postDeleteTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).postDeleteTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -127,7 +183,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preModifyTable(ctx, tableName, htd); + try { + ((MasterObserver)env.getInstance()).preModifyTable(ctx, tableName, htd); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -141,7 +202,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postModifyTable(ctx, tableName, htd); + try { + ((MasterObserver)env.getInstance()).postModifyTable(ctx, tableName, htd); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -155,7 +221,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preAddColumn(ctx, tableName, column); + try { + ((MasterObserver)env.getInstance()).preAddColumn(ctx, tableName, column); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -169,7 +240,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postAddColumn(ctx, tableName, column); + try { + ((MasterObserver)env.getInstance()).postAddColumn(ctx, tableName, column); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -183,8 +259,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preModifyColumn( + try { + ((MasterObserver)env.getInstance()).preModifyColumn( ctx, tableName, descriptor); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -198,8 +279,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postModifyColumn( - ctx, tableName, descriptor); + try { + ((MasterObserver)env.getInstance()).postModifyColumn( + ctx, tableName, descriptor); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -213,7 +299,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preDeleteColumn(ctx, tableName, c); + try { + ((MasterObserver)env.getInstance()).preDeleteColumn(ctx, tableName, c); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -227,7 +318,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postDeleteColumn(ctx, tableName, c); + try { + ((MasterObserver)env.getInstance()).postDeleteColumn(ctx, tableName, c); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -240,7 +336,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preEnableTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).preEnableTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -253,7 +354,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postEnableTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).postEnableTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -266,7 +372,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preDisableTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).preDisableTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -279,7 +390,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postDisableTable(ctx, tableName); + try { + ((MasterObserver)env.getInstance()).postDisableTable(ctx, tableName); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -293,8 +409,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preMove( - ctx, region, srcServer, destServer); + try { + ((MasterObserver)env.getInstance()).preMove( + ctx, region, srcServer, destServer); + } + catch (Throwable e) { + handleCoprocessorThrowableAsUnknownRegionException(env, e); + } if (ctx.shouldComplete()) { break; } @@ -308,8 +429,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postMove( - ctx, region, srcServer, destServer); + try { + ((MasterObserver)env.getInstance()).postMove( + ctx, region, srcServer, destServer); + } + catch (Throwable e) { + handleCoprocessorThrowableAsUnknownRegionException(env,e); + } if (ctx.shouldComplete()) { break; } @@ -324,7 +450,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preAssign(ctx, regionName, force); + try { + ((MasterObserver)env.getInstance()).preAssign(ctx, regionName, force); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -339,7 +470,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postAssign(ctx, regionInfo); + try { + ((MasterObserver)env.getInstance()).postAssign(ctx, regionInfo); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -354,8 +490,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preUnassign( - ctx, regionName, force); + try { + ((MasterObserver)env.getInstance()).preUnassign( + ctx, regionName, force); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -371,8 +512,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postUnassign( - ctx, regionInfo, force); + try { + ((MasterObserver)env.getInstance()).postUnassign( + ctx, regionInfo, force); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -386,7 +532,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preBalance(ctx); + try { + ((MasterObserver)env.getInstance()).preBalance(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -401,7 +552,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postBalance(ctx); + try { + ((MasterObserver)env.getInstance()).postBalance(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -415,8 +571,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - balance = ((MasterObserver)env.getInstance()).preBalanceSwitch( - ctx, balance); + try { + balance = ((MasterObserver)env.getInstance()).preBalanceSwitch( + ctx, balance); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -431,8 +592,13 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postBalanceSwitch( - ctx, oldValue, newValue); + try { + ((MasterObserver)env.getInstance()).postBalanceSwitch( + ctx, oldValue, newValue); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -445,7 +611,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preShutdown(ctx); + try { + ((MasterObserver)env.getInstance()).preShutdown(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -458,7 +629,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).preStopMaster(ctx); + try { + ((MasterObserver)env.getInstance()).preStopMaster(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } @@ -471,7 +647,12 @@ public class MasterCoprocessorHost for (MasterEnvironment env: coprocessors) { if (env.getInstance() instanceof MasterObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((MasterObserver)env.getInstance()).postStartMaster(ctx); + try { + ((MasterObserver)env.getInstance()).postStartMaster(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } if (ctx.shouldComplete()) { break; } diff --git src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java index a98117f..812eba2 100644 --- src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java +++ src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java @@ -168,6 +168,45 @@ public class RegionCoprocessorHost return new RegionEnvironment(instance, priority, seq, region, rsServices); } + private void abortWithCoprocessorInfo(final CoprocessorEnvironment env, final Throwable e) { + abortServiceWithCoprocessorInfo("regionserver",rsServices,env,e); + } + + private void handleCoprocessorThrowable(final CoprocessorEnvironment env, final Throwable e) + throws IOException { + if (e instanceof java.io.IOException) { + // Throwable objects which are instances of IOException should be passed + // on to the client. This is in conformance with the HBase idiom regarding + // IOException: that it represents a circumstance that should be passed along to the client + // for its own handling. For example, a coprocessor that implements access controls would + // throw a subclass of IOException, such as AccessDeniedException, in its preGet() method + // to prevent an unauthorized client's performing a Get on a particular table. + // + throw (IOException)e; + } + else { + // e is not an IOException: regionserver should abort. + abortWithCoprocessorInfo(env, e); + } + } + + private void handleCoprocessorThrowableNoRethrow(final CoprocessorEnvironment env, final Throwable e, final String hookName) { + if (e instanceof java.io.IOException) { + // Throwable objects which are instances of IOException should be passed + // on to the client. This is in conformance with the HBase idiom regarding + // IOException: that it represents a circumstance that the client as opposed to the server, + // one that the server must handle. + // + LOG.warn("The coprocessor : " + (env.getInstance()).getClass().getName() + " threw an exception: " + e + + ", but the regionserver's coprocessor host hook " + hookName + " is not intended to pass this on to the client." + + "Since the exception is an IOException, it will be ignored rather than causing the regionserver to abort. " + + "Current regionserver coprocessor set is : " + coprocessorSetAsString() + "."); + } + else { + abortWithCoprocessorInfo(env, e); + } + } + /** * Invoked before a region open */ @@ -177,7 +216,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preOpen(ctx); + try { + ((RegionObserver)env.getInstance()).preOpen(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"preOpen"); + } if (ctx.shouldComplete()) { break; } @@ -193,7 +237,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postOpen(ctx); + try { + ((RegionObserver)env.getInstance()).postOpen(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"postOpen"); + } if (ctx.shouldComplete()) { break; } @@ -210,7 +259,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preClose(ctx, abortRequested); + try { + ((RegionObserver)env.getInstance()).preClose(ctx, abortRequested); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"preClose"); + } } } } @@ -224,7 +278,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postClose(ctx, abortRequested); + try { + ((RegionObserver)env.getInstance()).postClose(ctx, abortRequested); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"postClose"); + } + } shutdown(env); } @@ -239,7 +299,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preCompact(ctx, willSplit); + try { + ((RegionObserver)env.getInstance()).preCompact(ctx, willSplit); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"preCompact"); + } if (ctx.shouldComplete()) { break; } @@ -256,7 +321,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postCompact(ctx, willSplit); + try { + ((RegionObserver)env.getInstance()).postCompact(ctx, willSplit); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"postCompact"); + } if (ctx.shouldComplete()) { break; } @@ -272,7 +342,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preFlush(ctx); + try { + ((RegionObserver)env.getInstance()).preFlush(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"preFlush"); + } if (ctx.shouldComplete()) { break; } @@ -288,7 +363,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postFlush(ctx); + try { + ((RegionObserver)env.getInstance()).postFlush(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"postFlush"); + } if (ctx.shouldComplete()) { break; } @@ -304,7 +384,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preSplit(ctx); + try { + ((RegionObserver)env.getInstance()).preSplit(ctx); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"preSplit"); + } if (ctx.shouldComplete()) { break; } @@ -322,7 +407,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postSplit(ctx, l, r); + try { + ((RegionObserver)env.getInstance()).postSplit(ctx, l, r); + } + catch (Throwable e) { + handleCoprocessorThrowableNoRethrow(env,e,"postSplit"); + } if (ctx.shouldComplete()) { break; } @@ -346,8 +436,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preGetClosestRowBefore(ctx, row, family, - result); + try { + ((RegionObserver)env.getInstance()).preGetClosestRowBefore(ctx, row, family, + result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -369,8 +464,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postGetClosestRowBefore(ctx, row, family, - result); + try { + ((RegionObserver)env.getInstance()).postGetClosestRowBefore(ctx, row, family, + result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -390,7 +490,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preGet(ctx, get, results); + try { + ((RegionObserver)env.getInstance()).preGet(ctx, get, results); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -412,7 +517,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postGet(ctx, get, results); + try { + ((RegionObserver)env.getInstance()).postGet(ctx, get, results); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -433,7 +543,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - exists = ((RegionObserver)env.getInstance()).preExists(ctx, get, exists); + try { + exists = ((RegionObserver)env.getInstance()).preExists(ctx, get, exists); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -455,7 +570,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - exists = ((RegionObserver)env.getInstance()).postExists(ctx, get, exists); + try { + exists = ((RegionObserver)env.getInstance()).postExists(ctx, get, exists); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -477,7 +597,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).prePut(ctx, familyMap, writeToWAL); + try { + ((RegionObserver)env.getInstance()).prePut(ctx, familyMap, writeToWAL); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -498,7 +623,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postPut(ctx, familyMap, writeToWAL); + try { + ((RegionObserver)env.getInstance()).postPut(ctx, familyMap, writeToWAL); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -519,7 +649,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preDelete(ctx, familyMap, writeToWAL); + try { + ((RegionObserver)env.getInstance()).preDelete(ctx, familyMap, writeToWAL); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -540,7 +675,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postDelete(ctx, familyMap, writeToWAL); + try { + ((RegionObserver)env.getInstance()).postDelete(ctx, familyMap, writeToWAL); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -569,8 +709,15 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - result = ((RegionObserver)env.getInstance()).preCheckAndPut(ctx, row, family, - qualifier, compareOp, comparator, put, result); + try { + result = ((RegionObserver)env.getInstance()).preCheckAndPut(ctx, row, family, + qualifier, compareOp, comparator, put, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } + + bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -598,8 +745,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - result = ((RegionObserver)env.getInstance()).postCheckAndPut(ctx, row, - family, qualifier, compareOp, comparator, put, result); + try { + result = ((RegionObserver)env.getInstance()).postCheckAndPut(ctx, row, + family, qualifier, compareOp, comparator, put, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -629,8 +781,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - result = ((RegionObserver)env.getInstance()).preCheckAndDelete(ctx, row, - family, qualifier, compareOp, comparator, delete, result); + try { + result = ((RegionObserver)env.getInstance()).preCheckAndDelete(ctx, row, + family, qualifier, compareOp, comparator, delete, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -658,9 +815,14 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - result = ((RegionObserver)env.getInstance()) - .postCheckAndDelete(ctx, row, family, qualifier, compareOp, - comparator, delete, result); + try { + result = ((RegionObserver)env.getInstance()) + .postCheckAndDelete(ctx, row, family, qualifier, compareOp, + comparator, delete, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -687,8 +849,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - amount = ((RegionObserver)env.getInstance()).preIncrementColumnValue(ctx, - row, family, qualifier, amount, writeToWAL); + try { + amount = ((RegionObserver)env.getInstance()).preIncrementColumnValue(ctx, + row, family, qualifier, amount, writeToWAL); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -715,8 +882,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - result = ((RegionObserver)env.getInstance()).postIncrementColumnValue(ctx, - row, family, qualifier, amount, writeToWAL, result); + try { + result = ((RegionObserver)env.getInstance()).postIncrementColumnValue(ctx, + row, family, qualifier, amount, writeToWAL, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -739,7 +911,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preIncrement(ctx, increment, result); + try { + ((RegionObserver)env.getInstance()).preIncrement(ctx, increment, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -760,7 +937,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postIncrement(ctx, increment, result); + try { + ((RegionObserver)env.getInstance()).postIncrement(ctx, increment, result); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -781,7 +963,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - s = ((RegionObserver)env.getInstance()).preScannerOpen(ctx, scan, s); + try { + s = ((RegionObserver)env.getInstance()).preScannerOpen(ctx, scan, s); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -803,7 +990,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - s = ((RegionObserver)env.getInstance()).postScannerOpen(ctx, scan, s); + try { + s = ((RegionObserver)env.getInstance()).postScannerOpen(ctx, scan, s); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -828,8 +1020,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - hasNext = ((RegionObserver)env.getInstance()).preScannerNext(ctx, s, results, - limit, hasNext); + try { + hasNext = ((RegionObserver)env.getInstance()).preScannerNext(ctx, s, results, + limit, hasNext); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -854,8 +1051,13 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - hasMore = ((RegionObserver)env.getInstance()).postScannerNext(ctx, s, - results, limit, hasMore); + try { + hasMore = ((RegionObserver)env.getInstance()).postScannerNext(ctx, s, + results, limit, hasMore); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -876,7 +1078,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preScannerClose(ctx, s); + try { + ((RegionObserver)env.getInstance()).preScannerClose(ctx, s); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; @@ -896,7 +1103,12 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postScannerClose(ctx, s); + try { + ((RegionObserver)env.getInstance()).postScannerClose(ctx, s); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } if (ctx.shouldComplete()) { break; } @@ -918,14 +1130,18 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).preWALRestore(ctx, info, logKey, - logEdit); + try { + ((RegionObserver)env.getInstance()).preWALRestore(ctx, info, logKey, + logEdit); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } bypass |= ctx.shouldBypass(); if (ctx.shouldComplete()) { break; } } - } return bypass; } @@ -942,8 +1158,14 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { if (env.getInstance() instanceof RegionObserver) { ctx = ObserverContext.createAndPrepare(env, ctx); - ((RegionObserver)env.getInstance()).postWALRestore(ctx, info, - logKey, logEdit); + try { + ((RegionObserver)env.getInstance()).postWALRestore(ctx, info, + logKey, logEdit); + } + catch (Throwable e) { + handleCoprocessorThrowable(env,e); + } + if (ctx.shouldComplete()) { break; } diff --git src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java index ab16880..635879e 100644 --- src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java +++ src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java @@ -336,7 +336,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable { break; // Abort the server if Disconnected or Expired - // TODO: Åny reason to handle these two differently? + // TODO: Any reason to handle these two differently? case Disconnected: LOG.debug(prefix("Received Disconnected from ZooKeeper, ignoring")); break; @@ -345,7 +345,7 @@ public class ZooKeeperWatcher implements Watcher, Abortable { String msg = prefix(this.identifier + " received expired from " + "ZooKeeper, aborting"); // TODO: One thought is to add call to ZooKeeperListener so say, - // ZooKeperNodeTracker can zero out its data values. + // ZooKeeperNodeTracker can zero out its data values. if (this.abortable != null) this.abortable.abort(msg, new KeeperException.SessionExpiredException()); break; diff --git src/test/java/org/apache/hadoop/hbase/coprocessor/BuggyCoprocessor.java src/test/java/org/apache/hadoop/hbase/coprocessor/BuggyCoprocessor.java new file mode 100644 index 0000000..1a931b9 --- /dev/null +++ src/test/java/org/apache/hadoop/hbase/coprocessor/BuggyCoprocessor.java @@ -0,0 +1,40 @@ +/* + * Copyright 2010 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.coprocessor; + +import org.apache.hadoop.hbase.KeyValue; +import java.util.List; +import java.util.Map; + +public class BuggyCoprocessor extends SimpleRegionObserver { + + // HBASE-4014: cause a deliberate NPE to test that RegionServer aborts as expected. + @Override + public void prePut(final ObserverContext c, final Map> familyMap, final boolean writeToWAL) { + String tableName = c.getEnvironment().getRegion().getRegionInfo().getTableNameAsString(); + if (tableName.equals("observed_table")) { + Integer i = null; + i = i + 1; + } + } +} + diff --git src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java new file mode 100644 index 0000000..c2c3017 --- /dev/null +++ src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterCoprocessorException.java @@ -0,0 +1,322 @@ +/* + * Copyright 2010 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.coprocessor; + +import java.io.IOException; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.Abortable; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HColumnDescriptor; +import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.MiniHBaseCluster; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.UnknownRegionException; +import org.apache.hadoop.hbase.client.HBaseAdmin; +import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperNodeTracker; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Tests unhandled exceptions thrown by coprocessors running on master. + * Expected result is that the master will abort with an informative + * error message describing the set of its loaded coprocessors for crash diagnosis. + * (HBASE-4014). + */ +public class TestMasterCoprocessorException { + + public static class MasterTracker extends ZooKeeperNodeTracker { + public boolean masterZKNodeWasDeleted = false; + + public MasterTracker(ZooKeeperWatcher zkw,String masterNode, Abortable abortable) { + super(zkw,masterNode,abortable); + } + + @Override + public synchronized void nodeDeleted(String path) { + if (path.equals("/hbase/master")) { + masterZKNodeWasDeleted = true; + } + } + + + } + + public static class CreateTableThread extends Thread { + HBaseTestingUtility UTIL; + public CreateTableThread(HBaseTestingUtility UTIL) { + this.UTIL = UTIL; + } + + @Override + public void run() { + // create a table : master coprocessor will throw an exception and not catch it. + HTableDescriptor htd = new HTableDescriptor(TEST_TABLE); + htd.addFamily(new HColumnDescriptor(TEST_FAMILY)); + try { + HBaseAdmin admin = UTIL.getHBaseAdmin(); + admin.createTable(htd); + } + catch (Exception e) { + // it's ok that an exception occurs here, since the BuggyCoprocessor is + // intentionally written to cause an exception (namely a NPE). + } + } + } + + public static class BuggyCoprocessor extends BaseMasterObserver { + + private boolean preCreateTableCalled; + private boolean postCreateTableCalled; + private boolean preDeleteTableCalled; + private boolean postDeleteTableCalled; + private boolean preModifyTableCalled; + private boolean postModifyTableCalled; + private boolean preAddColumnCalled; + private boolean postAddColumnCalled; + private boolean preModifyColumnCalled; + private boolean postModifyColumnCalled; + private boolean preDeleteColumnCalled; + private boolean postDeleteColumnCalled; + private boolean preEnableTableCalled; + private boolean postEnableTableCalled; + private boolean preBalanceSwitchCalled; + private boolean startCalled; + private boolean postStartMasterCalled; + + @Override + public void preCreateTable(ObserverContext env, + HTableDescriptor desc, byte[][] splitKeys) throws IOException { + preCreateTableCalled = true; + } + + @Override + public void postCreateTable(ObserverContext env, + HRegionInfo[] regions, boolean sync) throws IOException { + // cause a NullPointerException and don't catch it: this will cause the master to abort(). + Integer i; + i = null; + i = i++; + } + + public boolean wasCreateTableCalled() { + return preCreateTableCalled && postCreateTableCalled; + } + + @Override + public void preAddColumn(ObserverContext env, + byte[] tableName, HColumnDescriptor column) throws IOException { + preAddColumnCalled = true; + } + + @Override + public void postAddColumn(ObserverContext env, + byte[] tableName, HColumnDescriptor column) throws IOException { + postAddColumnCalled = true; + } + + public boolean wasAddColumnCalled() { + return preAddColumnCalled && postAddColumnCalled; + } + + @Override + public void preModifyColumn(ObserverContext env, + byte[] tableName, HColumnDescriptor descriptor) throws IOException { + preModifyColumnCalled = true; + } + + @Override + public void postModifyColumn(ObserverContext env, + byte[] tableName, HColumnDescriptor descriptor) throws IOException { + postModifyColumnCalled = true; + } + + public boolean wasModifyColumnCalled() { + return preModifyColumnCalled && postModifyColumnCalled; + } + + @Override + public void preDeleteColumn(ObserverContext env, + byte[] tableName, byte[] c) throws IOException { + preDeleteColumnCalled = true; + } + + @Override + public void postDeleteColumn(ObserverContext env, + byte[] tableName, byte[] c) throws IOException { + postDeleteColumnCalled = true; + } + + public boolean wasDeleteColumnCalled() { + return preDeleteColumnCalled && postDeleteColumnCalled; + } + + @Override + public void preEnableTable(ObserverContext env, + byte[] tableName) throws IOException { + preEnableTableCalled = true; + } + + @Override + public void postEnableTable(ObserverContext env, + byte[] tableName) throws IOException { + postEnableTableCalled = true; + } + + public boolean wasEnableTableCalled() { + return preEnableTableCalled && postEnableTableCalled; + } + + @Override + public boolean preBalanceSwitch(ObserverContext env, boolean b) + throws IOException { + preBalanceSwitchCalled = true; + return b; + } + + @Override + public void postBalanceSwitch(ObserverContext env, + boolean oldValue, boolean newValue) throws IOException { + } + + @Override + public void postStartMaster(ObserverContext ctx) + throws IOException { + postStartMasterCalled = true; + } + + public boolean wasStartMasterCalled() { + return postStartMasterCalled; + } + + @Override + public void start(CoprocessorEnvironment env) throws IOException { + startCalled = true; + } + + public boolean wasStarted() { return startCalled; } + + } + + private static HBaseTestingUtility UTIL = new HBaseTestingUtility(); + private static byte[] TEST_TABLE = Bytes.toBytes("observed_table"); + private static byte[] TEST_FAMILY = Bytes.toBytes("fam1"); + + @BeforeClass + public static void setupBeforeClass() throws Exception { + Configuration conf = UTIL.getConfiguration(); + conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, + BuggyCoprocessor.class.getName()); + + UTIL.startMiniCluster(2); + } + + @AfterClass + public static void teardownAfterClass() throws Exception { + UTIL.shutdownMiniCluster(); + } + + // TODO: This is simply copied from TestMasterObserver : factor it out rather than copying. + @Test + public void testStarted() throws Exception { + MiniHBaseCluster cluster = UTIL.getHBaseCluster(); + + HMaster master = cluster.getMaster(); + assertTrue("Master should be active", master.isActiveMaster()); + MasterCoprocessorHost host = master.getCoprocessorHost(); + assertNotNull("CoprocessorHost should not be null", host); + BuggyCoprocessor cp = (BuggyCoprocessor)host.findCoprocessor( + BuggyCoprocessor.class.getName()); + assertNotNull("BuggyCoprocessor coprocessor not found or not installed!", cp); + + // check basic lifecycle + assertTrue("MasterObserver should have been started", cp.wasStarted()); + assertTrue("postStartMaster() hook should have been called", + cp.wasStartMasterCalled()); + } + + @Test + public void testExceptionFromCoprocessorWhenCreatingTable() throws IOException { + MiniHBaseCluster cluster = UTIL.getHBaseCluster(); + + HMaster master = cluster.getMaster(); + MasterCoprocessorHost host = master.getCoprocessorHost(); + BuggyCoprocessor cp = (BuggyCoprocessor)host.findCoprocessor( + BuggyCoprocessor.class.getName()); + assertFalse("No table created yet", cp.wasCreateTableCalled()); + + // set a watch on the zookeeper /hbase/master node. If the master dies, the node will be deleted. + ZooKeeperWatcher zkw = new ZooKeeperWatcher(UTIL.getConfiguration(), + "unittest", new Abortable() { + @Override + public void abort(String why, Throwable e) { + throw new RuntimeException("Fatal ZK error, why=" + why, e); + } + }); + + MasterTracker masterTracker = new MasterTracker(zkw,"/hbase/master", new Abortable() { + @Override + public void abort(String why, Throwable e) { + throw new RuntimeException("Fatal ZK master tracker error, why=", e); + } + }); + + masterTracker.start(); + zkw.registerListener(masterTracker); + + CreateTableThread createTableThread = new CreateTableThread(UTIL); + + // Attempting to create a table (using createTableThread above) triggers an NPE in BuggyCoprocessor. + // Master will then abort and the /hbase/master zk node will be deleted. + createTableThread.start(); + + // Wait up to 30 seconds for master's /hbase/master zk node to go away after master aborts. + for (int i = 0; i < 30; i++) { + if (masterTracker.masterZKNodeWasDeleted == true) { + break; + } + try { + Thread.sleep(1000); + } + catch (InterruptedException e) { + assertFalse("InterruptedException while waiting for master zk node to be deleted.",true); + } + } + + assertTrue("Master aborted on coprocessor exception, as expected.",masterTracker.masterZKNodeWasDeleted); + createTableThread.interrupt(); + try { + createTableThread.join(1000); + } + catch (InterruptedException e) { + assertTrue("Ignoring InterruptedException while waiting for createTableThread.join().",true); + } + } + +} diff --git src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorException.java src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorException.java new file mode 100644 index 0000000..1164ef6 --- /dev/null +++ src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorException.java @@ -0,0 +1,178 @@ +/* + * Copyright 2010 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.coprocessor; + +import java.io.IOException; +import java.security.PublicKey; +import java.util.List; +import java.util.Map; + +import com.google.common.collect.Table; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.*; +import org.apache.hadoop.hbase.client.HBaseAdmin; +import org.apache.hadoop.hbase.client.HTable; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.master.ServerManager; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.regionserver.HRegionServer; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.JVMClusterUtil; +import org.apache.hadoop.hbase.zookeeper.RegionServerTracker; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperListener; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperNodeTracker; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.apache.zookeeper.WatchedEvent; +import org.apache.zookeeper.ZooKeeper; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Tests unhandled exceptions thrown by coprocessors running on a regionserver.. + * Expected result is that the regionserver will abort with an informative + * error message describing the set of its loaded coprocessors for crash diagnosis. + * (HBASE-4014). + */ +public class TestRegionServerCoprocessorException { + + public boolean rsZKNodeWasDeleted = false; + + private class DeadRegionServerTracker extends ZooKeeperNodeTracker { + + private String rsNode; + + public DeadRegionServerTracker(ZooKeeperWatcher zkw, String rsNode, Abortable abortable) { + super(zkw,rsNode,abortable); + this.rsNode = rsNode; + } + + @Override + public synchronized void nodeDeleted(String path) { + if (path.equals(rsNode)) { + rsZKNodeWasDeleted = true; + } + } + } + + private class WriteToTableThread extends Thread { + public HTable table; + + public WriteToTableThread(HTable table) throws IOException { + this.table = table; + } + + @Override + public void run() { + // write to the table: : regionserver coprocessor will throw an exception, causing the regionserver to abort. + final byte[] ROW = Bytes.toBytes("bbb"); + final byte[] TEST_FAMILY = Bytes.toBytes("aaa"); + Put put = new Put(ROW); + put.add(TEST_FAMILY, ROW, ROW); + try { + table.put(put); + } + catch (IOException e) { + assertFalse("failed to put to table.",true); + } + } + } + + private static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + + private static ZooKeeperWatcher zkw = null; + + @BeforeClass + public static void setupBeforeClass() throws Exception { + // set configure to indicate which cp should be loaded + Configuration conf = TEST_UTIL.getConfiguration(); + conf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, BuggyCoprocessor.class.getName()); + TEST_UTIL.startMiniCluster(2); + } + + @AfterClass + public static void teardownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + @Test + public void testExceptionFromCoprocessorWhenCreatingTable() throws IOException { + // Set watches on the zookeeper nodes for all of the regionservers in the cluster. + // When we try to write to TEST_TABLE, the buggy coprocessor will cause a NullPointerException, + // which will cause the regionserver (which hosts the region we attempted to write to) to abort. + // In turn, this will cause the nodeDeleted() method of the DeadRegionServer tracker to execute, + // which will set the rsZKNodeDeleted flag to true, which will pass this test. + + byte[] TEST_TABLE = Bytes.toBytes("observed_table"); + byte[] TEST_FAMILY = Bytes.toBytes("aaa"); + + HTable table = TEST_UTIL.createTable(TEST_TABLE, TEST_FAMILY); + TEST_UTIL.createMultiRegions(table, TEST_FAMILY); + + // Create a tracker for each RegionServer. + // As a future improvement, should only be necessary to track a single regionserver (the one + // hosting the region that we are going to attempt to write to). + // For now, we don't know how to determine which RegionServer this is, so we track all of them. + List threads = TEST_UTIL.getHBaseCluster().getRegionServerThreads(); + for(JVMClusterUtil.RegionServerThread rst : threads) { + String rsNodeString = "/hbase/rs/" + rst.getRegionServer().getServerName(); + zkw = TEST_UTIL.getHBaseAdmin().getConnection().getZooKeeperWatcher(); + DeadRegionServerTracker regionServerTracker = new DeadRegionServerTracker(zkw, + rsNodeString, + new Abortable() { + @Override + public void abort(String why, Throwable e) { + assertFalse("regionServerTracker failed because: " + why,true); + } + }); + regionServerTracker.start(); + // TODO: do we need to registerListener()? or is it already taken care of in the super constructor? + zkw.registerListener(regionServerTracker); + } + + // Attempting to write TEST_TABLE will trigger BuggyCoprocessor's NPE. + // The hosting region server will then abort and the regionserver's ephemeral node in /hbase/rs will be deleted. + WriteToTableThread writeToTableThread = new WriteToTableThread(table); + writeToTableThread.start(); + + // Wait up to 30 seconds for regionserver's ephemeral node to go away after the regionserver aborts. + for (int i = 0; i < 30; i++) { + if (rsZKNodeWasDeleted == true) { + break; + } + try { + Thread.sleep(1000); + } + catch (InterruptedException e) { + assertFalse("InterruptedException while waiting for regionserver zk node to be deleted.",true); + } + } + + assertTrue("RegionServer aborted on coprocessor exception, as expected.",rsZKNodeWasDeleted); + writeToTableThread.interrupt(); + TEST_UTIL.shutdownMiniCluster(); + } + +}