From 7212ca9ad1a4df4efb33dc831f555b24a370297b 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' argument to the execOperation methods. 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 | 84 ++++++++++++------- .../hadoop/hbase/coprocessor/ObserverContext.java | 15 ++-- .../apache/hadoop/hbase/regionserver/HRegion.java | 70 ++++++++-------- .../hadoop/hbase/regionserver/RSRpcServices.java | 92 ++++++++++---------- .../hbase/regionserver/RegionCoprocessorHost.java | 98 +++++++++++----------- .../hadoop/hbase/regionserver/TestHRegion.java | 4 +- 6 files changed, 193 insertions(+), 170 deletions(-) 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..3dd525da5a 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; @@ -621,38 +618,58 @@ 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(); + return execOperationWithResult(false, defaultValue, observerOperation); } - // what does bypass mean? - protected R execOperationWithResult(final boolean ifBypass, final R defaultValue, + /** + * @param bypassable True if this method is bypassable (bypassable methods are the exception). + */ + protected R execOperationWithResult(final boolean bypassable, final R defaultValue, 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; + return bypassable? null: defaultValue; } + observerOperation.setResult(defaultValue); + // Only bypassable methods can do an early exit via 'complete'. + boolean bypass = execOperation(bypassable, bypassable/*earlyExit*/, observerOperation); + R result = observerOperation.getResult(); + return bypass == bypassable ? result : null; + } + + /** + * @param bypassable True if this method is bypassable (bypassable methods are the exception). + */ + protected boolean execOperation(final boolean bypassable, + final ObserverOperation observerOperation) + throws IOException { + // Set the earlyExit flag same as whatever bypassable is. + return execOperation(bypassable, bypassable/*earlyExit*/, observerOperation); } + /** + * @return True if we are to bypass (but this method defaults NOT bypassable so return out of + * here should always be 'false' for do NOT bypass). + */ protected boolean execOperation(final ObserverOperation observerOperation) throws IOException { - return execOperation(true, observerOperation); + // Default to methods NOT being bypassable and NOT being early exitable. + return execOperation(false, false, observerOperation); } - protected boolean execOperation(final boolean earlyExit, + /** + * @param bypassable True if this method is bypassable (bypassable methods are the exception). + * @param earlyExit Generally, only bypassable methods can early exit from a chain of + * coprocessor calls by setting 'complete' on the context but for now let it be + * settable independent of bypassable in case we need this facility internally. + * @return True if we are to bypass (Can only be 'true' if bypassable is 'true'). + */ + protected boolean execOperation(final boolean bypassable, 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,7 +683,10 @@ public abstract class CoprocessorHost boolean execShutdown(final ObserverOperation observerOperation) - throws IOException { + protected boolean execShutdown(final ObserverOperation observerOperation) throws IOException { + // Default NOT bypassable and NO early exit. + return execShutdown(false, false, observerOperation); + } + + protected boolean execShutdown(final boolean bypassable, + final boolean earlyExit, final ObserverOperation observerOperation) throws IOException { if (observerOperation == null) return false; boolean bypass = false; List envs = coprocEnvironments.get(); @@ -706,9 +730,14 @@ public abstract class CoprocessorHostCoprocessorEnvironment reference swapped out for each * coprocessor. @@ -42,14 +42,20 @@ 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 normal HBase obtained value. Does not work on all invocations, + * only on a small subset of preXXX methods. 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. */ void bypass(); /** * 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. + * chain do not need to be invoked. 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. */ void complete(); @@ -60,5 +66,4 @@ public interface ObserverContext { * context. */ Optional getCaller(); - } 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 f0c9ec28f5..5950d3f8e3 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 @@ -3404,43 +3404,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/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 5d450cc6cd..3ede9b56f3 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 @@ -665,33 +665,31 @@ public class RSRpcServices implements HBaseRPCErrorHandler, checkCellSizeLimit(region, append); spaceQuota.getPolicyEnforcement(region).check(append); quota.addMutation(append); - Result r = null; if (region.getCoprocessorHost() != null) { - r = region.getCoprocessorHost().preAppend(append); + region.getCoprocessorHost().preAppend(append); } - if (r == null) { - boolean canProceed = startNonceOperation(mutation, nonceGroup); - boolean success = false; - try { - long nonce = mutation.hasNonce() ? mutation.getNonce() : HConstants.NO_NONCE; - if (canProceed) { - r = region.append(append, nonceGroup, nonce); - } else { - // convert duplicate append to get - List results = region.get(ProtobufUtil.toGet(mutation, cellScanner), false, - nonceGroup, nonce); - r = Result.create(results); - } - success = true; - } finally { - if (canProceed) { - endNonceOperation(mutation, nonceGroup, success); - } + boolean canProceed = startNonceOperation(mutation, nonceGroup); + boolean success = false; + Result r = null; + try { + long nonce = mutation.hasNonce() ? mutation.getNonce() : HConstants.NO_NONCE; + if (canProceed) { + region.append(append, nonceGroup, nonce); + } else { + // convert duplicate append to get + List results = region.get(ProtobufUtil.toGet(mutation, cellScanner), false, + nonceGroup, nonce); + Result.create(results); } - if (region.getCoprocessorHost() != null) { - r = region.getCoprocessorHost().postAppend(append, r); + success = true; + } finally { + if (canProceed) { + endNonceOperation(mutation, nonceGroup, success); } } + if (region.getCoprocessorHost() != null) { + r = region.getCoprocessorHost().postAppend(append, r); + } if (regionServer.metricsRegionServer != null) { regionServer.metricsRegionServer.updateAppend( EnvironmentEdgeManager.currentTime() - before); @@ -716,33 +714,31 @@ public class RSRpcServices implements HBaseRPCErrorHandler, checkCellSizeLimit(region, increment); spaceQuota.getPolicyEnforcement(region).check(increment); quota.addMutation(increment); - Result r = null; if (region.getCoprocessorHost() != null) { - r = region.getCoprocessorHost().preIncrement(increment); + region.getCoprocessorHost().preIncrement(increment); } - if (r == null) { - boolean canProceed = startNonceOperation(mutation, nonceGroup); - boolean success = false; - try { - long nonce = mutation.hasNonce() ? mutation.getNonce() : HConstants.NO_NONCE; - if (canProceed) { - r = region.increment(increment, nonceGroup, nonce); - } else { - // convert duplicate increment to get - List results = region.get(ProtobufUtil.toGet(mutation, cells), false, nonceGroup, - nonce); - r = Result.create(results); - } - success = true; - } finally { - if (canProceed) { - endNonceOperation(mutation, nonceGroup, success); - } + boolean canProceed = startNonceOperation(mutation, nonceGroup); + boolean success = false; + Result r = null; + try { + long nonce = mutation.hasNonce() ? mutation.getNonce() : HConstants.NO_NONCE; + if (canProceed) { + r = region.increment(increment, nonceGroup, nonce); + } else { + // convert duplicate increment to get + List results = region.get(ProtobufUtil.toGet(mutation, cells), false, nonceGroup, + nonce); + r = Result.create(results); } - if (region.getCoprocessorHost() != null) { - r = region.getCoprocessorHost().postIncrement(increment, r); + success = true; + } finally { + if (canProceed) { + endNonceOperation(mutation, nonceGroup, success); } } + if (region.getCoprocessorHost() != null) { + r = region.getCoprocessorHost().postIncrement(increment, r); + } if (regionServer.metricsRegionServer != null) { regionServer.metricsRegionServer.updateIncrement( EnvironmentEdgeManager.currentTime() - before); @@ -2731,8 +2727,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); + region.getCoprocessorHost().preCheckAndPut(row, family, qualifier, compareOp, + comparator, put); } if (processed == null) { boolean result = region.checkAndMutate(row, family, @@ -2762,8 +2758,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); + 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 e25b0905ce..4289213758 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 @@ -552,7 +552,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 RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { observer.preClose(this, abortRequested); @@ -566,7 +566,7 @@ public class RegionCoprocessorHost */ public void postClose(final boolean abortRequested) { try { - execOperation(false, new RegionObserverOperation() { + execOperation(new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { observer.postClose(this, abortRequested); @@ -635,7 +635,7 @@ public class RegionCoprocessorHost 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 : + return execOperationWithResult(scanner, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult( regionObserverGetter, user) { @Override @@ -671,7 +671,7 @@ public class RegionCoprocessorHost */ public InternalScanner preFlush(HStore store, InternalScanner scanner, FlushLifeCycleTracker tracker) throws IOException { - return execOperationWithResult(false, scanner, coprocEnvironments.isEmpty() ? null + return execOperationWithResult(scanner, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult(regionObserverGetter) { @Override public InternalScanner call(RegionObserver observer) throws IOException { @@ -722,13 +722,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() { + return execOperation(true/*bypassable*/, + coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { observer.preGetOp(this, get, results); @@ -738,7 +741,7 @@ 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) @@ -752,13 +755,14 @@ 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 : + return execOperationWithResult(true, false, + coprocEnvironments.isEmpty()? null: new ObserverOperationWithResult(regionObserverGetter) { @Override public Boolean call(RegionObserver observer) throws IOException { @@ -785,6 +789,7 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param put The Put object * @param edit The WALEdit object. * @param durability The durability used @@ -793,7 +798,8 @@ public class RegionCoprocessorHost */ public boolean prePut(final Put put, final WALEdit edit, final Durability durability) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + return execOperation(true, coprocEnvironments.isEmpty()? null: + new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { observer.prePut(this, put, edit, durability); @@ -802,21 +808,24 @@ 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() { + return execOperation(true, coprocEnvironments.isEmpty()? null: + new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { - observer.prePrepareTimeStampForDeleteVersion(this, mutation, kv, byteNow, get); + observer.prePrepareTimeStampForDeleteVersion(this, mutation, kv, byteNow, get); } }); } @@ -838,6 +847,7 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param delete The Delete object * @param edit The WALEdit object. * @param durability The durability used @@ -846,10 +856,11 @@ public class RegionCoprocessorHost */ public boolean preDelete(final Delete delete, final WALEdit edit, final Durability durability) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { + return execOperation(true, coprocEnvironments.isEmpty()? null: + new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { - observer.preDelete(this, delete, edit, durability); + observer.preDelete(this, delete, edit, durability); } }); } @@ -870,25 +881,16 @@ 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() { - @Override + execOperation(coprocEnvironments.isEmpty()? null: + new RegionObserverOperation() { @Override public void call(RegionObserver observer) throws IOException { observer.preBatchMutate(this, miniBatchOp); } }); } - /** - * @param miniBatchOp - * @throws IOException - */ public void postBatchMutate( final MiniBatchOperationInProgress miniBatchOp) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new RegionObserverOperation() { @@ -917,15 +919,13 @@ public class RegionCoprocessorHost * @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 */ - public Boolean preCheckAndPut(final byte [] row, final byte [] family, + public void 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 : + execOperationWithResult(false, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult(regionObserverGetter) { @Override public Boolean call(RegionObserver observer) throws IOException { @@ -936,6 +936,7 @@ public class RegionCoprocessorHost } /** + * Supports Coprocessor 'bypass'. * @param row row to check * @param family column family * @param qualifier column qualifier @@ -989,15 +990,13 @@ public class RegionCoprocessorHost * @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 */ - public Boolean preCheckAndDelete(final byte [] row, final byte [] family, + public void 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 : + execOperationWithResult(false, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult(regionObserverGetter) { @Override public Boolean call(RegionObserver observer) throws IOException { @@ -1008,14 +1007,15 @@ 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 + * @return true or false to return to client if default processing should be bypassed, + * or null otherwise * @throws IOException e */ public Boolean preCheckAndDeleteAfterRowLock(final byte[] row, final byte[] family, @@ -1056,12 +1056,10 @@ public class RegionCoprocessorHost /** * @param append append object - * @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 : + public void preAppend(final Append append) throws IOException { + execOperationWithResult(null, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult(regionObserverGetter) { @Override public Result call(RegionObserver observer) throws IOException { @@ -1071,9 +1069,9 @@ 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 { @@ -1088,12 +1086,10 @@ public class RegionCoprocessorHost /** * @param increment increment object - * @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 : + public void preIncrement(final Increment increment) throws IOException { + execOperationWithResult(null, coprocEnvironments.isEmpty() ? null : new ObserverOperationWithResult(regionObserverGetter) { @Override public Result call(RegionObserver observer) throws IOException { @@ -1103,9 +1099,9 @@ 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 { 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 41bd99780b..b9f3678eb8 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 @@ -358,7 +358,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. @@ -2439,8 +2439,10 @@ public class TestHRegion { return false; } }; + /* TODO: Broke Tests. You can no longer bypass so no longer can set answer. when(mockedCPHost.preBatchMutate(Mockito.isA(MiniBatchOperationInProgress.class))) .then(answer); + */ region.setCoprocessorHost(mockedCPHost); region.put(originalPut); region.setCoprocessorHost(normalCPHost); -- 2.11.0 (Apple Git-81)