diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java index 6aa5d977b678..b0a04c5044ac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java @@ -22,20 +22,19 @@ import java.util.List; import java.util.Optional; -import org.apache.yetus.audience.InterfaceAudience; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CoprocessorEnvironment; -import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor; import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.RegionObserver; -import org.apache.hadoop.hbase.regionserver.InternalScanner; import org.apache.hadoop.hbase.wal.WALEdit; +import org.apache.yetus.audience.InterfaceAudience; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /*** * Processes multiple {@link Constraint Constraints} on a given table. @@ -98,11 +97,4 @@ public void prePut(ObserverContext e, Put put, } // if we made it here, then the Put is valid } - - @Override - public boolean postScannerFilterRow(final ObserverContext e, - final InternalScanner s, final Cell curRowCell, final boolean hasMore) throws IOException { - // 'default' in RegionObserver might do unnecessary copy for Off heap backed Cells. - return hasMore; - } } 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 16fd33201d1a..2feb270664ef 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 @@ -105,6 +105,13 @@ public class RegionCoprocessorHost // optimization: no need to call postScannerFilterRow, if no coprocessor implements it private final boolean hasCustomPostScannerFilterRow; + /* + * Whether any configured CPs override postScannerFilterRow hook + */ + public boolean hasCustomPostScannerFilterRow() { + return hasCustomPostScannerFilterRow; + } + /** * * Encapsulation of the environment of each coprocessor @@ -278,11 +285,10 @@ public RegionCoprocessorHost(final HRegion region, out: for (RegionCoprocessorEnvironment env: coprocEnvironments) { if (env.getInstance() instanceof RegionObserver) { Class clazz = env.getInstance().getClass(); - for(;;) { - if (clazz == null) { - // we must have directly implemented RegionObserver - hasCustomPostScannerFilterRow = true; - break out; + for (;;) { + if (clazz == Object.class) { + // we dont need to look postScannerFilterRow into Object class + break; // break the inner loop } try { clazz.getDeclaredMethod("postScannerFilterRow", ObserverContext.class, diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index ab8a7e7341df..d747e7f5c9a0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -1877,13 +1877,6 @@ public void postScannerClose(final ObserverContext scannerOwners.remove(s); } - @Override - public boolean postScannerFilterRow(final ObserverContext e, - final InternalScanner s, final Cell curRowCell, final boolean hasMore) throws IOException { - // 'default' in RegionObserver might do unnecessary copy for Off heap backed Cells. - return hasMore; - } - /** * Verify, when servicing an RPC, that the caller is the scanner owner. * If so, we assume that access control is correctly enforced based on diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java index 2a18551ffcd9..65cc58e07397 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java @@ -741,13 +741,6 @@ private Cell createNewCellWithTags(Mutation mutation, Cell newCell) throws IOExc return PrivateCellUtil.createCell(newCell, tags); } - @Override - public boolean postScannerFilterRow(final ObserverContext e, - final InternalScanner s, final Cell curRowCell, final boolean hasMore) throws IOException { - // 'default' in RegionObserver might do unnecessary copy for Off heap backed Cells. - return hasMore; - } - /****************************** VisibilityEndpoint service related methods ******************************/ @Override public synchronized void addLabels(RpcController controller, VisibilityLabelsRequest request, diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java index ee6e21615b5c..fe2ccc58108f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java @@ -22,9 +22,14 @@ import static org.apache.hadoop.hbase.coprocessor.CoprocessorHost.SKIP_LOAD_DUPLICATE_TABLE_COPROCESSOR; import static org.apache.hadoop.hbase.coprocessor.CoprocessorHost.USER_COPROCESSORS_ENABLED_CONF_KEY; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.io.IOException; +import java.util.Optional; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -37,9 +42,13 @@ import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; import org.apache.hadoop.hbase.regionserver.RegionServerServices; import org.apache.hadoop.hbase.testclassification.SmallTests; + +import org.junit.Before; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; @Category({SmallTests.class}) public class TestRegionCoprocessorHost { @@ -48,6 +57,39 @@ public class TestRegionCoprocessorHost { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestRegionCoprocessorHost.class); + @Rule public final TestName name = new TestName(); + private Configuration conf; + private RegionInfo regionInfo; + private HRegion region; + private RegionServerServices rsServices; + + @Before public void setup() throws IOException { + init(null); + } + + private void init(Boolean flag) throws IOException { + conf = HBaseConfiguration.create(); + conf.setBoolean(COPROCESSORS_ENABLED_CONF_KEY, true); + conf.setBoolean(USER_COPROCESSORS_ENABLED_CONF_KEY, true); + TableName tableName = TableName.valueOf(name.getMethodName()); + regionInfo = RegionInfoBuilder.newBuilder(tableName).build(); + TableDescriptor tableDesc = null; + if (flag == null) { + // configure a coprocessor which override postScannerFilterRow + tableDesc = TableDescriptorBuilder.newBuilder(tableName) + .setCoprocessor(SimpleRegionObserver.class.getName()).build(); + } else if (flag) { + // configure a coprocessor which don't override postScannerFilterRow + tableDesc = TableDescriptorBuilder.newBuilder(tableName) + .setCoprocessor(TempRegionObserver.class.getName()).build(); + } else { + // configure two coprocessors, one don't override postScannerFilterRow but another one does + conf.set(REGION_COPROCESSOR_CONF_KEY, TempRegionObserver.class.getName()); + tableDesc = TableDescriptorBuilder.newBuilder(tableName) + .setCoprocessor(SimpleRegionObserver.class.getName()).build(); + } + } + @Test public void testLoadDuplicateCoprocessor() throws Exception { Configuration conf = HBaseConfiguration.create(); @@ -74,4 +116,35 @@ public void testLoadDuplicateCoprocessor() throws Exception { // Two duplicate coprocessors loaded assertEquals(2, host.coprocEnvironments.size()); } + + @Test + public void testPostScannerFilterRow() throws IOException { + // By default SimpleRegionObserver is set as region coprocessor which implements + // postScannerFilterRow + RegionCoprocessorHost host = new RegionCoprocessorHost(region, rsServices, conf); + assertTrue("Region coprocessor implement postScannerFilterRow", + host.hasCustomPostScannerFilterRow()); + + // Set a region CP which doesn't implement postScannerFilterRow + init(true); + host = new RegionCoprocessorHost(region, rsServices, conf); + assertFalse("Region coprocessor implement postScannerFilterRow", + host.hasCustomPostScannerFilterRow()); + + // Set multiple region CPs, in which one implements postScannerFilterRow + init(false); + host = new RegionCoprocessorHost(region, rsServices, conf); + assertTrue("Region coprocessor doesn't implement postScannerFilterRow", + host.hasCustomPostScannerFilterRow()); + } + + /* + * Simple region coprocessor which doesn't override postScannerFilterRow + */ + public static class TempRegionObserver implements RegionCoprocessor, RegionObserver { + @Override + public Optional getRegionObserver() { + return Optional.of(this); + } + } } \ No newline at end of file