This is an automated email from the ASF dual-hosted git repository. apurtell pushed a commit to branch branch-2.5 in repository https://gitbox.apache.org/repos/asf/hbase.git
commit 2bbddfee1e529600eedf1f196f24e94015239f34 Author: Wellington Ramos Chevreuil <[email protected]> AuthorDate: Fri Mar 25 10:04:13 2022 +0000 HBASE-26881 Backport HBASE-25368 to branch-2 (#4267) Signed-off-by: Andrew Purtell <[email protected]> --- .../hadoop/hbase/client/RawAsyncHBaseAdmin.java | 86 +++++++++++----------- .../org/apache/hadoop/hbase/client/RegionInfo.java | 20 ++++- .../org/apache/hadoop/hbase/client/TestAdmin1.java | 19 +++++ .../org/apache/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 3ae948b..29698c2 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 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { if (regionNameOrEncodedRegionName == null) { return failedFuture(new IllegalArgumentException("Passed region name can't be null")); } - try { - CompletableFuture<Optional<HRegionLocation>> 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<Optional<HRegionLocation>> 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<HRegionLocation> 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<HRegionLocation> 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 e105dbc..81dc633 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 @@ public interface RegionInfo extends Comparable<RegionInfo> { * @return True if <code>regionName</code> 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 fb3b4dd..d85feda 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 @@ package org.apache.hadoop.hbase.client; 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; @@ -131,6 +132,24 @@ public class TestAdmin1 extends TestAdminBase { } @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()); TableDescriptor htd = TableDescriptorBuilder.newBuilder(tableName) 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 5c855f4..6c16416 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 @@ package org.apache.hadoop.hbase.client; 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 class TestAdmin2 extends TestAdminBase { 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)); } } }
