Apache9 commented on a change in pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#discussion_r554266044
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##########
@@ -2388,51 +2388,56 @@ public void run(Timeout timeout) throws Exception {
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 =
ClientMetaTableAccessor.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 =
-
CatalogFamilyFormat.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 =
- ClientMetaTableAccessor.getRegionLocation(metaTable,
regionNameOrEncodedRegionName);
- }
+ future =
ClientMetaTableAccessor.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 = CatalogFamilyFormat.parseRegionInfoFromRegionName(
+ regionNameOrEncodedRegionName);
+ } catch (IOException ioe) {
+ throw new IllegalArgumentException(ioe.getMessage());
Review comment:
Please return a failedFuture here instead of throwing it directly?
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java
##########
@@ -300,8 +300,9 @@ public void testCloseRegionIfInvalidRegionNameIsPassed()
throws Exception {
info = regionInfo;
try {
ADMIN.unassign(Bytes.toBytes("sample"), true);
- } catch (UnknownRegionException nsre) {
- // expected, ignore it
+ } catch (IllegalArgumentException iae) {
Review comment:
I think here we want to test UnknownRegionException, not
IllegalArgumentException? So I think we should change 'sample' to a valid but
not exists region name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
