From 541d748e841022ce53b7f39ca4ef8c81afcf8028 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Fri, 25 Mar 2022 10:04:13 +0000 Subject: [PATCH] HBASE-26881 Backport HBASE-25368 to branch-2 (#4267) Signed-off-by: Andrew Purtell --- .../hbase/client/RawAsyncHBaseAdmin.java | 86 ++++++++++--------- .../hadoop/hbase/client/RegionInfo.java | 20 ++++- .../hadoop/hbase/client/TestAdmin1.java | 19 ++++ .../hadoop/hbase/client/TestAdmin2.java | 9 +- 4 files changed, 86 insertions(+), 48 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index 3ae948bb8d50..29698c201336 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java @@ -2394,51 +2394,55 @@ CompletableFuture getRegionLocation(byte[] regionNameOrEncodedR if (regionNameOrEncodedRegionName == null) { return failedFuture(new IllegalArgumentException("Passed region name can't be null")); } - try { - CompletableFuture> future; - if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) { - String encodedName = Bytes.toString(regionNameOrEncodedRegionName); - if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) { - // old format encodedName, should be meta region - future = connection.registry.getMetaRegionLocations() - .thenApply(locs -> Stream.of(locs.getRegionLocations()) - .filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst()); - } else { - future = AsyncMetaTableAccessor.getRegionLocationWithEncodedName(metaTable, - regionNameOrEncodedRegionName); - } + + CompletableFuture> future; + if (RegionInfo.isEncodedRegionName(regionNameOrEncodedRegionName)) { + String encodedName = Bytes.toString(regionNameOrEncodedRegionName); + if (encodedName.length() < RegionInfo.MD5_HEX_LENGTH) { + // old format encodedName, should be meta region + future = connection.registry.getMetaRegionLocations() + .thenApply(locs -> Stream.of(locs.getRegionLocations()) + .filter(loc -> loc.getRegion().getEncodedName().equals(encodedName)).findFirst()); } else { - RegionInfo regionInfo = - MetaTableAccessor.parseRegionInfoFromRegionName(regionNameOrEncodedRegionName); - if (regionInfo.isMetaRegion()) { - future = connection.registry.getMetaRegionLocations() - .thenApply(locs -> Stream.of(locs.getRegionLocations()) - .filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId()) - .findFirst()); - } else { - future = - AsyncMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName); - } + future = AsyncMetaTableAccessor.getRegionLocationWithEncodedName(metaTable, + regionNameOrEncodedRegionName); + } + } else { + // Not all regionNameOrEncodedRegionName here is going to be a valid region name, + // it needs to throw out IllegalArgumentException in case tableName is passed in. + RegionInfo regionInfo; + try { + regionInfo = MetaTableAccessor.parseRegionInfoFromRegionName(regionNameOrEncodedRegionName); + } catch (IOException ioe) { + return failedFuture(new IllegalArgumentException(ioe.getMessage())); } - CompletableFuture returnedFuture = new CompletableFuture<>(); - addListener(future, (location, err) -> { - if (err != null) { - returnedFuture.completeExceptionally(err); - return; - } - if (!location.isPresent() || location.get().getRegion() == null) { - returnedFuture.completeExceptionally( - new UnknownRegionException("Invalid region name or encoded region name: " + - Bytes.toStringBinary(regionNameOrEncodedRegionName))); - } else { - returnedFuture.complete(location.get()); - } - }); - return returnedFuture; - } catch (IOException e) { - return failedFuture(e); + if (regionInfo.isMetaRegion()) { + future = connection.registry.getMetaRegionLocations() + .thenApply(locs -> Stream.of(locs.getRegionLocations()) + .filter(loc -> loc.getRegion().getReplicaId() == regionInfo.getReplicaId()) + .findFirst()); + } else { + future = + AsyncMetaTableAccessor.getRegionLocation(metaTable, regionNameOrEncodedRegionName); + } } + + CompletableFuture returnedFuture = new CompletableFuture<>(); + addListener(future, (location, err) -> { + if (err != null) { + returnedFuture.completeExceptionally(err); + return; + } + if (!location.isPresent() || location.get().getRegion() == null) { + returnedFuture.completeExceptionally( + new UnknownRegionException("Invalid region name or encoded region name: " + + Bytes.toStringBinary(regionNameOrEncodedRegionName))); + } else { + returnedFuture.complete(location.get()); + } + }); + return returnedFuture; } /** 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 e105dbc3ad81..81dc63346c4a 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 @@ -362,9 +362,25 @@ static byte[] getStartKey(final byte[] regionName) throws IOException { * @return True if regionName represents an encoded name. */ @InterfaceAudience.Private // For use by internals only. - public static boolean isEncodedRegionName(byte[] regionName) { + static boolean isEncodedRegionName(byte[] regionName) { // If not parseable as region name, presume encoded. TODO: add stringency; e.g. if hex. - return parseRegionNameOrReturnNull(regionName) == null && regionName.length <= MD5_HEX_LENGTH; + if (parseRegionNameOrReturnNull(regionName) == null) { + if (regionName.length > MD5_HEX_LENGTH) { + return false; + } else if (regionName.length == MD5_HEX_LENGTH) { + return true; + } else { + String encodedName = Bytes.toString(regionName); + try { + Integer.parseInt(encodedName); + // If this is a valid integer, it could be hbase:meta's encoded region name. + return true; + } catch(NumberFormatException er) { + return false; + } + } + } + return false; } /** 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 fb3b4dd8bfd0..d85feda69a5b 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 @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -130,6 +131,24 @@ public void testSplitFlushCompactUnknownTable() throws InterruptedException { assertTrue(exception instanceof TableNotFoundException); } + @Test + public void testCompactATableWithSuperLongTableName() throws Exception { + TableName tableName = TableName.valueOf(name.getMethodName()); + TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of("fam1")).build(); + try { + ADMIN.createTable(htd); + assertThrows(IllegalArgumentException.class, + () -> ADMIN.majorCompactRegion(tableName.getName())); + + assertThrows(IllegalArgumentException.class, + () -> ADMIN.majorCompactRegion(Bytes.toBytes("abcd"))); + } finally { + ADMIN.disableTable(tableName); + ADMIN.deleteTable(tableName); + } + } + @Test public void testCompactionTimestamps() throws Exception { TableName tableName = TableName.valueOf(name.getMethodName()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java index 5c855f4b47f7..6c16416eac7e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -316,11 +317,9 @@ public void testCloseRegionIfInvalidRegionNameIsPassed() throws Exception { if (!regionInfo.isMetaRegion()) { if (regionInfo.getRegionNameAsString().contains(name)) { info = regionInfo; - try { - ADMIN.unassign(Bytes.toBytes("sample"), true); - } catch (UnknownRegionException nsre) { - // expected, ignore it - } + assertThrows(UnknownRegionException.class, + () -> ADMIN.unassign(Bytes.toBytes( + "test,,1358563771069.acc1ad1b7962564fc3a43e5907e8db33."), true)); } } }