From c491ec939671e3d0e755fc1749c08d06a7d43369 Mon Sep 17 00:00:00 2001 From: huaxiangsun Date: Thu, 17 Dec 2020 15:21:30 -0800 Subject: [PATCH] HBASE-25356 HBaseAdmin#getRegion() needs to filter out non-regionName and non-encodedRegionName (#2759) Signed-off-by: stack --- .../hadoop/hbase/MetaTableAccessor.java | 4 ++- .../hadoop/hbase/client/HBaseAdmin.java | 9 +++++- .../hadoop/hbase/client/RegionInfo.java | 14 ++++++++ .../hadoop/hbase/client/TestAdmin1.java | 32 +++++++++++++++++++ .../hadoop/hbase/client/TestAdminBase.java | 3 +- 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index 8ba102e8c6b5..f9fb2621006c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -285,7 +285,9 @@ public static HRegionLocation getRegionLocation(Connection connection, byte[] re parsedInfo = parseRegionInfoFromRegionName(regionName); row = getMetaKeyForRegion(parsedInfo); } catch (Exception parseEx) { - // Ignore. This is used with tableName passed as regionName. + // If it is not a valid regionName(i.e, tableName), it needs to return null here + // as querying meta table wont help. + return null; } Get get = new Get(row); get.addFamily(HConstants.CATALOG_FAMILY); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index 4b1a7381304e..ef6589a4a04b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -1946,8 +1946,15 @@ Pair getRegion(final byte[] regionName) throws IOExcepti } Pair pair = MetaTableAccessor.getRegion(connection, regionName); if (pair == null) { - final AtomicReference> result = new AtomicReference<>(null); final String encodedName = Bytes.toString(regionName); + // When it is not a valid regionName, it is possible that it could be an encoded regionName. + // To match the encoded regionName, it has to scan the meta table and compare entry by entry. + // Since it scans meta table, so it has to be the MD5 hash, it can filter out + // most of invalid cases. + if (!RegionInfo.isMD5Hash(encodedName)) { + return null; + } + final AtomicReference> result = new AtomicReference<>(null); MetaTableAccessor.Visitor visitor = new MetaTableAccessor.Visitor() { @Override public boolean visit(Result data) throws IOException { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java index d860c7681a37..5882d0f2d142 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java @@ -418,6 +418,20 @@ static RegionInfo parseFrom(final byte [] bytes, int offset, int len) } } + static boolean isMD5Hash(String encodedRegionName) { + if (encodedRegionName.length() != MD5_HEX_LENGTH) { + return false; + } + + for (int i = 0; i < encodedRegionName.length(); i++) { + char c = encodedRegionName.charAt(i); + if (!((c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f') || (c >= '0' && c <= '9'))) { + return false; + } + } + return true; + } + /** * Check whether two regions are adjacent; i.e. lies just before or just * after in a table. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java index 77e59cf3780e..394be9e4bb9c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java @@ -41,8 +41,10 @@ import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.regionserver.DisabledRegionSplitPolicy; import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.HStore; import org.apache.hadoop.hbase.regionserver.HStoreFile; +import org.apache.hadoop.hbase.regionserver.Region; import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.util.Bytes; @@ -70,6 +72,36 @@ public class TestAdmin1 extends TestAdminBase { private static final Logger LOG = LoggerFactory.getLogger(TestAdmin1.class); + @Test + public void testCompactRegionWithTableName() throws Exception { + TableName tableName = TableName.valueOf(name.getMethodName()); + try { + TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName). + setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build(); + ADMIN.createTable(htd); + Region metaRegion = null; + for (int i = 0; i < NB_SERVERS; i++) { + HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(i); + List onlineRegions = rs.getRegions(TableName.META_TABLE_NAME); + if (!onlineRegions.isEmpty()) { + metaRegion = onlineRegions.get(0); + break; + } + } + + long metaReadCountBeforeCompact = metaRegion.getReadRequestsCount(); + try { + ADMIN.majorCompactRegion(tableName.getName()); + } catch (IllegalArgumentException iae) { + LOG.info("This is expected"); + } + assertEquals(metaReadCountBeforeCompact, metaRegion.getReadRequestsCount()); + } finally { + ADMIN.disableTable(tableName); + ADMIN.deleteTable(tableName); + } + } + @Test public void testSplitFlushCompactUnknownTable() throws InterruptedException { final TableName unknowntable = TableName.valueOf(name.getMethodName()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdminBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdminBase.java index c379775a602f..d7a910713db9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdminBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdminBase.java @@ -33,6 +33,7 @@ public class TestAdminBase { protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + protected final static int NB_SERVERS = 3; protected static Admin ADMIN; @Rule @@ -47,7 +48,7 @@ public static void setUpBeforeClass() throws Exception { TEST_UTIL.getConfiguration().setInt(HConstants.REGION_SERVER_HIGH_PRIORITY_HANDLER_COUNT, 30); TEST_UTIL.getConfiguration().setInt(HConstants.REGION_SERVER_HANDLER_COUNT, 30); TEST_UTIL.getConfiguration().setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true); - TEST_UTIL.startMiniCluster(3); + TEST_UTIL.startMiniCluster(NB_SERVERS); ADMIN = TEST_UTIL.getAdmin(); }