You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by pa...@apache.org on 2020/12/10 18:37:06 UTC
[hbase] branch branch-2.4 updated: HBASE-25277 postScannerFilterRow
impacts Scan performance a lot in HBase 2.x (#2675)
This is an automated email from the ASF dual-hosted git repository.
pankajkumar pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.4 by this push:
new ec8eb65 HBASE-25277 postScannerFilterRow impacts Scan performance a lot in HBase 2.x (#2675)
ec8eb65 is described below
commit ec8eb653f624baaf15bc3b61978832ffde417493
Author: Pankaj <pa...@apache.org>
AuthorDate: Mon Dec 7 23:00:48 2020 +0530
HBASE-25277 postScannerFilterRow impacts Scan performance a lot in HBase 2.x (#2675)
* HBASE-25277 postScannerFilterRow impacts Scan performance a lot in HBase 2.x
1. Added a check for Object class in RegionCoprocessorHost to avoid wrong initialization of hasCustomPostScannerFilterRow
2. Removed dummy implementation of postScannerFilterRow from AccessController, VisibilityController & ConstraintProcessor (which are not required currently)
Signed-off-by Ramkrishna S Vasudevan <ra...@apache.org>
Signed-off-by Anoop Sam John <an...@apache.org>
Signed-off-by: Duo Zhang <zh...@apache.org>
(cherry picked from commit fb6e498b32e48aa606ef5427013fd84452cc762f)
---
.../hbase/constraint/ConstraintProcessor.java | 18 ++-----
.../hbase/regionserver/RegionCoprocessorHost.java | 16 ++++--
.../hbase/security/access/AccessController.java | 7 ---
.../security/visibility/VisibilityController.java | 7 ---
.../coprocessor/TestRegionCoprocessorHost.java | 57 ++++++++++++++++++++--
5 files changed, 69 insertions(+), 36 deletions(-)
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 6aa5d97..b0a04c5 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.ArrayList;
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 class ConstraintProcessor implements RegionCoprocessor, RegionObserver {
}
// if we made it here, then the Put is valid
}
-
- @Override
- public boolean postScannerFilterRow(final ObserverContext<RegionCoprocessorEnvironment> 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 56664db..5818524 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 class RegionCoprocessorHost
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 0d5ac23fc..86884be 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
@@ -1852,13 +1852,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
scannerOwners.remove(s);
}
- @Override
- public boolean postScannerFilterRow(final ObserverContext<RegionCoprocessorEnvironment> 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 305c6d1..c8a6dcd 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
@@ -677,13 +677,6 @@ public class VisibilityController implements MasterCoprocessor, RegionCoprocesso
return PrivateCellUtil.createCell(newCell, tags);
}
- @Override
- public boolean postScannerFilterRow(final ObserverContext<RegionCoprocessorEnvironment> 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 423a412..b0188d9 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,11 +22,14 @@ import static org.apache.hadoop.hbase.coprocessor.CoprocessorHost.REGION_COPROCE
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.CellComparator;
import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -58,7 +61,6 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;
-import java.io.IOException;
@Category({SmallTests.class})
public class TestRegionCoprocessorHost {
@@ -79,19 +81,36 @@ public class TestRegionCoprocessorHost {
@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();
- // config a same coprocessor with system coprocessor
- TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName)
- .setCoprocessor(SimpleRegionObserver.class.getName()).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();
+ }
region = mock(HRegion.class);
when(region.getRegionInfo()).thenReturn(regionInfo);
when(region.getTableDescriptor()).thenReturn(tableDesc);
rsServices = mock(RegionServerServices.class);
}
+
@Test
public void testLoadDuplicateCoprocessor() throws Exception {
conf.setBoolean(SKIP_LOAD_DUPLICATE_TABLE_COPROCESSOR, true);
@@ -158,6 +177,27 @@ public class TestRegionCoprocessorHost {
verifyScanInfo(newScanInfo);
}
+ @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());
+ }
+
private void verifyScanInfo(ScanInfo newScanInfo) {
assertEquals(KeepDeletedCells.TRUE, newScanInfo.getKeepDeletedCells());
assertEquals(MAX_VERSIONS, newScanInfo.getMaxVersions());
@@ -175,4 +215,13 @@ public class TestRegionCoprocessorHost {
CellComparator.getInstance(), true);
}
+ /*
+ * Simple region coprocessor which doesn't override postScannerFilterRow
+ */
+ public static class TempRegionObserver implements RegionCoprocessor, RegionObserver {
+ @Override
+ public Optional<RegionObserver> getRegionObserver() {
+ return Optional.of(this);
+ }
+ }
}