You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2021/01/09 00:30:44 UTC

[GitHub] [hbase] huaxiangsun opened a new pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

huaxiangsun opened a new pull request #2868:
URL: https://github.com/apache/hbase/pull/2868


   …me(byte[] regionName)


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-761078776


   Ping @Apache9 for review, thanks.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757070057


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 47s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   3m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 25s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m  5s |  hbase-server: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 22s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m 17s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m  6s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux a57e55e6465f 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 54eae0fc5c |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-759210930


   Hi @Apache9, please review the update which addresses your comments when you get chance, thanks.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757081366


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 11s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 40s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 42s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 12s |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  |  89m 54s |  hbase-server in the patch failed.  |
   |  |   | 121m 32s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 4d1124f52d51 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 54eae0fc5c |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/testReport/ |
   | Max. process+thread count | 1768 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#discussion_r559170729



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -99,6 +99,25 @@ 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);
+      try {
+        ADMIN.majorCompactRegion(tableName.getName());
+        ADMIN.majorCompactRegion(Bytes.toBytes("abcd"));

Review comment:
       At least please insert a `fail()` here, or better use assertThrows.

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java
##########
@@ -299,7 +299,8 @@ public void testCloseRegionIfInvalidRegionNameIsPassed() throws Exception {
         if (regionInfo.getRegionNameAsString().contains(name)) {
           info = regionInfo;
           try {
-            ADMIN.unassign(Bytes.toBytes("sample"), true);
+            ADMIN.unassign(Bytes.toBytes(

Review comment:
       Ditto. I know this is not your fault but let's fix it too. Change to use assertThrows.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun merged pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun merged pull request #2868:
URL: https://github.com/apache/hbase/pull/2868


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-759066599


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 23s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 23s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  8s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 54s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 11s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 56s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 11s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 208m 30s |  hbase-server in the patch passed.  |
   |  |   | 240m 31s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux ba0544edeb9b 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / c218e576fe |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/testReport/ |
   | Max. process+thread count | 3059 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on a change in pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#discussion_r560423945



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -99,6 +99,25 @@ 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);
+      try {
+        ADMIN.majorCompactRegion(tableName.getName());
+        ADMIN.majorCompactRegion(Bytes.toBytes("abcd"));

Review comment:
       Done

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java
##########
@@ -299,7 +299,8 @@ public void testCloseRegionIfInvalidRegionNameIsPassed() throws Exception {
         if (regionInfo.getRegionNameAsString().contains(name)) {
           info = regionInfo;
           try {
-            ADMIN.unassign(Bytes.toBytes("sample"), true);
+            ADMIN.unassign(Bytes.toBytes(

Review comment:
       Done.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-763173969


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m 12s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 14s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 36s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 43s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  7s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 37s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 41s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 10s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 134m 29s |  hbase-server in the patch passed.  |
   |  |   | 170m 14s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux cec6bd8043b6 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 871eb09b3d |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/testReport/ |
   | Max. process+thread count | 4472 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757062253


   This is the update for review #2753 , it fixes the test failure in TestAdmin2.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-758863686


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 29s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 31s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 28s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 58s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  18m 26s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m 24s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  43m 54s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 4f319329134a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / c218e576fe |
   | Max. process+thread count | 94 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757062253


   This is the update for review #2753 , it fixes the test failure in TestAdmin2.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-758973371


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 24s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 35s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 45s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 34s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 43s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 11s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 135m 14s |  hbase-server in the patch passed.  |
   |  |   | 166m 43s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 49aa6bbb96a7 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / c218e576fe |
   | Default Java | AdoptOpenJDK-11.0.6+10 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/testReport/ |
   | Max. process+thread count | 4525 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-763099197


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  1s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 33s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 35s |  master passed  |
   | +1 :green_heart: |  compile  |   4m  8s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 30s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  javac  |   4m  1s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  17m 11s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.  |
   | +1 :green_heart: |  spotbugs  |   3m 20s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 27s |  The patch does not generate ASF License warnings.  |
   |  |   |  51m 21s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 0c38861314a0 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 871eb09b3d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | Max. process+thread count | 96 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/console |
   | versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-763174463


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 35s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 33s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 23s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 36s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  1s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 30s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  6s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  | 141m 23s |  hbase-server in the patch passed.  |
   |  |   | 171m 20s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 200325dacef0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 871eb09b3d |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/testReport/ |
   | Max. process+thread count | 4044 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/3/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757069579


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 34s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 20s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m  5s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 25s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 11s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 48s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 24s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m 16s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 57s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 13s |  hbase-client in the patch passed.  |
   | -1 :x: |  unit  |   8m 45s |  hbase-server in the patch failed.  |
   |  |   |  40m 44s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2868 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 6cd715b74cd9 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 54eae0fc5c |
   | Default Java | AdoptOpenJDK-1.8.0_232-b09 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/testReport/ |
   | Max. process+thread count | 465 (vs. ulimit of 30000) |
   | modules | C: hbase-client hbase-server U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2868/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on a change in pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#discussion_r555218331



##########
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:
       ok.
   

##########
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:
       ok.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] huaxiangsun commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-758844576


   Pushed changes which addressed review comments.
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2868: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2868:
URL: https://github.com/apache/hbase/pull/2868#issuecomment-757069579






----------------------------------------------------------------
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:
users@infra.apache.org