From f97e04379480fb777948f0991e7a4b1b9af7efda Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Thu, 26 Oct 2017 14:55:53 -0700 Subject: [PATCH] HBASE-18770 Remove bypass method in ObserverContext and implement the 'bypass' logic case by case Changes Coprocessor context 'bypass' semantic. Changes default so it is NOT supported; only a couple of preXXX methods in RegionObserver allow it: e.g. preGet and prePut but not preFlush, etc. Changes Coprocessor 'complete' semantic too. It only has an effect on those methods that support 'bypass'; i.e. you can only exit a Coprocessor chain early via 'complete' on methods that are bypassable. See javadoc for whether a Coprocessor Observer method supports 'bypass'. If no mention, 'bypass' is NOT supported. M hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java Added passing of 'bypassable' (and 'completable') argument to the Operation constructors. Methods that support 'bypass' must set this flag. M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Refactoring in here is minor. A few methods that used support bypass no longer do so removed the check and the need of an if/else meant a left-shift in some code. M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java Ditto M hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java In here label explicitly those methods that are bypassable. Some changes to make sure we call the corresponding execOperation. TODO: What to do w/ the Scanner methods. --- .../hadoop/hbase/coprocessor/CoprocessorHost.java | 82 ++-- .../coprocessor/DoesNotSupportBypassException.java | 26 +- .../DoesNotSupportCompleteException.java | 24 ++ .../hadoop/hbase/coprocessor/MasterObserver.java | 43 +-- .../hadoop/hbase/coprocessor/ObserverContext.java | 25 +- .../hbase/coprocessor/ObserverContextImpl.java | 30 +- .../org/apache/hadoop/hbase/master/HMaster.java | 48 +-- .../hadoop/hbase/master/MasterCoprocessorHost.java | 76 ++-- .../hadoop/hbase/master/MasterRpcServices.java | 15 +- .../assignment/MergeTableRegionsProcedure.java | 15 +- .../assignment/SplitTableRegionProcedure.java | 5 +- .../apache/hadoop/hbase/regionserver/HRegion.java | 70 ++-- .../apache/hadoop/hbase/regionserver/HStore.java | 8 +- .../hadoop/hbase/regionserver/RSRpcServices.java | 25 +- .../hbase/regionserver/RegionCoprocessorHost.java | 415 +++++++++++++-------- .../regionserver/RegionServerCoprocessorHost.java | 8 +- .../hbase/regionserver/SecureBulkLoadManager.java | 75 ++-- .../hbase/regionserver/wal/WALCoprocessorHost.java | 9 +- .../hbase/coprocessor/TestMasterObserver.java | 153 +------- .../hbase/mob/compactions/TestMobCompactor.java | 3 + .../hadoop/hbase/regionserver/TestHRegion.java | 18 +- 21 files changed, 553 insertions(+), 620 deletions(-) rename hbase-client/src/main/java/org/apache/hadoop/hbase/coprocessor/BypassCoprocessorException.java => hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/DoesNotSupportBypassException.java (64%) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/DoesNotSupportCompleteException.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index c785b0b594..c367e18400 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -1,5 +1,4 @@ /* - * * 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 @@ -36,14 +35,12 @@ import java.util.function.Function; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.yetus.audience.InterfaceStability; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; -import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.security.User; @@ -548,11 +545,20 @@ public abstract class CoprocessorHost observerGetter; ObserverOperation(ObserverGetter observerGetter) { - this(observerGetter, RpcServer.getRequestUser().orElse(null)); + this(observerGetter, null, false); } ObserverOperation(ObserverGetter observerGetter, User user) { - super(user); + this(observerGetter, user, false); + } + + ObserverOperation(ObserverGetter observerGetter, boolean bypassable) { + this(observerGetter, null, bypassable); + } + + ObserverOperation(ObserverGetter observerGetter, User user, boolean bypassable) { + super(user != null? user: RpcServer.getRequestUser().orElse(null), + bypassable, bypassable/*'completable': make completable same as bypassable*/); this.observerGetter = observerGetter; } @@ -574,6 +580,11 @@ public abstract class CoprocessorHost observerGetter, User user, + boolean bypassable) { + super(observerGetter, user, bypassable); + } + /** * In case of coprocessors which have many kinds of observers (for eg, {@link RegionCoprocessor} * has BulkLoadObserver, RegionObserver, etc), some implementations may not need all @@ -594,15 +605,23 @@ public abstract class CoprocessorHost observerGetter) { - super(observerGetter); + public ObserverOperationWithResult(ObserverGetter observerGetter, R result) { + this(observerGetter, result, false); } - public ObserverOperationWithResult(ObserverGetter observerGetter, User user) { - super(observerGetter, user); + public ObserverOperationWithResult(ObserverGetter observerGetter, R result, + boolean bypassable) { + this(observerGetter, result, null, bypassable); + } + + public ObserverOperationWithResult(ObserverGetter observerGetter, R result, + User user) { + this(observerGetter, result, user, false); } - void setResult(final R result) { + private ObserverOperationWithResult(ObserverGetter observerGetter, R result, User user, + boolean bypassable) { + super(observerGetter, user, bypassable); this.result = result; } @@ -621,38 +640,25 @@ public abstract class CoprocessorHost R execOperationWithResult(final R defaultValue, - final ObserverOperationWithResult observerOperation) throws IOException { - if (observerOperation == null) { - return defaultValue; - } - observerOperation.setResult(defaultValue); - execOperation(observerOperation); - return observerOperation.getResult(); - } - // what does bypass mean? - protected R execOperationWithResult(final boolean ifBypass, final R defaultValue, + /** + * Do not call with an observerOperation that is null! Have the caller check. + */ + protected R execOperationWithResult( final ObserverOperationWithResult observerOperation) throws IOException { - if (observerOperation == null) { - return ifBypass ? null : defaultValue; - } else { - observerOperation.setResult(defaultValue); - boolean bypass = execOperation(true, observerOperation); - R result = observerOperation.getResult(); - return bypass == ifBypass ? result : null; - } + boolean bypass = execOperation(observerOperation); + R result = observerOperation.getResult(); + return bypass == observerOperation.isBypassable()? result: null; } + /** + * @return True if we are to bypass (Can only be true if + * {@link ObserverOperation#isBypassable()} + */ protected boolean execOperation(final ObserverOperation observerOperation) throws IOException { - return execOperation(true, observerOperation); - } - - protected boolean execOperation(final boolean earlyExit, - final ObserverOperation observerOperation) throws IOException { - if (observerOperation == null) return false; boolean bypass = false; + if (observerOperation == null) return bypass; List envs = coprocEnvironments.get(); for (E env : envs) { observerOperation.prepare(env); @@ -666,8 +672,10 @@ public abstract class CoprocessorHostctx.bypass() will not have any - * impact on this hook. + * Called prior to marking a given region as offline. * @param ctx the environment to interact with the framework and master * @param regionInfo */ @@ -538,8 +522,7 @@ public interface MasterObserver { final RegionInfo regionInfoB) throws IOException {} /** - * This will be called before update META step as part of split transaction. Calling - * {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} rollback the split + * This will be called before update META step as part of split transaction. * @param ctx the environment to interact with the framework and master * @param splitKey * @param metaEntries @@ -552,8 +535,6 @@ public interface MasterObserver { /** * This will be called after update META step as part of split transaction - * Calling {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} has no - * effect in this hook. * @param ctx the environment to interact with the framework and master */ default void preSplitRegionAfterMETAAction( @@ -570,7 +551,6 @@ public interface MasterObserver { /** * Called before the regions merge. - * Call {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} to skip the merge. * @param ctx the environment to interact with the framework and master */ default void preMergeRegionsAction( @@ -587,8 +567,7 @@ public interface MasterObserver { final RegionInfo mergedRegion) throws IOException {} /** - * This will be called before update META step as part of regions merge transaction. Calling - * {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} rollback the merge + * This will be called before update META step as part of regions merge transaction. * @param ctx the environment to interact with the framework and master * @param metaEntries mutations to execute on hbase:meta atomically with regions merge updates. * Any puts or deletes to execute on hbase:meta can be added to the mutations. @@ -667,7 +646,6 @@ public interface MasterObserver { /** * Called before a new snapshot is taken. * Called as part of snapshot RPC call. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor for the snapshot * @param tableDescriptor the TableDescriptor of the table to snapshot @@ -689,7 +667,6 @@ public interface MasterObserver { /** * Called before listSnapshots request has been processed. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor of the snapshot to list */ @@ -698,7 +675,6 @@ public interface MasterObserver { /** * Called after listSnapshots request has been processed. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor of the snapshot to list */ @@ -708,7 +684,6 @@ public interface MasterObserver { /** * Called before a snapshot is cloned. * Called as part of restoreSnapshot RPC call. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor for the snapshot * @param tableDescriptor the TableDescriptor of the table to create @@ -731,7 +706,6 @@ public interface MasterObserver { /** * Called before a snapshot is restored. * Called as part of restoreSnapshot RPC call. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor for the snapshot * @param tableDescriptor the TableDescriptor of the table to restore @@ -754,7 +728,6 @@ public interface MasterObserver { /** * Called before a snapshot is deleted. * Called as part of deleteSnapshot RPC call. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param snapshot the SnapshotDescriptor of the snapshot to delete */ @@ -774,7 +747,7 @@ public interface MasterObserver { * Called before a getTableDescriptors request has been processed. * @param ctx the environment to interact with the framework and master * @param tableNamesList the list of table names, or null if querying for all - * @param descriptors an empty list, can be filled with what to return if bypassing + * @param descriptors an empty list, can be filled with what to return in coprocessor * @param regex regular expression used for filtering the table names */ default void preGetTableDescriptors(ObserverContext ctx, @@ -795,7 +768,7 @@ public interface MasterObserver { /** * Called before a getTableNames request has been processed. * @param ctx the environment to interact with the framework and master - * @param descriptors an empty list, can be filled with what to return if bypassing + * @param descriptors an empty list, can be filled with what to return by coprocessor * @param regex regular expression used for filtering the table names */ default void preGetTableNames(ObserverContext ctx, @@ -815,7 +788,6 @@ public interface MasterObserver { /** * Called before a new namespace is created by * {@link org.apache.hadoop.hbase.master.HMaster}. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param ns the NamespaceDescriptor for the table */ @@ -832,7 +804,6 @@ public interface MasterObserver { /** * Called before {@link org.apache.hadoop.hbase.master.HMaster} deletes a * namespace - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param namespace the name of the namespace */ @@ -849,7 +820,6 @@ public interface MasterObserver { /** * Called prior to modifying a namespace's properties. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx the environment to interact with the framework and master * @param ns the NamespaceDescriptor */ @@ -883,7 +853,7 @@ public interface MasterObserver { /** * Called before a listNamespaceDescriptors request has been processed. * @param ctx the environment to interact with the framework and master - * @param descriptors an empty list, can be filled with what to return if bypassing + * @param descriptors an empty list, can be filled with what to return by coprocessor */ default void preListNamespaceDescriptors(ObserverContext ctx, List descriptors) throws IOException {} @@ -1013,7 +983,6 @@ public interface MasterObserver { /** * Called before merge regions request. - * It can't bypass the default action, e.g., ctx.bypass() won't have effect. * @param ctx coprocessor environment * @param regionsToMerge regions to be merged */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java index 3cb054b44d..78ba226625 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java @@ -28,7 +28,7 @@ import java.util.Optional; /** * Carries the execution state for a given invocation of an Observer coprocessor * ({@link RegionObserver}, {@link MasterObserver}, or {@link WALObserver}) - * method. The same ObserverContext instance is passed sequentially to all loaded + * method. The same ObserverContext instance is passed sequentially to all loaded * coprocessors for a given Observer method trigger, with the * CoprocessorEnvironment reference swapped out for each * coprocessor. @@ -42,16 +42,26 @@ public interface ObserverContext { /** * Call to indicate that the current coprocessor's return value should be - * used in place of the normal HBase obtained value. + * used in place of the value that would be obtained via normal processing; i.e. bypass and + * return the Coprocessor's result instead. DOES NOT work on all invocations, only on a small + * subset of methods, mostly preXXX calls in RegionObserver. Check javadoc on the pertinent + * Coprocessor Observer to see if bypass is supported. + * This behavior of honoring only a subset of methods is new since hbase-2.0.0. + * @exception CoprocessorException Thrown if you call bypass on a method that is not + * bypassable. */ - void bypass(); + void bypass() throws CoprocessorException; /** - * Call to indicate that additional coprocessors further down the execution - * chain do not need to be invoked. Implies that this coprocessor's response - * is definitive. + * Call to indicate that additional coprocessors further down the execution chain should be + * skipped and not run. Implies that this coprocessor's response is definitive. + * Since hbase-2.0.0, only complete of bypassable methods has an effect. See javadoc + * on the Coprocessor Observer method as to whether bypass (and thereby 'complete') is + * supported. + * This behavior of honoring only a subset of methods is new * since hbase-2.0.0. + * @exception CoprocessorException Thrown if you call complete on a non-completable method. */ - void complete(); + void complete() throws CoprocessorException; /** * Returns the active user for the coprocessor call. If an explicit {@code User} instance was @@ -60,5 +70,4 @@ public interface ObserverContext { * context. */ Optional getCaller(); - } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContextImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContextImpl.java index ff829564b6..245adf5430 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContextImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContextImpl.java @@ -35,11 +35,25 @@ import org.apache.yetus.audience.InterfaceStability; public class ObserverContextImpl implements ObserverContext { private E env; private boolean bypass; + /** + * Is this operation bypassable? + */ + private final boolean bypassable; + /** + * Is this operation completable? + */ private boolean complete; + private final boolean completable; private final User caller; public ObserverContextImpl(User caller) { + this(caller, false, false); + } + + public ObserverContextImpl(User caller, boolean bypassable, boolean completable) { this.caller = caller; + this.bypassable = bypassable; + this.completable = completable; } public E getEnvironment() { @@ -50,11 +64,21 @@ public class ObserverContextImpl implements Ob this.env = env; } - public void bypass() { + public boolean isBypassable() {return this.bypassable;}; + + public void bypass() throws DoesNotSupportBypassException { + if (!this.bypassable) { + throw new DoesNotSupportBypassException(); + } bypass = true; } - public void complete() { + public boolean isCompleable() {return this.completable;}; + + public void complete() throws DoesNotSupportCompleteException { + if (!this.completable) { + throw new DoesNotSupportCompleteException(); + } complete = true; } @@ -63,6 +87,7 @@ public class ObserverContextImpl implements Ob * coprocessors, {@code false} otherwise. */ public boolean shouldBypass() { + if (!isBypassable()) return false; if (bypass) { bypass = false; return true; @@ -75,6 +100,7 @@ public class ObserverContextImpl implements Ob * coprocessors, {@code false} otherwise. */ public boolean shouldComplete() { + if (!isCompleable()) return false; if (complete) { complete = false; return true; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index a990a4bce4..23bc9c050d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -58,7 +58,6 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.ClusterStatus; import org.apache.hadoop.hbase.ClusterStatus.Option; import org.apache.hadoop.hbase.CoordinatedStateException; -import org.apache.hadoop.hbase.CoordinatedStateManager; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HBaseInterfaceAudience; @@ -83,7 +82,6 @@ import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.client.TableState; -import org.apache.hadoop.hbase.coprocessor.BypassCoprocessorException; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.exceptions.MergeRegionException; @@ -1703,9 +1701,7 @@ public class HMaster extends HRegionServer implements MasterServices { try { checkInitialized(); if (this.cpHost != null) { - if (this.cpHost.preMove(hri, rp.getSource(), rp.getDestination())) { - return; - } + this.cpHost.preMove(hri, rp.getSource(), rp.getDestination()); } // Warmup the region on the destination before initiating the move. this call // is synchronous and takes some time. doing it before the source region gets @@ -2898,9 +2894,7 @@ public class HMaster extends HRegionServer implements MasterServices { new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) { @Override protected void run() throws IOException { - if (getMaster().getMasterCoprocessorHost().preCreateNamespace(namespaceDescriptor)) { - throw new BypassCoprocessorException(); - } + getMaster().getMasterCoprocessorHost().preCreateNamespace(namespaceDescriptor); LOG.info(getClientIdAuditPrefix() + " creating " + namespaceDescriptor); // Execute the operation synchronously - wait for the operation to complete before // continuing. @@ -2932,9 +2926,7 @@ public class HMaster extends HRegionServer implements MasterServices { new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) { @Override protected void run() throws IOException { - if (getMaster().getMasterCoprocessorHost().preModifyNamespace(namespaceDescriptor)) { - throw new BypassCoprocessorException(); - } + getMaster().getMasterCoprocessorHost().preModifyNamespace(namespaceDescriptor); LOG.info(getClientIdAuditPrefix() + " modify " + namespaceDescriptor); // Execute the operation synchronously - wait for the operation to complete before // continuing. @@ -2964,9 +2956,7 @@ public class HMaster extends HRegionServer implements MasterServices { new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) { @Override protected void run() throws IOException { - if (getMaster().getMasterCoprocessorHost().preDeleteNamespace(name)) { - throw new BypassCoprocessorException(); - } + getMaster().getMasterCoprocessorHost().preDeleteNamespace(name); LOG.info(getClientIdAuditPrefix() + " delete " + name); // Execute the operation synchronously - wait for the operation to complete before // continuing. @@ -3001,14 +2991,11 @@ public class HMaster extends HRegionServer implements MasterServices { List getNamespaces() throws IOException { checkInitialized(); final List nsds = new ArrayList<>(); - boolean bypass = false; if (cpHost != null) { - bypass = cpHost.preListNamespaceDescriptors(nsds); - } - if (!bypass) { - nsds.addAll(this.clusterSchemaService.getNamespaces()); - if (this.cpHost != null) this.cpHost.postListNamespaceDescriptors(nsds); + cpHost.preListNamespaceDescriptors(nsds); } + nsds.addAll(this.clusterSchemaService.getNamespaces()); + if (this.cpHost != null) this.cpHost.postListNamespaceDescriptors(nsds); return nsds; } @@ -3084,13 +3071,12 @@ public class HMaster extends HRegionServer implements MasterServices { final List tableNameList, final boolean includeSysTables) throws IOException { List htds = new ArrayList<>(); - boolean bypass = cpHost != null? - cpHost.preGetTableDescriptors(tableNameList, htds, regex): false; - if (!bypass) { - htds = getTableDescriptors(htds, namespace, regex, tableNameList, includeSysTables); - if (cpHost != null) { - cpHost.postGetTableDescriptors(tableNameList, htds, regex); - } + if (cpHost != null) { + cpHost.preGetTableDescriptors(tableNameList, htds, regex); + } + htds = getTableDescriptors(htds, namespace, regex, tableNameList, includeSysTables); + if (cpHost != null) { + cpHost.postGetTableDescriptors(tableNameList, htds, regex); } return htds; } @@ -3105,11 +3091,9 @@ public class HMaster extends HRegionServer implements MasterServices { public List listTableNames(final String namespace, final String regex, final boolean includeSysTables) throws IOException { List htds = new ArrayList<>(); - boolean bypass = cpHost != null? cpHost.preGetTableNames(htds, regex): false; - if (!bypass) { - htds = getTableDescriptors(htds, namespace, regex, null, includeSysTables); - if (cpHost != null) cpHost.postGetTableNames(htds, regex); - } + if (cpHost != null) cpHost.preGetTableNames(htds, regex); + htds = getTableDescriptors(htds, namespace, regex, null, includeSysTables); + if (cpHost != null) cpHost.postGetTableNames(htds, regex); List result = new ArrayList<>(htds.size()); for (TableDescriptor htd: htds) result.add(htd.getTableName()); return result; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index fa2a0a9dc6..0fd8ebc7fc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -203,8 +203,8 @@ public class MasterCoprocessorHost ////////////////////////////////////////////////////////////////////////////////////////////////// - public boolean preCreateNamespace(final NamespaceDescriptor ns) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preCreateNamespace(final NamespaceDescriptor ns) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preCreateNamespace(this, ns); @@ -221,8 +221,8 @@ public class MasterCoprocessorHost }); } - public boolean preDeleteNamespace(final String namespaceName) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preDeleteNamespace(final String namespaceName) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preDeleteNamespace(this, namespaceName); @@ -239,8 +239,8 @@ public class MasterCoprocessorHost }); } - public boolean preModifyNamespace(final NamespaceDescriptor ns) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preModifyNamespace(final NamespaceDescriptor ns) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preModifyNamespace(this, ns); @@ -277,9 +277,9 @@ public class MasterCoprocessorHost }); } - public boolean preListNamespaceDescriptors(final List descriptors) + public void preListNamespaceDescriptors(final List descriptors) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preListNamespaceDescriptors(this, descriptors); @@ -528,10 +528,10 @@ public class MasterCoprocessorHost }); } - public boolean preAbortProcedure( + public void preAbortProcedure( final ProcedureExecutor procEnv, final long procId) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preAbortProcedure(this, procId); @@ -548,8 +548,8 @@ public class MasterCoprocessorHost }); } - public boolean preGetProcedures() throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preGetProcedures() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preGetProcedures(this); @@ -566,8 +566,8 @@ public class MasterCoprocessorHost }); } - public boolean preGetLocks() throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preGetLocks() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preGetLocks(this); @@ -584,9 +584,9 @@ public class MasterCoprocessorHost }); } - public boolean preMove(final RegionInfo region, final ServerName srcServer, + public void preMove(final RegionInfo region, final ServerName srcServer, final ServerName destServer) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preMove(this, region, srcServer, destServer); @@ -604,8 +604,8 @@ public class MasterCoprocessorHost }); } - public boolean preAssign(final RegionInfo regionInfo) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + public void preAssign(final RegionInfo regionInfo) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preAssign(this, regionInfo); @@ -622,9 +622,9 @@ public class MasterCoprocessorHost }); } - public boolean preUnassign(final RegionInfo regionInfo, final boolean force) + public void preUnassign(final RegionInfo regionInfo, final boolean force) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preUnassign(this, regionInfo, force); @@ -697,9 +697,9 @@ public class MasterCoprocessorHost }); } - public boolean preSetSplitOrMergeEnabled(final boolean newValue, + public void preSetSplitOrMergeEnabled(final boolean newValue, final MasterSwitchType switchType) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preSetSplitOrMergeEnabled(this, newValue, switchType); @@ -779,11 +779,11 @@ public class MasterCoprocessorHost * @param user the user * @throws IOException */ - public boolean preSplitBeforeMETAAction( + public void preSplitBeforeMETAAction( final byte[] splitKey, final List metaEntries, final User user) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { @Override public void call(MasterObserver observer) throws IOException { observer.preSplitRegionBeforeMETAAction(this, splitKey, metaEntries); @@ -825,9 +825,9 @@ public class MasterCoprocessorHost * @param user the user * @throws IOException */ - public boolean preMergeRegionsAction( + public void preMergeRegionsAction( final RegionInfo[] regionsToMerge, final User user) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { @Override public void call(MasterObserver observer) throws IOException { observer.preMergeRegionsAction(this, regionsToMerge); @@ -861,11 +861,11 @@ public class MasterCoprocessorHost * @param user the user * @throws IOException */ - public boolean preMergeRegionsCommit( + public void preMergeRegionsCommit( final RegionInfo[] regionsToMerge, final @MetaMutationAnnotation List metaEntries, final User user) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { @Override public void call(MasterObserver observer) throws IOException { observer.preMergeRegionsCommitAction(this, regionsToMerge, metaEntries); @@ -908,9 +908,11 @@ public class MasterCoprocessorHost }); } + // This hook allows Coprocessor change value of balance switch. public boolean preBalanceSwitch(final boolean b) throws IOException { - return execOperationWithResult(b, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(masterObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return b; + return execOperationWithResult( + new ObserverOperationWithResult(masterObserverGetter, b) { @Override public Boolean call(MasterObserver observer) throws IOException { return observer.preBalanceSwitch(this, getResult()); @@ -931,7 +933,8 @@ public class MasterCoprocessorHost public void preShutdown() throws IOException { // While stopping the cluster all coprocessors method should be executed first then the // coprocessor should be cleaned up. - execShutdown(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + if (coprocEnvironments.isEmpty()) return; + execShutdown(new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preShutdown(this); @@ -947,7 +950,8 @@ public class MasterCoprocessorHost public void preStopMaster() throws IOException { // While stopping master all coprocessors method should be executed first then the coprocessor // environment should be cleaned up. - execShutdown(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + if(coprocEnvironments.isEmpty()) return; + execShutdown(new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preStopMaster(this); @@ -1074,9 +1078,9 @@ public class MasterCoprocessorHost }); } - public boolean preGetTableDescriptors(final List tableNamesList, + public void preGetTableDescriptors(final List tableNamesList, final List descriptors, final String regex) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preGetTableDescriptors(this, tableNamesList, descriptors, regex); @@ -1094,9 +1098,9 @@ public class MasterCoprocessorHost }); } - public boolean preGetTableNames(final List descriptors, + public void preGetTableNames(final List descriptors, final String regex) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override public void call(MasterObserver observer) throws IOException { observer.preGetTableNames(this, descriptors, regex); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java index 1bd6487ff6..3a2fc7ed53 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java @@ -509,9 +509,7 @@ public class MasterRpcServices extends RSRpcServices final AssignRegionResponse arr = AssignRegionResponse.newBuilder().build(); if (master.cpHost != null) { - if (master.cpHost.preAssign(regionInfo)) { - return arr; - } + master.cpHost.preAssign(regionInfo); } LOG.info(master.getClientIdAuditPrefix() + " assign " + regionInfo.getRegionNameAsString()); master.getAssignmentManager().assign(regionInfo, true); @@ -1517,9 +1515,7 @@ public class MasterRpcServices extends RSRpcServices RegionInfo hri = pair.getFirst(); if (master.cpHost != null) { - if (master.cpHost.preUnassign(hri, force)) { - return urr; - } + master.cpHost.preUnassign(hri, force); } LOG.debug(master.getClientIdAuditPrefix() + " unassign " + hri.getRegionNameAsString() + " in current location if it is online and reassign.force=" + force); @@ -1704,13 +1700,10 @@ public class MasterRpcServices extends RSRpcServices MasterSwitchType switchType = convert(masterSwitchType); boolean oldValue = master.isSplitOrMergeEnabled(switchType); response.addPrevValue(oldValue); - boolean bypass = false; if (master.cpHost != null) { - bypass = master.cpHost.preSetSplitOrMergeEnabled(newValue, switchType); - } - if (!bypass) { - master.getSplitOrMergeTracker().setSplitOrMergeEnabled(newValue, switchType); + master.cpHost.preSetSplitOrMergeEnabled(newValue, switchType); } + master.getSplitOrMergeTracker().setSplitOrMergeEnabled(newValue, switchType); if (master.cpHost != null) { master.cpHost.postSetSplitOrMergeEnabled(newValue, switchType); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java index da6afc906c..642bb0d130 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java @@ -530,12 +530,7 @@ public class MergeTableRegionsProcedure private void preMergeRegions(final MasterProcedureEnv env) throws IOException { final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); if (cpHost != null) { - boolean ret = cpHost.preMergeRegionsAction(regionsToMerge, getUser()); - if (ret) { - throw new IOException( - "Coprocessor bypassing regions " + RegionInfo.getShortNameToLog(regionsToMerge) + - " merge."); - } + cpHost.preMergeRegionsAction(regionsToMerge, getUser()); } // TODO: Clean up split and merge. Currently all over the place. try { @@ -702,13 +697,7 @@ public class MergeTableRegionsProcedure if (cpHost != null) { @MetaMutationAnnotation final List metaEntries = new ArrayList(); - boolean ret = cpHost.preMergeRegionsCommit(regionsToMerge, metaEntries, getUser()); - - if (ret) { - throw new IOException( - "Coprocessor bypassing regions " + RegionInfo.getShortNameToLog(regionsToMerge) + - " merge."); - } + cpHost.preMergeRegionsCommit(regionsToMerge, metaEntries, getUser()); try { for (Mutation p : metaEntries) { RegionInfo.parseRegionName(p.getRow()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java index 78ed7b4935..201d0aef03 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java @@ -709,10 +709,7 @@ public class SplitTableRegionProcedure final List metaEntries = new ArrayList(); final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); if (cpHost != null) { - if (cpHost.preSplitBeforeMETAAction(getSplitRow(), metaEntries, getUser())) { - throw new IOException("Coprocessor bypassing region " + - getParentRegion().getRegionNameAsString() + " split."); - } + cpHost.preSplitBeforeMETAAction(getSplitRow(), metaEntries, getUser()); try { for (Mutation p : metaEntries) { RegionInfo.parseRegionName(p.getRow()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 4d0f6d00b6..e5e97e1a6a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -3416,43 +3416,39 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi MiniBatchOperationInProgress miniBatchOp = new MiniBatchOperationInProgress<>(batchOp.getMutationsForCoprocs(), batchOp.retCodeDetails, batchOp.walEditsFromCoprocessors, firstIndex, lastIndexExclusive); - if (coprocessorHost.preBatchMutate(miniBatchOp)) { - doneByCoprocessor = true; - return; - } else { - for (int i = firstIndex; i < lastIndexExclusive; i++) { - if (batchOp.retCodeDetails[i].getOperationStatusCode() != OperationStatusCode.NOT_RUN) { - // lastIndexExclusive was incremented above. - continue; - } - // we pass (i - firstIndex) below since the call expects a relative index - Mutation[] cpMutations = miniBatchOp.getOperationsFromCoprocessors(i - firstIndex); - if (cpMutations == null) { - continue; - } - Mutation mutation = batchOp.getMutation(i); - boolean skipWal = getEffectiveDurability(mutation.getDurability()) == Durability.SKIP_WAL; - // Else Coprocessor added more Mutations corresponding to the Mutation at this index. - for (int j = 0; j < cpMutations.length; j++) { - Mutation cpMutation = cpMutations[j]; - checkAndPrepareMutation(cpMutation, replay, now); - - // Acquire row locks. If not, the whole batch will fail. - acquiredRowLocks.add(getRowLockInternal(cpMutation.getRow(), true)); - - // Returned mutations from coprocessor correspond to the Mutation at index i. We can - // directly add the cells from those mutations to the familyMaps of this mutation. - Map> cpFamilyMap = cpMutation.getFamilyCellMap(); - // will get added to the memStore later - mergeFamilyMaps(batchOp.familyCellMaps[i], cpFamilyMap); - - // The durability of returned mutation is replaced by the corresponding mutation. - // If the corresponding mutation contains the SKIP_WAL, we shouldn't count the - // cells of returned mutation. - if (!skipWal) { - for (List cells : cpFamilyMap.values()) { - cellCount += cells.size(); - } + coprocessorHost.preBatchMutate(miniBatchOp); + for (int i = firstIndex; i < lastIndexExclusive; i++) { + if (batchOp.retCodeDetails[i].getOperationStatusCode() != OperationStatusCode.NOT_RUN) { + // lastIndexExclusive was incremented above. + continue; + } + // we pass (i - firstIndex) below since the call expects a relative index + Mutation[] cpMutations = miniBatchOp.getOperationsFromCoprocessors(i - firstIndex); + if (cpMutations == null) { + continue; + } + Mutation mutation = batchOp.getMutation(i); + boolean skipWal = getEffectiveDurability(mutation.getDurability()) == Durability.SKIP_WAL; + // Else Coprocessor added more Mutations corresponding to the Mutation at this index. + for (int j = 0; j < cpMutations.length; j++) { + Mutation cpMutation = cpMutations[j]; + checkAndPrepareMutation(cpMutation, replay, now); + + // Acquire row locks. If not, the whole batch will fail. + acquiredRowLocks.add(getRowLockInternal(cpMutation.getRow(), true)); + + // Returned mutations from coprocessor correspond to the Mutation at index i. We can + // directly add the cells from those mutations to the familyMaps of this mutation. + Map> cpFamilyMap = cpMutation.getFamilyCellMap(); + // will get added to the memStore later + mergeFamilyMaps(batchOp.familyCellMaps[i], cpFamilyMap); + + // The durability of returned mutation is replaced by the corresponding mutation. + // If the corresponding mutation contains the SKIP_WAL, we shouldn't count the + // cells of returned mutation. + if (!skipWal) { + for (List cells : cpFamilyMap.values()) { + cellCount += cells.size(); } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index 7b8ca79ab5..92df3b2875 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -936,7 +936,7 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat /** * Snapshot this stores memstore. Call before running - * {@link #flushCache(long, MemStoreSnapshot, MonitoredTask, ThroughputController)} + * #flushCache(long, MemStoreSnapshot, MonitoredTask, ThroughputController) * so it has some work to do. */ void snapshot() { @@ -1670,10 +1670,8 @@ public class HStore implements Store, HeapSize, StoreConfigInformation, Propagat // First, see if coprocessor would want to override selection. if (this.getCoprocessorHost() != null) { final List candidatesForCoproc = compaction.preSelect(this.filesCompacting); - boolean override = false; - //TODO: is it correct way to get CompactionRequest? - override = getCoprocessorHost().preCompactSelection(this, candidatesForCoproc, - tracker, user); + boolean override = getCoprocessorHost().preCompactSelection(this, + candidatesForCoproc, tracker, user); if (override) { // Coprocessor is overriding normal file selection. compaction.forceSelect(new CompactionRequestImpl(candidatesForCoproc)); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index bff69ba0b7..d9530ef015 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -680,7 +680,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } else { // convert duplicate append to get List results = region.get(ProtobufUtil.toGet(mutation, cellScanner), false, - nonceGroup, nonce); + nonceGroup, nonce); r = Result.create(results); } success = true; @@ -731,7 +731,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } else { // convert duplicate increment to get List results = region.get(ProtobufUtil.toGet(mutation, cells), false, nonceGroup, - nonce); + nonce); r = Result.create(results); } success = true; @@ -2250,7 +2250,6 @@ public class RSRpcServices implements HBaseRPCErrorHandler, checkOpen(); requestCount.increment(); HRegion region = getRegion(request.getRegion()); - boolean bypass = false; boolean loaded = false; Map> map = null; @@ -2277,15 +2276,13 @@ public class RSRpcServices implements HBaseRPCErrorHandler, familyPaths.add(new Pair<>(familyPath.getFamily().toByteArray(), familyPath.getPath())); } if (region.getCoprocessorHost() != null) { - bypass = region.getCoprocessorHost().preBulkLoadHFile(familyPaths); + region.getCoprocessorHost().preBulkLoadHFile(familyPaths); } try { - if (!bypass) { - map = region.bulkLoadHFiles(familyPaths, request.getAssignSeqNum(), null, - request.getCopyFile()); - if (map != null) { - loaded = true; - } + map = region.bulkLoadHFiles(familyPaths, request.getAssignSeqNum(), null, + request.getCopyFile()); + if (map != null) { + loaded = true; } } finally { if (region.getCoprocessorHost() != null) { @@ -2731,8 +2728,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler, ByteArrayComparable comparator = ProtobufUtil.toComparator(condition.getComparator()); if (region.getCoprocessorHost() != null) { - processed = region.getCoprocessorHost().preCheckAndPut( - row, family, qualifier, compareOp, comparator, put); + processed = region.getCoprocessorHost().preCheckAndPut(row, family, qualifier, + compareOp, comparator, put); } if (processed == null) { boolean result = region.checkAndMutate(row, family, @@ -2762,8 +2759,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler, ByteArrayComparable comparator = ProtobufUtil.toComparator(condition.getComparator()); if (region.getCoprocessorHost() != null) { - processed = region.getCoprocessorHost().preCheckAndDelete( - row, family, qualifier, op, comparator, delete); + processed = region.getCoprocessorHost().preCheckAndDelete(row, family, qualifier, op, + comparator, delete); } if (processed == null) { boolean result = region.checkAndMutate(row, family, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java index c5a3de37e5..3a93dcfd51 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java @@ -473,14 +473,23 @@ public class RegionCoprocessorHost private ObserverGetter endpointObserverGetter = RegionCoprocessor::getEndpointObserver; - abstract class RegionObserverOperation extends ObserverOperationWithoutResult { - public RegionObserverOperation() { + abstract class RegionObserverOperationWithoutResult extends + ObserverOperationWithoutResult { + public RegionObserverOperationWithoutResult() { super(regionObserverGetter); } - public RegionObserverOperation(User user) { + public RegionObserverOperationWithoutResult(User user) { super(regionObserverGetter, user); } + + public RegionObserverOperationWithoutResult(boolean bypassable) { + super(regionObserverGetter, null, bypassable); + } + + public RegionObserverOperationWithoutResult(User user, boolean bypassable) { + super(regionObserverGetter, user, bypassable); + } } abstract class BulkLoadObserverOperation extends @@ -505,7 +514,8 @@ public class RegionCoprocessorHost * @throws IOException Signals that an I/O exception has occurred. */ public void preOpen() throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preOpen(this); @@ -518,8 +528,9 @@ public class RegionCoprocessorHost * Invoked after a region open */ public void postOpen() { + if (coprocEnvironments.isEmpty()) return; try { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postOpen(this); @@ -534,8 +545,9 @@ public class RegionCoprocessorHost * Invoked after log replay on region */ public void postLogReplay() { + if (coprocEnvironments.isEmpty()) return; try { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postLogReplay(this); @@ -551,7 +563,7 @@ public class RegionCoprocessorHost * @param abortRequested true if the server is aborting */ public void preClose(final boolean abortRequested) throws IOException { - execOperation(false, new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preClose(this, abortRequested); @@ -565,7 +577,7 @@ public class RegionCoprocessorHost */ public void postClose(final boolean abortRequested) { try { - execOperation(false, new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postClose(this, abortRequested); @@ -584,16 +596,19 @@ public class RegionCoprocessorHost /** * Called prior to selecting the {@link HStoreFile}s for compaction from the list of currently * available candidates. + *

Supports Coprocessor 'bypass' -- 'bypass' is how this method indicates that it changed + * the passed in candidates. * @param store The store where compaction is being requested * @param candidates The currently available store files * @param tracker used to track the life cycle of a compaction * @param user the user - * @return If {@code true}, skip the normal selection process and use the current list * @throws IOException */ public boolean preCompactSelection(final HStore store, final List candidates, final CompactionLifeCycleTracker tracker, final User user) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation(user) { + if (coprocEnvironments.isEmpty()) return false; + boolean bypassable = true; + return execOperation(new RegionObserverOperationWithoutResult(user, bypassable) { @Override public void call(RegionObserver observer) throws IOException { observer.preCompactSelection(this, store, candidates, tracker); @@ -613,7 +628,8 @@ public class RegionCoprocessorHost public void postCompactSelection(final HStore store, final List selected, final CompactionLifeCycleTracker tracker, final CompactionRequest request, final User user) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation(user) { + if (coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult(user) { @Override public void call(RegionObserver observer) throws IOException { observer.postCompactSelection(this, store, selected, tracker, request); @@ -626,8 +642,9 @@ public class RegionCoprocessorHost */ public ScanInfo preCompactScannerOpen(HStore store, ScanType scanType, CompactionLifeCycleTracker tracker, CompactionRequest request, User user) throws IOException { + if (coprocEnvironments.isEmpty()) return store.getScanInfo(); CustomizedScanInfoBuilder builder = new CustomizedScanInfoBuilder(store.getScanInfo()); - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation(user) { + execOperation(new RegionObserverOperationWithoutResult(user) { @Override public void call(RegionObserver observer) throws IOException { observer.preCompactScannerOpen(this, store, scanType, builder, tracker, request); @@ -646,12 +663,15 @@ public class RegionCoprocessorHost * @param user the user * @throws IOException */ + // A Coprocessor can return null to cancel Compact. Leaving for now but this is a form of 'bypass'. public InternalScanner preCompact(final HStore store, final InternalScanner scanner, final ScanType scanType, final CompactionLifeCycleTracker tracker, final CompactionRequest request, final User user) throws IOException { - return execOperationWithResult(false, scanner, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult( - regionObserverGetter, user) { + InternalScanner defaultResult = scanner; + if (coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, user) { @Override public InternalScanner call(RegionObserver observer) throws IOException { return observer.preCompact(this, store, getResult(), scanType, tracker, request); @@ -671,7 +691,7 @@ public class RegionCoprocessorHost public void postCompact(final HStore store, final HStoreFile resultFile, final CompactionLifeCycleTracker tracker, final CompactionRequest request, final User user) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation(user) { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult(user) { @Override public void call(RegionObserver observer) throws IOException { observer.postCompact(this, store, resultFile, tracker, request); @@ -681,12 +701,12 @@ public class RegionCoprocessorHost /** * Invoked before create StoreScanner for flush. - * @throws IOException */ public ScanInfo preFlushScannerOpen(HStore store, FlushLifeCycleTracker tracker) throws IOException { + if (coprocEnvironments.isEmpty()) return store.getScanInfo(); CustomizedScanInfoBuilder builder = new CustomizedScanInfoBuilder(store.getScanInfo()); - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preFlushScannerOpen(this, store, builder, tracker); @@ -699,10 +719,12 @@ public class RegionCoprocessorHost * Invoked before a memstore flush * @throws IOException */ + // A Coprocessor can return null to cancel Flush. Leaving for now but this is a form of 'bypass'. public InternalScanner preFlush(HStore store, InternalScanner scanner, FlushLifeCycleTracker tracker) throws IOException { - return execOperationWithResult(false, scanner, coprocEnvironments.isEmpty() ? null - : new ObserverOperationWithResult(regionObserverGetter) { + if (coprocEnvironments.isEmpty()) return scanner; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, scanner) { @Override public InternalScanner call(RegionObserver observer) throws IOException { return observer.preFlush(this, store, getResult(), tracker); @@ -715,7 +737,7 @@ public class RegionCoprocessorHost * @throws IOException */ public void preFlush(FlushLifeCycleTracker tracker) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preFlush(this, tracker); @@ -728,7 +750,7 @@ public class RegionCoprocessorHost * @throws IOException */ public void postFlush(FlushLifeCycleTracker tracker) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postFlush(this, tracker); @@ -742,7 +764,7 @@ public class RegionCoprocessorHost */ public void postFlush(HStore store, HStoreFile storeFile, FlushLifeCycleTracker tracker) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postFlush(this, store, storeFile, tracker); @@ -752,13 +774,16 @@ public class RegionCoprocessorHost // RegionObserver support /** + * Supports Coprocessor 'bypass'. * @param get the Get request - * @return true if default processing should be bypassed + * @param results What to return if return is true/'bypass'. + * @return true if default processing should be bypassed. * @exception IOException Exception */ - public boolean preGet(final Get get, final List results) - throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + public boolean preGet(final Get get, final List results) throws IOException { + if (coprocEnvironments.isEmpty()) return false; + boolean bypassable = true; + return execOperation(new RegionObserverOperationWithoutResult(bypassable) { @Override public void call(RegionObserver observer) throws IOException { observer.preGetOp(this, get, results); @@ -768,12 +793,13 @@ public class RegionCoprocessorHost /** * @param get the Get request - * @param results the result sett + * @param results the result set * @exception IOException Exception */ public void postGet(final Get get, final List results) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postGetOp(this, get, results); @@ -782,14 +808,18 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param get the Get request - * @return true or false to return to client if bypassing normal operation, - * or null otherwise + * @return true or false to return to client if bypassing normal operation, or null otherwise * @exception IOException Exception */ public Boolean preExists(final Get get) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: false; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preExists(this, get, getResult()); @@ -799,14 +829,15 @@ public class RegionCoprocessorHost /** * @param get the Get request - * @param exists the result returned by the region server + * @param result the result returned by the region server * @return the result to return to the client * @exception IOException Exception */ - public boolean postExists(final Get get, boolean exists) + public boolean postExists(final Get get, boolean result) throws IOException { - return execOperationWithResult(exists, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postExists(this, get, getResult()); @@ -815,6 +846,7 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param put The Put object * @param edit The WALEdit object. * @param durability The durability used @@ -823,7 +855,9 @@ public class RegionCoprocessorHost */ public boolean prePut(final Put put, final WALEdit edit, final Durability durability) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (coprocEnvironments.isEmpty()) return false; + boolean bypassable = true; + return execOperation(new RegionObserverOperationWithoutResult(bypassable) { @Override public void call(RegionObserver observer) throws IOException { observer.prePut(this, put, edit, durability); @@ -832,21 +866,25 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param mutation - the current mutation * @param kv - the current cell * @param byteNow - current timestamp in bytes * @param get - the get that could be used * Note that the get only does not specify the family and qualifier that should be used * @return true if default processing should be bypassed - * @exception IOException - * Exception + * @deprecated In hbase-2.0.0. Will be removed in hbase-3.0.0. Added explicitly for a single + * Coprocessor for its needs only. Will be removed. */ + @Deprecated public boolean prePrepareTimeStampForDeleteVersion(final Mutation mutation, final Cell kv, final byte[] byteNow, final Get get) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (coprocEnvironments.isEmpty()) return false; + boolean bypassable = true; + return execOperation(new RegionObserverOperationWithoutResult(bypassable) { @Override public void call(RegionObserver observer) throws IOException { - observer.prePrepareTimeStampForDeleteVersion(this, mutation, kv, byteNow, get); + observer.prePrepareTimeStampForDeleteVersion(this, mutation, kv, byteNow, get); } }); } @@ -859,7 +897,8 @@ public class RegionCoprocessorHost */ public void postPut(final Put put, final WALEdit edit, final Durability durability) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postPut(this, put, edit, durability); @@ -868,6 +907,7 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param delete The Delete object * @param edit The WALEdit object. * @param durability The durability used @@ -876,10 +916,12 @@ public class RegionCoprocessorHost */ public boolean preDelete(final Delete delete, final WALEdit edit, final Durability durability) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (this.coprocEnvironments.isEmpty()) return false; + boolean bypassable = true; + return execOperation(new RegionObserverOperationWithoutResult(bypassable) { @Override public void call(RegionObserver observer) throws IOException { - observer.preDelete(this, delete, edit, durability); + observer.preDelete(this, delete, edit, durability); } }); } @@ -892,7 +934,8 @@ public class RegionCoprocessorHost */ public void postDelete(final Delete delete, final WALEdit edit, final Durability durability) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postDelete(this, delete, edit, durability); @@ -900,14 +943,10 @@ public class RegionCoprocessorHost }); } - /** - * @param miniBatchOp - * @return true if default processing should be bypassed - * @throws IOException - */ - public boolean preBatchMutate( + public void preBatchMutate( final MiniBatchOperationInProgress miniBatchOp) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (this.coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preBatchMutate(this, miniBatchOp); @@ -915,13 +954,10 @@ public class RegionCoprocessorHost }); } - /** - * @param miniBatchOp - * @throws IOException - */ public void postBatchMutate( final MiniBatchOperationInProgress miniBatchOp) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (this.coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postBatchMutate(this, miniBatchOp); @@ -932,7 +968,8 @@ public class RegionCoprocessorHost public void postBatchMutateIndispensably( final MiniBatchOperationInProgress miniBatchOp, final boolean success) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + if (this.coprocEnvironments.isEmpty()) return; + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postBatchMutateIndispensably(this, miniBatchOp, success); @@ -941,22 +978,26 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param row row to check * @param family column family * @param qualifier column qualifier * @param op the comparison operation * @param comparator the comparator * @param put data to put if check succeeds - * @return true or false to return to client if default processing should - * be bypassed, or null otherwise - * @throws IOException e + * @return true or false to return to client if default processing should be bypassed, or null + * otherwise */ public Boolean preCheckAndPut(final byte [] row, final byte [] family, final byte [] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Put put) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preCheckAndPut(this, row, family, qualifier, @@ -966,21 +1007,25 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param row row to check * @param family column family * @param qualifier column qualifier * @param op the comparison operation * @param comparator the comparator * @param put data to put if check succeeds - * @return true or false to return to client if default processing should - * be bypassed, or null otherwise - * @throws IOException e + * @return true or false to return to client if default processing should be bypassed, or null + * otherwise */ public Boolean preCheckAndPutAfterRowLock( final byte[] row, final byte[] family, final byte[] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Put put) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preCheckAndPutAfterRowLock(this, row, family, qualifier, @@ -1002,8 +1047,9 @@ public class RegionCoprocessorHost final byte [] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Put put, boolean result) throws IOException { - return execOperationWithResult(result, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postCheckAndPut(this, row, family, qualifier, @@ -1013,22 +1059,26 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param row row to check * @param family column family * @param qualifier column qualifier * @param op the comparison operation * @param comparator the comparator * @param delete delete to commit if check succeeds - * @return true or false to return to client if default processing should - * be bypassed, or null otherwise - * @throws IOException e + * @return true or false to return to client if default processing should be bypassed, + * or null otherwise */ public Boolean preCheckAndDelete(final byte [] row, final byte [] family, final byte [] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Delete delete) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preCheckAndDelete(this, row, family, @@ -1038,21 +1088,25 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param row row to check * @param family column family * @param qualifier column qualifier * @param op the comparison operation * @param comparator the comparator * @param delete delete to commit if check succeeds - * @return true or false to return to client if default processing should - * be bypassed, or null otherwise - * @throws IOException e + * @return true or false to return to client if default processing should be bypassed, + * or null otherwise */ public Boolean preCheckAndDeleteAfterRowLock(final byte[] row, final byte[] family, final byte[] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Delete delete) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preCheckAndDeleteAfterRowLock(this, row, @@ -1074,8 +1128,9 @@ public class RegionCoprocessorHost final byte [] qualifier, final CompareOperator op, final ByteArrayComparable comparator, final Delete delete, boolean result) throws IOException { - return execOperationWithResult(result, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postCheckAndDelete(this, row, family, @@ -1085,14 +1140,18 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param append append object - * @return result to return to client if default operation should be - * bypassed, null otherwise + * @return result to return to client if default operation should be bypassed, null otherwise * @throws IOException if an error occurred on the coprocessor */ public Result preAppend(final Append append) throws IOException { - return execOperationWithResult(true, null, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + Result defaultResult = null; + if (this.coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, defaultResult, + bypassable) { @Override public Result call(RegionObserver observer) throws IOException { return observer.preAppend(this, append); @@ -1101,14 +1160,18 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param append append object - * @return result to return to client if default operation should be - * bypassed, null otherwise + * @return result to return to client if default operation should be bypassed, null otherwise * @throws IOException if an error occurred on the coprocessor */ public Result preAppendAfterRowLock(final Append append) throws IOException { - return execOperationWithResult(true, null, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + Result defaultResult = null; + if (this.coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Result call(RegionObserver observer) throws IOException { return observer.preAppendAfterRowLock(this, append); @@ -1117,14 +1180,18 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param increment increment object - * @return result to return to client if default operation should be - * bypassed, null otherwise + * @return result to return to client if default operation should be bypassed, null otherwise * @throws IOException if an error occurred on the coprocessor */ public Result preIncrement(final Increment increment) throws IOException { - return execOperationWithResult(true, null, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + Result defaultResult = null; + if (coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, defaultResult, + bypassable) { @Override public Result call(RegionObserver observer) throws IOException { return observer.preIncrement(this, increment); @@ -1133,14 +1200,18 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param increment increment object - * @return result to return to client if default operation should be - * bypassed, null otherwise + * @return result to return to client if default operation should be bypassed, null otherwise * @throws IOException if an error occurred on the coprocessor */ public Result preIncrementAfterRowLock(final Increment increment) throws IOException { - return execOperationWithResult(true, null, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + Result defaultResult = null; + if (coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, defaultResult, + bypassable) { @Override public Result call(RegionObserver observer) throws IOException { return observer.preIncrementAfterRowLock(this, increment); @@ -1154,8 +1225,9 @@ public class RegionCoprocessorHost * @throws IOException if an error occurred on the coprocessor */ public Result postAppend(final Append append, final Result result) throws IOException { - return execOperationWithResult(result, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Result call(RegionObserver observer) throws IOException { return observer.postAppend(this, append, result); @@ -1169,8 +1241,9 @@ public class RegionCoprocessorHost * @throws IOException if an error occurred on the coprocessor */ public Result postIncrement(final Increment increment, Result result) throws IOException { - return execOperationWithResult(result, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Result call(RegionObserver observer) throws IOException { return observer.postIncrement(this, increment, getResult()); @@ -1183,7 +1256,7 @@ public class RegionCoprocessorHost * @exception IOException Exception */ public void preScannerOpen(final Scan scan) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preScannerOpen(this, scan); @@ -1198,8 +1271,9 @@ public class RegionCoprocessorHost * @exception IOException Exception */ public RegionScanner postScannerOpen(final Scan scan, RegionScanner s) throws IOException { - return execOperationWithResult(s, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return s; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, s) { @Override public RegionScanner call(RegionObserver observer) throws IOException { return observer.postScannerOpen(this, scan, getResult()); @@ -1211,14 +1285,17 @@ public class RegionCoprocessorHost * @param s the scanner * @param results the result set returned by the region server * @param limit the maximum number of results to return - * @return 'has next' indication to client if bypassing default behavior, or - * null otherwise + * @return 'has next' indication to client if bypassing default behavior, or null otherwise * @exception IOException Exception */ public Boolean preScannerNext(final InternalScanner s, final List results, final int limit) throws IOException { - return execOperationWithResult(true, false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean bypassable = true; + boolean defaultResult = false; + if (coprocEnvironments.isEmpty()) return bypassable? null: defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, + defaultResult, bypassable) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.preScannerNext(this, s, results, limit, getResult()); @@ -1237,8 +1314,9 @@ public class RegionCoprocessorHost public boolean postScannerNext(final InternalScanner s, final List results, final int limit, boolean hasMore) throws IOException { - return execOperationWithResult(hasMore, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return hasMore; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, hasMore) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postScannerNext(this, s, results, limit, getResult()); @@ -1257,9 +1335,11 @@ public class RegionCoprocessorHost public boolean postScannerFilterRow(final InternalScanner s, final Cell curRowCell) throws IOException { // short circuit for performance - if (!hasCustomPostScannerFilterRow) return true; - return execOperationWithResult(true, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + boolean defaultResult = true; + if (!hasCustomPostScannerFilterRow) return defaultResult; + if (this.coprocEnvironments.isEmpty()) return defaultResult; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, defaultResult) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postScannerFilterRow(this, s, curRowCell, getResult()); @@ -1268,12 +1348,15 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param s the scanner * @return true if default behavior should be bypassed, false otherwise * @exception IOException Exception */ + // Should this be bypassable? public boolean preScannerClose(final InternalScanner s) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + return execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult(true) { @Override public void call(RegionObserver observer) throws IOException { observer.preScannerClose(this, s); @@ -1285,7 +1368,8 @@ public class RegionCoprocessorHost * @exception IOException Exception */ public void postScannerClose(final InternalScanner s) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postScannerClose(this, s); @@ -1297,8 +1381,9 @@ public class RegionCoprocessorHost * Called before open store scanner for user scan. */ public ScanInfo preStoreScannerOpen(HStore store) throws IOException { + if (coprocEnvironments.isEmpty()) return store.getScanInfo(); CustomizedScanInfoBuilder builder = new CustomizedScanInfoBuilder(store.getScanInfo()); - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preStoreScannerOpen(this, store, builder); @@ -1308,12 +1393,14 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param info the RegionInfo for this region * @param edits the file of recovered edits - * @throws IOException Exception + * @return true if default behavior should be bypassed, false otherwise */ - public void preReplayWALs(final RegionInfo info, final Path edits) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + public boolean preReplayWALs(final RegionInfo info, final Path edits) throws IOException { + return execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult(true) { @Override public void call(RegionObserver observer) throws IOException { observer.preReplayWALs(this, info, edits); @@ -1327,7 +1414,8 @@ public class RegionCoprocessorHost * @throws IOException Exception */ public void postReplayWALs(final RegionInfo info, final Path edits) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postReplayWALs(this, info, edits); @@ -1336,15 +1424,16 @@ public class RegionCoprocessorHost } /** - * @param info - * @param logKey - * @param logEdit + * Supports Coprocessor 'bypass'. * @return true if default behavior should be bypassed, false otherwise - * @throws IOException + * @deprecated Since hbase-2.0.0. No replacement. To be removed in hbase-3.0.0 and replaced + * with something that doesn't expose IntefaceAudience.Private classes. */ - public boolean preWALRestore(final RegionInfo info, final WALKey logKey, - final WALEdit logEdit) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + @Deprecated + public boolean preWALRestore(final RegionInfo info, final WALKey logKey, final WALEdit logEdit) + throws IOException { + return execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult(true) { @Override public void call(RegionObserver observer) throws IOException { observer.preWALRestore(this, info, logKey, logEdit); @@ -1353,14 +1442,14 @@ public class RegionCoprocessorHost } /** - * @param info - * @param logKey - * @param logEdit - * @throws IOException + * @deprecated Since hbase-2.0.0. No replacement. To be removed in hbase-3.0.0 and replaced + * with something that doesn't expose IntefaceAudience.Private classes. */ + @Deprecated public void postWALRestore(final RegionInfo info, final WALKey logKey, final WALEdit logEdit) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postWALRestore(this, info, logKey, logEdit); @@ -1370,11 +1459,9 @@ public class RegionCoprocessorHost /** * @param familyPaths pairs of { CF, file path } submitted for bulk load - * @return true if the default operation should be bypassed - * @throws IOException */ - public boolean preBulkLoadHFile(final List> familyPaths) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + public void preBulkLoadHFile(final List> familyPaths) throws IOException { + execOperation(coprocEnvironments.isEmpty()? null: new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preBulkLoadHFile(this, familyPaths); @@ -1384,7 +1471,8 @@ public class RegionCoprocessorHost public boolean preCommitStoreFile(final byte[] family, final List> pairs) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + return execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.preCommitStoreFile(this, family, pairs); @@ -1392,7 +1480,8 @@ public class RegionCoprocessorHost }); } public void postCommitStoreFile(final byte[] family, Path srcPath, Path dstPath) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postCommitStoreFile(this, family, srcPath, dstPath); @@ -1403,14 +1492,15 @@ public class RegionCoprocessorHost /** * @param familyPaths pairs of { CF, file path } submitted for bulk load * @param map Map of CF to List of file paths for the final loaded files - * @param hasLoaded whether load was successful or not + * @param result whether load was successful or not * @return the possibly modified value of hasLoaded * @throws IOException */ public boolean postBulkLoadHFile(final List> familyPaths, - Map> map, boolean hasLoaded) throws IOException { - return execOperationWithResult(hasLoaded, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + Map> map, boolean result) throws IOException { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public Boolean call(RegionObserver observer) throws IOException { return observer.postBulkLoadHFile(this, familyPaths, map, getResult()); @@ -1419,7 +1509,8 @@ public class RegionCoprocessorHost } public void postStartRegionOperation(final Operation op) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postStartRegionOperation(this, op); @@ -1428,7 +1519,8 @@ public class RegionCoprocessorHost } public void postCloseRegionOperation(final Operation op) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperationWithoutResult() { @Override public void call(RegionObserver observer) throws IOException { observer.postCloseRegionOperation(this, op); @@ -1450,8 +1542,9 @@ public class RegionCoprocessorHost public StoreFileReader preStoreFileReaderOpen(final FileSystem fs, final Path p, final FSDataInputStreamWrapper in, final long size, final CacheConfig cacheConf, final Reference r) throws IOException { - return execOperationWithResult(null, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (coprocEnvironments.isEmpty()) return null; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, null) { @Override public StoreFileReader call(RegionObserver observer) throws IOException { return observer.preStoreFileReaderOpen(this, fs, p, in, size, cacheConf, r, @@ -1474,8 +1567,9 @@ public class RegionCoprocessorHost public StoreFileReader postStoreFileReaderOpen(final FileSystem fs, final Path p, final FSDataInputStreamWrapper in, final long size, final CacheConfig cacheConf, final Reference r, final StoreFileReader reader) throws IOException { - return execOperationWithResult(reader, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return reader; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, reader) { @Override public StoreFileReader call(RegionObserver observer) throws IOException { return observer.postStoreFileReaderOpen(this, fs, p, in, size, cacheConf, r, @@ -1486,8 +1580,9 @@ public class RegionCoprocessorHost public Cell postMutationBeforeWAL(final MutationType opType, final Mutation mutation, final Cell oldCell, Cell newCell) throws IOException { - return execOperationWithResult(newCell, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + if (this.coprocEnvironments.isEmpty()) return newCell; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, newCell) { @Override public Cell call(RegionObserver observer) throws IOException { return observer.postMutationBeforeWAL(this, opType, mutation, oldCell, getResult()); @@ -1497,8 +1592,9 @@ public class RegionCoprocessorHost public Message preEndpointInvocation(final Service service, final String methodName, Message request) throws IOException { - return execOperationWithResult(request, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(endpointObserverGetter) { + if (coprocEnvironments.isEmpty()) return request; + return execOperationWithResult( + new ObserverOperationWithResult(endpointObserverGetter, request) { @Override public Message call(EndpointObserver observer) throws IOException { return observer.preEndpointInvocation(this, service, methodName, getResult()); @@ -1517,9 +1613,14 @@ public class RegionCoprocessorHost }); } - public DeleteTracker postInstantiateDeleteTracker(DeleteTracker tracker) throws IOException { - return execOperationWithResult(tracker, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(regionObserverGetter) { + /** + * @deprecated Since 2.0 with out any replacement and will be removed in 3.0 + */ + @Deprecated + public DeleteTracker postInstantiateDeleteTracker(DeleteTracker result) throws IOException { + if (this.coprocEnvironments.isEmpty()) return result; + return execOperationWithResult( + new ObserverOperationWithResult(regionObserverGetter, result) { @Override public DeleteTracker call(RegionObserver observer) throws IOException { return observer.postInstantiateDeleteTracker(this, getResult()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java index 27a3e201ea..39047b3e55 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java @@ -115,7 +115,8 @@ public class RegionServerCoprocessorHost extends public void preStop(String message, User user) throws IOException { // While stopping the region server all coprocessors method should be executed first then the // coprocessor should be cleaned up. - execShutdown(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation(user) { + if (coprocEnvironments.isEmpty()) return; + execShutdown(new RegionServerObserverOperation(user) { @Override public void call(RegionServerObserver observer) throws IOException { observer.preStopRegionServer(this); @@ -169,9 +170,10 @@ public class RegionServerCoprocessorHost extends public ReplicationEndpoint postCreateReplicationEndPoint(final ReplicationEndpoint endpoint) throws IOException { - return execOperationWithResult(endpoint, coprocEnvironments.isEmpty() ? null : + if (this.coprocEnvironments.isEmpty()) return endpoint; + return execOperationWithResult( new ObserverOperationWithResult( - rsObserverGetter) { + rsObserverGetter, endpoint) { @Override public ReplicationEndpoint call(RegionServerObserver observer) throws IOException { return observer.postCreateReplicationEndPoint(this, getResult()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java index 653ec75394..6ce44fe859 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java @@ -192,57 +192,54 @@ public class SecureBulkLoadManager { throw new DoNotRetryIOException("User token cannot be null"); } - boolean bypass = false; if (region.getCoprocessorHost() != null) { - bypass = region.getCoprocessorHost().preBulkLoadHFile(familyPaths); + region.getCoprocessorHost().preBulkLoadHFile(familyPaths); } boolean loaded = false; Map> map = null; try { - if (!bypass) { - // Get the target fs (HBase region server fs) delegation token - // Since we have checked the permission via 'preBulkLoadHFile', now let's give - // the 'request user' necessary token to operate on the target fs. - // After this point the 'doAs' user will hold two tokens, one for the source fs - // ('request user'), another for the target fs (HBase region server principal). - if (userProvider.isHadoopSecurityEnabled()) { - FsDelegationToken targetfsDelegationToken = new FsDelegationToken(userProvider,"renewer"); - targetfsDelegationToken.acquireDelegationToken(fs); - - Token targetFsToken = targetfsDelegationToken.getUserToken(); - if (targetFsToken != null - && (userToken == null || !targetFsToken.getService().equals(userToken.getService()))){ - ugi.addToken(targetFsToken); - } + // Get the target fs (HBase region server fs) delegation token + // Since we have checked the permission via 'preBulkLoadHFile', now let's give + // the 'request user' necessary token to operate on the target fs. + // After this point the 'doAs' user will hold two tokens, one for the source fs + // ('request user'), another for the target fs (HBase region server principal). + if (userProvider.isHadoopSecurityEnabled()) { + FsDelegationToken targetfsDelegationToken = new FsDelegationToken(userProvider,"renewer"); + targetfsDelegationToken.acquireDelegationToken(fs); + + Token targetFsToken = targetfsDelegationToken.getUserToken(); + if (targetFsToken != null + && (userToken == null || !targetFsToken.getService().equals(userToken.getService()))){ + ugi.addToken(targetFsToken); } + } - map = ugi.doAs(new PrivilegedAction>>() { - @Override - public Map> run() { - FileSystem fs = null; - try { - fs = FileSystem.get(conf); - for(Pair el: familyPaths) { - Path stageFamily = new Path(bulkToken, Bytes.toString(el.getFirst())); - if(!fs.exists(stageFamily)) { - fs.mkdirs(stageFamily); - fs.setPermission(stageFamily, PERM_ALL_ACCESS); - } + map = ugi.doAs(new PrivilegedAction>>() { + @Override + public Map> run() { + FileSystem fs = null; + try { + fs = FileSystem.get(conf); + for(Pair el: familyPaths) { + Path stageFamily = new Path(bulkToken, Bytes.toString(el.getFirst())); + if(!fs.exists(stageFamily)) { + fs.mkdirs(stageFamily); + fs.setPermission(stageFamily, PERM_ALL_ACCESS); } - //We call bulkLoadHFiles as requesting user - //To enable access prior to staging - return region.bulkLoadHFiles(familyPaths, true, - new SecureBulkLoadListener(fs, bulkToken, conf), request.getCopyFile()); - } catch (Exception e) { - LOG.error("Failed to complete bulk load", e); } - return null; + //We call bulkLoadHFiles as requesting user + //To enable access prior to staging + return region.bulkLoadHFiles(familyPaths, true, + new SecureBulkLoadListener(fs, bulkToken, conf), request.getCopyFile()); + } catch (Exception e) { + LOG.error("Failed to complete bulk load", e); } - }); - if (map != null) { - loaded = true; + return null; } + }); + if (map != null) { + loaded = true; } } finally { if (region.getCoprocessorHost() != null) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java index 34f93fa9e2..72f3f4221f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java @@ -141,16 +141,15 @@ public class WALCoprocessorHost /** - * @param info - * @param logKey - * @param logEdit * @return true if default behavior should be bypassed, false otherwise * @throws IOException */ public boolean preWALWrite(final RegionInfo info, final WALKey logKey, final WALEdit logEdit) throws IOException { - return execOperationWithResult(false, coprocEnvironments.isEmpty() ? null : - new ObserverOperationWithResult(walObserverGetter) { + // Not bypassable. + if (this.coprocEnvironments.isEmpty()) return false; + return execOperationWithResult( + new ObserverOperationWithResult(walObserverGetter, false) { @Override public Boolean call(WALObserver oserver) throws IOException { return oserver.preWALWrite(this, info, logKey, logEdit); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index e9a56bd956..a525a532b8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -90,7 +90,6 @@ public class TestMasterObserver { public static class CPMasterObserver implements MasterCoprocessor, MasterObserver { - private boolean bypass = false; private boolean preCreateTableCalled; private boolean postCreateTableCalled; private boolean preDeleteTableCalled; @@ -182,10 +181,6 @@ public class TestMasterObserver { private boolean preLockHeartbeatCalled; private boolean postLockHeartbeatCalled; - public void enableBypass(boolean bypass) { - this.bypass = bypass; - } - public void resetStates() { preCreateTableCalled = false; postCreateTableCalled = false; @@ -301,9 +296,6 @@ public class TestMasterObserver { @Override public void preCreateTable(ObserverContext env, TableDescriptor desc, RegionInfo[] regions) throws IOException { - if (bypass) { - env.bypass(); - } preCreateTableCalled = true; } @@ -324,9 +316,6 @@ public class TestMasterObserver { @Override public void preDeleteTable(ObserverContext env, TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preDeleteTableCalled = true; } @@ -347,9 +336,6 @@ public class TestMasterObserver { @Override public void preTruncateTable(ObserverContext env, TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preTruncateTableCalled = true; } @@ -381,9 +367,6 @@ public class TestMasterObserver { @Override public void preModifyTable(ObserverContext env, TableName tableName, TableDescriptor htd) throws IOException { - if (bypass) { - env.bypass(); - } preModifyTableCalled = true; } @@ -404,9 +387,6 @@ public class TestMasterObserver { @Override public void preCreateNamespace(ObserverContext env, NamespaceDescriptor ns) throws IOException { - if (bypass) { - env.bypass(); - } preCreateNamespaceCalled = true; } @@ -427,9 +407,6 @@ public class TestMasterObserver { @Override public void preDeleteNamespace(ObserverContext env, String name) throws IOException { - if (bypass) { - env.bypass(); - } preDeleteNamespaceCalled = true; } @@ -450,9 +427,6 @@ public class TestMasterObserver { @Override public void preModifyNamespace(ObserverContext env, NamespaceDescriptor ns) throws IOException { - if (bypass) { - env.bypass(); - } preModifyNamespaceCalled = true; } @@ -490,9 +464,6 @@ public class TestMasterObserver { @Override public void preListNamespaceDescriptors(ObserverContext env, List descriptors) throws IOException { - if (bypass) { - env.bypass(); - } preListNamespaceDescriptorsCalled = true; } @@ -513,9 +484,6 @@ public class TestMasterObserver { @Override public void preEnableTable(ObserverContext env, TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preEnableTableCalled = true; } @@ -536,9 +504,6 @@ public class TestMasterObserver { @Override public void preDisableTable(ObserverContext env, TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preDisableTableCalled = true; } @@ -619,9 +584,6 @@ public class TestMasterObserver { public void preMove(ObserverContext env, RegionInfo region, ServerName srcServer, ServerName destServer) throws IOException { - if (bypass) { - env.bypass(); - } preMoveCalled = true; } @@ -643,9 +605,6 @@ public class TestMasterObserver { @Override public void preAssign(ObserverContext env, final RegionInfo regionInfo) throws IOException { - if (bypass) { - env.bypass(); - } preAssignCalled = true; } @@ -666,9 +625,6 @@ public class TestMasterObserver { @Override public void preUnassign(ObserverContext env, final RegionInfo regionInfo, final boolean force) throws IOException { - if (bypass) { - env.bypass(); - } preUnassignCalled = true; } @@ -709,9 +665,6 @@ public class TestMasterObserver { @Override public void preBalance(ObserverContext env) throws IOException { - if (bypass) { - env.bypass(); - } preBalanceCalled = true; } @@ -732,9 +685,6 @@ public class TestMasterObserver { @Override public boolean preBalanceSwitch(ObserverContext env, boolean b) throws IOException { - if (bypass) { - env.bypass(); - } preBalanceSwitchCalled = true; return b; } @@ -898,9 +848,6 @@ public class TestMasterObserver { final ObserverContext env, final TableDescriptor desc, final RegionInfo[] regions) throws IOException { - if (bypass) { - env.bypass(); - } preCreateTableActionCalled = true; } @@ -928,9 +875,6 @@ public class TestMasterObserver { public void preDeleteTableAction( final ObserverContext env, final TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preDeleteTableActionCalled = true; } @@ -954,9 +898,6 @@ public class TestMasterObserver { public void preTruncateTableAction( final ObserverContext env, final TableName tableName) throws IOException { - if (bypass) { - env.bypass(); - } preTruncateTableActionCalled = true; } @@ -980,9 +921,6 @@ public class TestMasterObserver { final ObserverContext env, final TableName tableName, final TableDescriptor htd) throws IOException { - if (bypass) { - env.bypass(); - } preModifyTableActionCalled = true; } @@ -1005,9 +943,6 @@ public class TestMasterObserver { public void preEnableTableAction( final ObserverContext ctx, final TableName tableName) throws IOException { - if (bypass) { - ctx.bypass(); - } preEnableTableActionCalled = true; } @@ -1030,9 +965,6 @@ public class TestMasterObserver { public void preDisableTableAction( final ObserverContext ctx, final TableName tableName) throws IOException { - if (bypass) { - ctx.bypass(); - } preDisableTableActionCalled = true; } @@ -1357,7 +1289,6 @@ public class TestMasterObserver { HMaster master = cluster.getMaster(); MasterCoprocessorHost host = master.getMasterCoprocessorHost(); CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.enableBypass(true); cp.resetStates(); assertFalse("No table created yet", cp.wasCreateTableCalled()); @@ -1370,7 +1301,6 @@ public class TestMasterObserver { admin.createTable(htd, Arrays.copyOfRange(HBaseTestingUtility.KEYS, 1, HBaseTestingUtility.KEYS.length)); - // preCreateTable can't bypass default action. assertTrue("Test table should be created", cp.wasCreateTableCalled()); tableCreationLatch.await(); assertTrue("Table pre create handler called.", cp @@ -1389,7 +1319,6 @@ public class TestMasterObserver { tableCreationLatch = new CountDownLatch(1); admin.disableTable(tableName); assertTrue(admin.isTableDisabled(tableName)); - // preDisableTable can't bypass default action. assertTrue("Coprocessor should have been called on table disable", cp.wasDisableTableCalled()); assertTrue("Disable table handler should be called.", @@ -1399,7 +1328,6 @@ public class TestMasterObserver { assertFalse(cp.wasEnableTableCalled()); admin.enableTable(tableName); assertTrue(admin.isTableEnabled(tableName)); - // preEnableTable can't bypass default action. assertTrue("Coprocessor should have been called on table enable", cp.wasEnableTableCalled()); assertTrue("Enable table handler should be called.", @@ -1411,7 +1339,6 @@ public class TestMasterObserver { // modify table htd.setMaxFileSize(512 * 1024 * 1024); modifyTableSync(admin, tableName, htd); - // preModifyTable can't bypass default action. assertTrue("Test table should have been modified", cp.wasModifyTableCalled()); @@ -1424,14 +1351,12 @@ public class TestMasterObserver { deleteTable(admin, tableName); assertFalse("Test table should have been deleted", admin.tableExists(tableName)); - // preDeleteTable can't bypass default action. assertTrue("Coprocessor should have been called on table delete", cp.wasDeleteTableCalled()); assertTrue("Delete table handler should be called.", cp.wasDeleteTableActionCalled()); - // turn off bypass, run the tests again - cp.enableBypass(false); + // When bypass was supported, we'd turn off bypass and rerun tests. Leaving rerun in place. cp.resetStates(); admin.createTable(htd); @@ -1555,10 +1480,6 @@ public class TestMasterObserver { MasterCoprocessorHost host = master.getMasterCoprocessorHost(); CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.enableBypass(false); - cp.resetStates(); - - // create a table Admin admin = UTIL.getAdmin(); admin.createNamespace(NamespaceDescriptor.create(testNamespace).build()); @@ -1567,75 +1488,8 @@ public class TestMasterObserver { assertNotNull(admin.getNamespaceDescriptor(testNamespace)); assertTrue("Test namespace descriptor should have been called", cp.wasGetNamespaceDescriptorCalled()); - - // turn off bypass, run the tests again - cp.enableBypass(true); - cp.resetStates(); - - boolean expected = false; - try { - admin.modifyNamespace(NamespaceDescriptor.create(testNamespace).build()); - } catch (BypassCoprocessorException ce) { - expected = true; - } - assertTrue(expected); - assertTrue("Test namespace should not have been modified", - cp.preModifyNamespaceCalledOnly()); - - assertNotNull(admin.getNamespaceDescriptor(testNamespace)); - assertTrue("Test namespace descriptor should have been called", - cp.wasGetNamespaceDescriptorCalled()); - - expected = false; - try { - admin.deleteNamespace(testNamespace); - } catch (BypassCoprocessorException ce) { - expected = true; - } - assertTrue(expected); - assertTrue("Test namespace should not have been deleted", cp.preDeleteNamespaceCalledOnly()); - - assertNotNull(admin.getNamespaceDescriptor(testNamespace)); - assertTrue("Test namespace descriptor should have been called", - cp.wasGetNamespaceDescriptorCalled()); - - cp.enableBypass(false); - cp.resetStates(); - - // delete table - admin.modifyNamespace(NamespaceDescriptor.create(testNamespace).build()); - assertTrue("Test namespace should have been modified", cp.wasModifyNamespaceCalled()); - - admin.deleteNamespace(testNamespace); - assertTrue("Test namespace should have been deleted", cp.wasDeleteNamespaceCalled()); - - cp.enableBypass(true); - cp.resetStates(); - - expected = false; - try { - admin.createNamespace(NamespaceDescriptor.create(testNamespace).build()); - } catch (BypassCoprocessorException ce) { - expected = true; - } - assertTrue(expected); - assertTrue("Test namespace should not be created", cp.preCreateNamespaceCalledOnly()); - - // turn on bypass, run the test - cp.enableBypass(true); - cp.resetStates(); - - admin.listNamespaceDescriptors(); - assertTrue("post listNamespace should not have been called", - cp.preListNamespaceDescriptorsCalledOnly()); - - // turn off bypass, run the tests again - cp.enableBypass(false); - cp.resetStates(); - - admin.listNamespaceDescriptors(); - assertTrue("post listNamespace should have been called", - cp.wasListNamespaceDescriptorsCalled()); + // This test used to do a bunch w/ bypass but bypass of these table and namespace stuff has + // been removed so the testing code was removed. } private void modifyTableSync(Admin admin, TableName tableName, HTableDescriptor htd) @@ -1659,7 +1513,6 @@ public class TestMasterObserver { HMaster master = cluster.getMaster(); MasterCoprocessorHost host = master.getMasterCoprocessorHost(); CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.enableBypass(false); cp.resetStates(); Table table = UTIL.createMultiRegionTable(tableName, TEST_FAMILY); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/compactions/TestMobCompactor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/compactions/TestMobCompactor.java index e0d9fa24bb..54071d081d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/compactions/TestMobCompactor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/compactions/TestMobCompactor.java @@ -714,6 +714,9 @@ public class TestMobCompactor { while (fileList.length != num) { Thread.sleep(50); fileList = fs.listStatus(path); + for (FileStatus fileStatus: fileList) { + LOG.info(fileStatus); + } } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 268b352239..a5650c5c09 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -169,6 +169,7 @@ import org.junit.rules.TestName; import org.junit.rules.TestRule; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; +import org.mockito.Matchers; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -359,7 +360,7 @@ public class TestHRegion { /** * Create a WAL outside of the usual helper in - * {@link HBaseTestingUtility#createWal(Configuration, Path, HRegionInfo)} because that method + * {@link HBaseTestingUtility#createWal(Configuration, Path, RegionInfo)} because that method * doesn't play nicely with FaultyFileSystem. Call this method before overriding * {@code fs.file.impl}. * @param callingMethod a unique component for the path, probably the name of the test method. @@ -2386,6 +2387,9 @@ public class TestHRegion { FileSystem fs = FileSystem.get(CONF); Path rootDir = new Path(dir + "testDataInMemoryWithoutWAL"); FSHLog hLog = new FSHLog(fs, rootDir, "testDataInMemoryWithoutWAL", CONF); + // This chunk creation is done throughout the code base. Do we want to move it into core? + // It is missing from this test. W/o it we NPE. + ChunkCreator.initialize(MemStoreLABImpl.CHUNK_SIZE_DEFAULT, false, 0, 0, 0, null); HRegion region = initHRegion(tableName, null, null, false, Durability.SYNC_WAL, hLog, COLUMN_FAMILY_BYTES); @@ -2433,17 +2437,17 @@ public class TestHRegion { // save normalCPHost and replaced by mockedCPHost RegionCoprocessorHost normalCPHost = region.getCoprocessorHost(); RegionCoprocessorHost mockedCPHost = Mockito.mock(RegionCoprocessorHost.class); - Answer answer = new Answer() { + // Because the preBatchMutate returns void, we can't do usual Mockito when...then form. Must + // do below format (from Mockito doc). + Mockito.doAnswer(new Answer() { @Override - public Boolean answer(InvocationOnMock invocation) throws Throwable { + public Object answer(InvocationOnMock invocation) throws Throwable { MiniBatchOperationInProgress mb = invocation.getArgumentAt(0, MiniBatchOperationInProgress.class); mb.addOperationsFromCP(0, new Mutation[]{addPut}); - return false; + return null; } - }; - when(mockedCPHost.preBatchMutate(Mockito.isA(MiniBatchOperationInProgress.class))) - .then(answer); + }).when(mockedCPHost).preBatchMutate(Mockito.isA(MiniBatchOperationInProgress.class)); region.setCoprocessorHost(mockedCPHost); region.put(originalPut); region.setCoprocessorHost(normalCPHost); -- 2.11.0 (Apple Git-81)