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 2020/12/08 18:59:17 UTC
[GitHub] [hbase] huaxiangsun opened a new pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
huaxiangsun opened a new pull request #2753:
URL: https://github.com/apache/hbase/pull/2753
…me(byte[] regionName)
This addresses a few issues:
1. It fixes the issue that flush/compact/split from hbase shell cannot run with a table name length over 32 bytes.
2. It saves one query to registry when flush/compact/split a tablename as parameter.
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-741177776
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | 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 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 0s | master passed |
| +1 :green_heart: | compile | 1m 27s | master passed |
| +1 :green_heart: | shadedjars | 7m 12s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 59s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 52s | 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 | 7m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 12s | hbase-client in the patch passed. |
| -1 :x: | unit | 205m 16s | hbase-server in the patch failed. |
| | | 237m 12s | |
| 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-2753/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9569241c9899 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 / 56dd3eba81 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/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-2753/1/testReport/ |
| Max. process+thread count | 3578 (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-2753/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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r538798602
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
It needs to have meta regionname to compare (another query). This will defeat the purpose of optimization, so instead, it just check if it is an integer. If it is an integer, then this is a possible encoded 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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742273859
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 9s | 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 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 7s | master passed |
| +1 :green_heart: | checkstyle | 1m 39s | master passed |
| +1 :green_heart: | spotbugs | 3m 10s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 46s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 39s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 40s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 42s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 47m 48s | |
| 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-2753/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 43ef7efeecd7 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 / 7a532f8328 |
| Max. process+thread count | 84 (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-2753/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] saintstack commented on pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
saintstack commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742993832
Seems good to go @huaxiangsun ?
----------------------------------------------------------------
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] saintstack commented on pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
saintstack commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742254863
Failure seems unrelated. Rerunning anyways while Huaxiang sleeps.
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742817104
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 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 | 3m 59s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 7m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 58s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 51s | 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 9s | 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 12s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 205m 54s | hbase-server in the patch passed. |
| | | 242m 48s | |
| 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-2753/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux abc6c132d4ae 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 / c62c18dca7 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/3/testReport/ |
| Max. process+thread count | 3034 (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-2753/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] saintstack commented on a change in pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r539859613
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
THanks for explanation. Shove this up in the JIRA description? Its good. 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] saintstack commented on a change in pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r538781244
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
We know the hbase:meta int. It does not change. Compare it? When meta splits, it will have md5 for new regions.
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
What are we protecting against? Could we be passed a tablename? If so, why can't we have a tablename that is an MD5? Or 32 bytes in size? Should we check it is all hex at least? I suppose if someone passes a tablename that is an md5, then they are asking for trouble?
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
How about some tests for this change in this method?
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742413922
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 14s | 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 21s | 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 22s | 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 15s | the patch passed |
| +1 :green_heart: | compile | 1m 32s | the patch passed |
| +1 :green_heart: | javac | 1m 32s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 18s | 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 14s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 220m 5s | hbase-server in the patch passed. |
| | | 253m 2s | |
| 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-2753/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 87a069fb2681 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 / 7a532f8328 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/2/testReport/ |
| Max. process+thread count | 2956 (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-2753/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] saintstack commented on pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
saintstack commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-744640880
Ok. Still +1 on this @huaxiangsun
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742821769
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 46s | 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 41s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 58s | master passed |
| +1 :green_heart: | compile | 1m 48s | master passed |
| +1 :green_heart: | shadedjars | 7m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 15s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 44s | the patch passed |
| +1 :green_heart: | compile | 1m 47s | the patch passed |
| +1 :green_heart: | javac | 1m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 9s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 26s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 215m 43s | hbase-server in the patch passed. |
| | | 252m 48s | |
| 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-2753/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 79a132cbaa4c 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 / c62c18dca7 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/3/testReport/ |
| Max. process+thread count | 3005 (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-2753/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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742365686
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | 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 13s | master passed |
| +1 :green_heart: | compile | 1m 33s | master passed |
| +1 :green_heart: | shadedjars | 6m 41s | 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 6s | 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 40s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 12s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 133m 21s | hbase-server in the patch passed. |
| | | 164m 46s | |
| 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-2753/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a0cdfea8847f 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 / 7a532f8328 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/2/testReport/ |
| Max. process+thread count | 3910 (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-2753/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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r538802217
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
The new test case testCompactATableWithSuperLongTableName() is for this purpose.
It calls majorCompactRegion() for tableName with length over 32 bytes and tested a short-length table name, expecting it throw out the expected exception.
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-743392205
Let me introduce the isMD5Hash, @saintstack
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r539631859
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
##########
@@ -116,6 +130,8 @@ public void testCompactionTimestamps() throws Exception {
assertEquals(0, ts);
ADMIN.flush(tableName);
+ //ADMIN.majorCompactRegion(Bytes.toBytes("abcdefghijklmnopqrstuvwxyzabcdedgeghijklmnopqrs"));
+ ADMIN.majorCompactRegion(Bytes.toBytes("abcd"));
Review comment:
Will remove these two lines, they are used for testing before the new test case.
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-742708941
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 46s | 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 26s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 19s | master passed |
| +1 :green_heart: | checkstyle | 1m 43s | master passed |
| +1 :green_heart: | spotbugs | 3m 26s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 52s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 41s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 55s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 39s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 28s | The patch does not generate ASF License warnings. |
| | | 51m 57s | |
| 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-2753/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 3dca26296484 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 / c62c18dca7 |
| Max. process+thread count | 84 (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-2753/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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-741082772
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 12s | master passed |
| +1 :green_heart: | compile | 1m 33s | master passed |
| +1 :green_heart: | shadedjars | 6m 38s | 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 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 4s | 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 38s | 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 19s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 133m 32s | hbase-server in the patch passed. |
| | | 165m 17s | |
| 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-2753/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1427c472c5a8 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 / 56dd3eba81 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2753/1/testReport/ |
| Max. process+thread count | 4164 (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-2753/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 pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-743699822
Actually, I thought about it. If I introduce apache.common.codec.MD5Hex.decodeHex(), it is heavy (allocate/copy). I think it is ok without introducing MD5 as most of the calls are the real encodedRegionNames. Only for shell commends, table name can be passed in. In this case, it is ok to have a query to registry in case table name length is 32 bytes (which could be rare as well).
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#issuecomment-740928698
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 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 20s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | checkstyle | 1m 43s | master passed |
| +1 :green_heart: | spotbugs | 3m 20s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 56s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 40s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 28s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 48m 33s | |
| 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-2753/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2753 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 05f6f026e08e 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 / 56dd3eba81 |
| Max. process+thread count | 84 (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-2753/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 merged pull request #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun merged pull request #2753:
URL: https://github.com/apache/hbase/pull/2753
----------------------------------------------------------------
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 #2753: HBASE-25368 Filter out more invalid encoded name in isEncodedRegionNa…
Posted by GitBox <gi...@apache.org>.
huaxiangsun commented on a change in pull request #2753:
URL: https://github.com/apache/hbase/pull/2753#discussion_r538795053
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
##########
@@ -363,7 +363,23 @@ static TableName getTable(final byte [] regionName) {
@InterfaceAudience.Private // For use by internals only.
public 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;
Review comment:
Thanks @saintstack. Two issues are addressed.
For compact/flush/split with the tableName,
` # Requests a table or region or column family major compaction
def major_compact(table_or_region_name, family = nil, type = 'NORMAL')
family_bytes = nil
family_bytes = family.to_java_bytes unless family.nil?
compact_type = nil
if type == 'NORMAL'
compact_type = org.apache.hadoop.hbase.client.CompactType::NORMAL
elsif type == 'MOB'
compact_type = org.apache.hadoop.hbase.client.CompactType::MOB
else
raise ArgumentError, 'only NORMAL or MOB accepted for type!'
end
begin
if family_bytes.nil?
@admin.majorCompactRegion(table_or_region_name.to_java_bytes)
else
@admin.majorCompactRegion(table_or_region_name.to_java_bytes, family_bytes)
end
rescue java.lang.IllegalArgumentException, org.apache.hadoop.hbase.UnknownRegionException
if family_bytes.nil?
@admin.majorCompact(TableName.valueOf(table_or_region_name), compact_type)
else
@admin.majorCompact(TableName.valueOf(table_or_region_name), family_bytes, compact_type)
end
end
end
`
it first calls majorCompactRegion() with tableName as input. It expects an IllegalArgumentException or UnknownRegionException to call majorCompact().
This normally involves a registry query to get this UnknownRegionException, this is an expensive path.
If it knows that the input string is not an encodedRegionName or regionName, it can throw out IllegalArgumentException without registry query.
For the case that a md5 hex being used as tableName, it has to go through expensive path to find out that this is not an encodedRegionName, it will still work, just optimization wont be applied in this case.
It also fixes a bug for a table name length over 32 bytes, currently, compact/flush/split this tableName from shell will fail.
----------------------------------------------------------------
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