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/04/09 07:26:51 UTC
[GitHub] [hbase] ZhaoBQ opened a new pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
ZhaoBQ opened a new pull request #3139:
URL: https://github.com/apache/hbase/pull/3139
…t` to `hbase.normalizer.merge.min.region.count`
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-842073105
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 31s | 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 | 5m 4s | master passed |
| +1 :green_heart: | compile | 1m 45s | master passed |
| +1 :green_heart: | shadedjars | 8m 54s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 49s | the patch passed |
| +1 :green_heart: | compile | 1m 45s | the patch passed |
| +1 :green_heart: | javac | 1m 45s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 13s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 204m 1s | hbase-server in the patch passed. |
| | | 245m 10s | |
| 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-3139/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5d1a97bda39c 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1c6994ad70 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/testReport/ |
| Max. process+thread count | 3469 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-816518785
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 1s | 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 20s | master passed |
| +1 :green_heart: | compile | 4m 23s | master passed |
| +1 :green_heart: | checkstyle | 1m 41s | master passed |
| +0 :ok: | refguide | 3m 50s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 3m 10s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 1s | the patch passed |
| +1 :green_heart: | compile | 4m 20s | the patch passed |
| +1 :green_heart: | javac | 4m 20s | the patch passed |
| -0 :warning: | checkstyle | 1m 11s | hbase-server: The patch generated 3 new + 11 unchanged - 2 fixed = 14 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 3m 44s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 19m 53s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 22s | The patch does not generate ASF License warnings. |
| | | 65m 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-3139/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | dupname asflicense javac refguide xml spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux cdab344b2b14 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 / 8e08952499 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 87 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-818500366
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 7s | 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 52s | master passed |
| +1 :green_heart: | compile | 1m 44s | master passed |
| +1 :green_heart: | shadedjars | 8m 26s | 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 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 46s | 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 | 8m 27s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 35s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 200m 9s | hbase-server in the patch passed. |
| | | 239m 0s | |
| 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-3139/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 63b493b8e3b4 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / de012d7d1f |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/testReport/ |
| Max. process+thread count | 3290 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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] ndimiduk commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623288418
##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -638,12 +638,6 @@ possible configurations would overwhelm and obscure the important.
<value>true</value>
<description>Whether to merge a region as part of normalization.</description>
</property>
- <property>
- <name>hbase.normalizer.min.region.count</name>
Review comment:
What's the reasoning for removing the default value from this file? While it's redundant to use in code, this file serves as a source of documentation ; I believe it is parsed by our site building process to populate a section of the online book.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
I _believe_ you can register the old key via the [deprecation API](https://hadoop.apache.org/docs/r2.10.1/api/org/apache/hadoop/conf/Configuration.html#addDeprecation(java.lang.String,%20java.lang.String,%20java.lang.String)) and only ever access the new key from code. I believe this has the benefit of only logging the warning once, instead of every time the implementation happens to do a lookup.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java
##########
@@ -370,6 +371,35 @@ public void testHonorsMergeEnabledInTD() {
@Test
public void testHonorsMinimumRegionCount() {
+ conf.setInt(MERGE_MIN_REGION_COUNT_KEY, 1);
Review comment:
Can you instead keep a single implementation in a helper method that allows for the configuration used to be passed in as a parameter? This would be better than copy-pasting the unit test method body.
--
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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-833219457
Here is my test for Configuration.addDeprecation().
Suppose we define the following configuration：
hbase-site.xml: <property><name>deprecate.conf</name><value>99</value></property>
hbase-default.xml: <property><name>new.conf</name><value>42</value></property>
Test 1, create a Configuration object first:
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 42
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); //120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
`
Test 2, call Configuration.addDeprecation() first:
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
Test 2 is what I expected, even if the deprecated configuration in the configuration file is not changed to a new configuration, it can still run correctly. But we should call Configuration.addDeprecation before create configuration object.
`
Test 3, hbase-default.xml is empty, and hbase-site.xml has:<property><name>deprecate.conf</name><value>99</value></property> <property><name>new.conf</name><value>42</value></property>
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
Then change the order of deprecate.conf and new.conf in hbase-site.xml: <property><name>new.conf</name><value>42</value></property> <property><name>deprecate.conf</name><value>99</value></property>
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
The user should not configure the deprecated and new configuration at the same time, so Test3 and Test4 can be ignored.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-830152140
: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 30s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 44s | master passed |
| +1 :green_heart: | compile | 1m 46s | master passed |
| +1 :green_heart: | shadedjars | 9m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 45s | the patch passed |
| +1 :green_heart: | compile | 1m 44s | the patch passed |
| +1 :green_heart: | javac | 1m 44s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 1s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 36s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 203m 42s | hbase-server in the patch passed. |
| | | 243m 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-3139/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6c05678f044d 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6c65314cdf |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/testReport/ |
| Max. process+thread count | 3174 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-830043211
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | 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 | 4m 4s | master passed |
| +1 :green_heart: | compile | 4m 10s | master passed |
| +1 :green_heart: | checkstyle | 1m 34s | master passed |
| +0 :ok: | refguide | 3m 40s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 2m 55s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 3s | the patch passed |
| +1 :green_heart: | compile | 4m 9s | the patch passed |
| +1 :green_heart: | javac | 4m 9s | the patch passed |
| -0 :warning: | checkstyle | 1m 10s | hbase-server: The patch generated 2 new + 11 unchanged - 2 fixed = 13 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 3m 42s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 19m 57s | 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 22s | The patch does not generate ASF License warnings. |
| | | 63m 56s | |
| 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-3139/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | dupname asflicense javac refguide xml spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux a8d5c9ddea0d 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 / 6c65314cdf |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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] ZhaoBQ edited a comment on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ edited a comment on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-833219457
Here is my test for Configuration.addDeprecation().
Suppose we define the following configuration：
hbase-site.xml: <property><name>deprecate.conf</name><value>99</value></property>
hbase-default.xml: <property><name>new.conf</name><value>42</value></property>
**Test 1, create a Configuration object first:**
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 42
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); //120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
`
**Test 2, call Configuration.addDeprecation() first:**
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
`
**Test 2 is what I expected, even if the deprecated configuration in the configuration file is not changed to a new configuration, it can still run correctly. But we should call Configuration.addDeprecation before create configuration object.**
**Test 3, hbase-default.xml is empty, and hbase-site.xml has:<property><name>deprecate.conf</name><value>99</value></property> <property><name>new.conf</name><value>42</value></property>**
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
**Then change the order of deprecate.conf and new.conf in hbase-site.xml: <property><name>new.conf</name><value>42</value></property> <property><name>deprecate.conf</name><value>99</value></property>**
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
**The user should not configure the deprecated and new configuration at the same time, so Test3 and Test4 can be ignored.**
--
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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623735082
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
And I found after conf.addDeprecation(deprecatedConf, newConf), the conf.get(deprecatedConf) can not get value even we configured in conf file. So if the configuration is not modified before upgrade to HBase-2.5, then the
configuration will be use default value. I don't know if I express cleared...
--
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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-828969307
Ping @ndimiduk
--
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] ndimiduk merged pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk merged pull request #3139:
URL: https://github.com/apache/hbase/pull/3139
--
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] ndimiduk commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-847307159
@ZhaoBQ the master patch has conflicts when I backport it to branch-2. I'm guessing there's a patch on master that's missing on branch-2, because branch-2 doesn't have the `NormalizeContext#getOrDefault` method. If I don't get to it later today, maybe you can take a look? 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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623577936
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
The deprecation API is good, but has some confusion as I said in the jira. When we call conf.addDeprecation(deprecatedConf, newConf), and only set newConf, conf.get(deprecatedConf) always get value even it not set.
--
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] ndimiduk commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-833926085
> So do we need to create another issue to correct this wrong usage first?
First or in addition to, I think is up to you. It looks like you've identified an issue in our usage of this Hadoop API, and have worked out a testing strategy -- this is great! I find 10 usages of `addDeprecation` on a recent checkout of master. If you'd like to pursue this cleanup, and can add explicit test coverage that we're preserving these configurations correctly, I think it'd be great! I don't know how we might prevent future miss-use of this API going forward...
```
$ find . -iname '*.java' -exec grep -iIn 'addDeprecation' {} \+
./hbase-common/src/main/java/org/apache/hadoop/hbase/util/DNS.java:62: Configuration.addDeprecation(RS_HOSTNAME_KEY, UNSAFE_RS_HOSTNAME_KEY);
./hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBuffAllocator.java:104: Configuration.addDeprecation(DEPRECATED_ALLOCATOR_POOL_ENABLED_KEY, ALLOCATOR_POOL_ENABLED_KEY);
./hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBuffAllocator.java:105: Configuration.addDeprecation(DEPRECATED_MAX_BUFFER_COUNT_KEY, MAX_BUFFER_COUNT_KEY);
./hbase-common/src/main/java/org/apache/hadoop/hbase/io/ByteBuffAllocator.java:106: Configuration.addDeprecation(DEPRECATED_BUFFER_SIZE_KEY, BUFFER_SIZE_KEY);
./hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockCacheFactory.java:101: Configuration.addDeprecation(DEPRECATED_BLOCKCACHE_BLOCKSIZE_KEY, BLOCKCACHE_BLOCKSIZE_KEY);
./hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java:493: Configuration.addDeprecation(RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY);
./hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java:69: Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
./hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALPlayer.java:88: Configuration.addDeprecation("hlog.bulk.output", BULK_OUTPUT_CONF_KEY);
./hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALPlayer.java:89: Configuration.addDeprecation("hlog.input.tables", TABLES_KEY);
./hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALPlayer.java:90: Configuration.addDeprecation("hlog.input.tablesmap", TABLE_MAP_KEY);
```
--
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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623735082
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
And I found after conf.addDeprecation(deprecatedConf, newConf), the conf.get(deprecatedConf) can not get value even we configured in conf file. So if the configuration is not modified before upgrade to HBase-2.5, then the
configuration will be use default value. I don't know if i express cleared...
--
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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r627063578
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
Your example is special because you call the c.set() method. But we all define the configuration in hbase-site.xml. If your hbase-site.xml define: <property><name>foo</name><value>99</value></property> and do not call c.set("foo", "99"), the value of c.get("bar") and c.get("foo") is "42". If you call Configuration.addDeprecation("foo", "bar") first, then call Configuration.addDeprecation("foo", "bar"), the value of c.get("bar") and c.get("foo") is 99.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-841971335
--
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] ndimiduk commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r638234965
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -540,24 +546,34 @@ public boolean isMergeEnabled() {
return mergeEnabled;
}
- public int getMinRegionCount() {
- return minRegionCount;
+ public int getMergeMinRegionCount() {
+ return mergeMinRegionCount;
}
- public int getMinRegionCount(NormalizeContext context) {
- int minRegionCount = context.getOrDefault(MIN_REGION_COUNT_KEY, Integer::parseInt, 0);
- if (minRegionCount <= 0) {
- minRegionCount = getMinRegionCount();
+ public int getMergeMinRegionCount(NormalizeContext context) {
+ String stringValue = context.getOrDefault(MERGE_MIN_REGION_COUNT_KEY,
+ Function.identity(), null);
+ if (stringValue == null) {
+ stringValue = context.getOrDefault(MIN_REGION_COUNT_KEY, Function.identity(), null);
+ if (stringValue != null) {
+ LOG.debug("The config key {} in table descriptor is deprecated. Instead please use {}. "
Review comment:
huh. maybe `TableDescriptor` should handle deprecations as well. For another patch.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-818511029
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | 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 9s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 8m 28s | 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 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 23s | the patch passed |
| +1 :green_heart: | javac | 1m 23s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 22s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 36s | hbase-server generated 2 new + 18 unchanged - 2 fixed = 20 total (was 20) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 54s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 220m 40s | hbase-server in the patch passed. |
| | | 256m 8s | |
| 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-3139/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 966f5b830e8d 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / de012d7d1f |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/testReport/ |
| Max. process+thread count | 2786 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-834041587
Create an issue HBASE-25861 to resolve the problem first.
--
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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623577803
##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -638,12 +638,6 @@ possible configurations would overwhelm and obscure the important.
<value>true</value>
<description>Whether to merge a region as part of normalization.</description>
</property>
- <property>
- <name>hbase.normalizer.min.region.count</name>
Review comment:
If here just change to "hbase.normalizer.merge.min.region.count", conf.get(MERGE_MIN_REGION_COUNT_KEY) always get value even user not configured, and we can't be backward compatible.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-817637248
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 15s | 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 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | compile | 4m 12s | master passed |
| +1 :green_heart: | checkstyle | 1m 34s | master passed |
| +0 :ok: | refguide | 3m 40s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 2m 57s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 1s | the patch passed |
| +1 :green_heart: | compile | 4m 8s | the patch passed |
| +1 :green_heart: | javac | 4m 8s | the patch passed |
| -0 :warning: | checkstyle | 1m 11s | hbase-server: The patch generated 3 new + 11 unchanged - 2 fixed = 14 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 3m 35s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 19m 51s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 21s | The patch does not generate ASF License warnings. |
| | | 63m 45s | |
| 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-3139/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | dupname asflicense javac refguide xml spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 1a4a4baa311a 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 / f9e928e5a7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-816622267
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 57s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | 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 | 6m 0s | master passed |
| +1 :green_heart: | compile | 2m 16s | master passed |
| +1 :green_heart: | shadedjars | 9m 48s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 30s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 19s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 19s | the patch passed |
| +1 :green_heart: | compile | 2m 1s | the patch passed |
| +1 :green_heart: | javac | 2m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 10m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 30s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 14s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 205m 14s | hbase-server in the patch passed. |
| | | 251m 56s | |
| 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-3139/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d3d5827a9dc4 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 8e08952499 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/testReport/ |
| Max. process+thread count | 3281 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-845640359
HBASE-25861 is resolved, and I modified this PR. Please review @ndimiduk
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-818432082
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 44s | 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 32s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 23s | master passed |
| +1 :green_heart: | compile | 6m 7s | master passed |
| +1 :green_heart: | checkstyle | 2m 28s | master passed |
| +0 :ok: | refguide | 5m 22s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 4m 40s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 42s | the patch passed |
| +1 :green_heart: | compile | 6m 19s | the patch passed |
| +1 :green_heart: | javac | 6m 19s | the patch passed |
| -0 :warning: | checkstyle | 1m 49s | hbase-server: The patch generated 2 new + 11 unchanged - 2 fixed = 13 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 3s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 5m 8s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 26m 14s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 89m 4s | |
| 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-3139/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | dupname asflicense javac refguide xml spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 6b6c194e3b52 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 / de012d7d1f |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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] ndimiduk commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r625367579
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
I think we need to keep the old config in hbase-defaults.xml and add the new config there as well.
```
modified hbase-common/src/main/resources/hbase-default.xml
@@ -41,6 +41,8 @@ possible configurations would overwhelm and obscure the important.
<configuration>
<!--Configs you will likely change are listed here at the top of the file.
-->
+ <property><name>foo</name><value>42</value><description>deprecated; see 'bar'</description></property>
+ <property><name>bar</name><value>42</value></property>
<property >
<name>hbase.tmp.dir</name>
<value>${java.io.tmpdir}/hbase-${user.name}</value>
---
| Welcome to JShell -- Version 11.0.10
| For an introduction type: /help intro
jshell> import org.apache.hadoop.conf.Configuration
jshell> import org.apache.hadoop.hbase.HBaseConfiguration
jshell> Configuration c = HBaseConfiguration.create()
c ==> Configuration: core-default.xml, core-site.xml, h ... efault.xml, hbase-site.xml
jshell> c.get("foo")
$4 ==> "42"
jshell> c.get("bar")
$5 ==> "42"
jshell> Configuration.addDeprecation("foo", "bar")
jshell> c.get("foo")
2021-05-03T14:03:00,038 INFO [main] Configuration.deprecation: foo is deprecated. Instead, use bar
$7 ==> "42"
jshell> c.get("bar")
$8 ==> "42"
jshell> c.set("foo", "99")
jshell> c.get("foo")
$10 ==> "99"
jshell> c.get("bar")
$11 ==> "99"
```
--
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] ndimiduk commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r625251956
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
The example of the use of conf deprecation from block cache that you mention on Jira is, in my opinion, an example that should not be followed.
> And I found after conf.addDeprecation(deprecatedConf, newConf), the conf.get(deprecatedConf) can not get value even we configured in conf file. So if the configuration is not modified before upgrade to HBase-2.5, then the configuration will be use default value.
Okay, this is bad.
##########
File path: hbase-common/src/main/resources/hbase-default.xml
##########
@@ -638,12 +638,6 @@ possible configurations would overwhelm and obscure the important.
<value>true</value>
<description>Whether to merge a region as part of normalization.</description>
</property>
- <property>
- <name>hbase.normalizer.min.region.count</name>
Review comment:
I don't understand the backward compatibility concern here, can you explain again for me? The variable `MERGE_MIN_REGION_COUNT_KEY` is not public, just like `MIN_REGION_COUNT_KEY`, so users won't use the variables in their code. The configuration key `"hbase.normalizer.merge.min.region.count"` is new with this patch, so if a user was looking up that value in an existing release, it should error or return no value. We don't need to preserve backward compatibility of a configuration key that is not defined.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-842081640
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 13s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | 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 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 8s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 8m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 2s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 36s | hbase-server generated 2 new + 18 unchanged - 2 fixed = 20 total (was 20) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 56s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 215m 4s | hbase-server in the patch passed. |
| | | 256m 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-3139/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 165b0912a7ae 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1c6994ad70 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/testReport/ |
| Max. process+thread count | 3015 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-847517956
Thanks @ndimiduk, let me do the backport.
--
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] ZhaoBQ edited a comment on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ edited a comment on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-833219457
Here is my test for Configuration.addDeprecation().
Suppose we define the following configuration：
hbase-site.xml: <property><name>deprecate.conf</name><value>99</value></property>
hbase-default.xml: <property><name>new.conf</name><value>42</value></property>
**Test 1, create a Configuration object first:**
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 42
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); //120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
`
**Test 2, call Configuration.addDeprecation() first:**
`
public class TestConfiguration {
@Deprecated
static final String DEPRECATED_KEY = "deprecate.conf";
static final String NEW_KEY = "new.conf";
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); //150
}
}
`
**Test 2 is what I expected, even if the deprecated configuration in the configuration file is not changed to a new configuration, it can still run correctly. But we should call Configuration.addDeprecation before create configuration object.**
**Test 3, hbase-default.xml is empty, and hbase-site.xml has:<property><name>deprecate.conf</name><value>99</value></property> <property><name>new.conf</name><value>42</value></property>**
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 42
System.out.println(conf.get(NEW_KEY)); // 42
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
**Then change the order of deprecate.conf and new.conf in hbase-site.xml: <property><name>new.conf</name><value>42</value></property> <property><name>deprecate.conf</name><value>99</value></property>**
`
public static void main(String[] args) {
Configuration.addDeprecation(DEPRECATED_KEY, NEW_KEY);
Configuration conf = HBaseConfiguration.create();
System.out.println(conf.get(DEPRECATED_KEY)); // 99
System.out.println(conf.get(NEW_KEY)); // 99
conf.set(DEPRECATED_KEY, "120");
System.out.println(conf.get(DEPRECATED_KEY)); // 120
System.out.println(conf.get(NEW_KEY)); // 120
conf.set(NEW_KEY, "150");
System.out.println(conf.get(DEPRECATED_KEY)); // 150
System.out.println(conf.get(NEW_KEY)); // 150
}
`
**The user should not configure the deprecated and new configuration at the same time, so Test3 and Test4 can be ignored.**
--
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] ZhaoBQ commented on a change in pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#discussion_r623633690
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -101,11 +106,21 @@ public void onConfigurationChange(Configuration conf) {
setConf(conf);
}
- private static int parseMinRegionCount(final Configuration conf) {
- final int parsedValue = conf.getInt(MIN_REGION_COUNT_KEY, DEFAULT_MIN_REGION_COUNT);
+ private static int parseMergeMinRegionCount(final Configuration conf) {
+ String parsedStringValue = conf.get(MERGE_MIN_REGION_COUNT_KEY);
Review comment:
The key point is how to recognize that the user has used the old configuration and log warnings once.
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-817762072
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | 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 9s | master passed |
| +1 :green_heart: | compile | 1m 26s | master passed |
| +1 :green_heart: | shadedjars | 8m 27s | 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 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | 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 | 8m 28s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 36s | hbase-server generated 2 new + 18 unchanged - 2 fixed = 20 total (was 20) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 54s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 212m 10s | hbase-server in the patch passed. |
| | | 247m 51s | |
| 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-3139/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8284d62aecc1 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f9e928e5a7 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/2/testReport/ |
| Max. process+thread count | 2810 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-830158635
:confetti_ball: **+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 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 3s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 8m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 57s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 34s | hbase-server generated 2 new + 18 unchanged - 2 fixed = 20 total (was 20) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 54s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 216m 51s | hbase-server in the patch passed. |
| | | 253m 0s | |
| 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-3139/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0054c3eccfbe 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6c65314cdf |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/testReport/ |
| Max. process+thread count | 2975 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/4/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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-841971335
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 3s | 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 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 12s | master passed |
| +1 :green_heart: | compile | 4m 12s | master passed |
| +1 :green_heart: | checkstyle | 1m 35s | master passed |
| +0 :ok: | refguide | 3m 44s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 2m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 58s | the patch passed |
| +1 :green_heart: | compile | 4m 5s | the patch passed |
| +1 :green_heart: | javac | 4m 5s | the patch passed |
| -0 :warning: | checkstyle | 1m 8s | hbase-server: The patch generated 2 new + 11 unchanged - 2 fixed = 13 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 3m 37s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 19m 51s | 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 23s | The patch does not generate ASF License warnings. |
| | | 63m 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-3139/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile refguide xml |
| uname | Linux d7a381d69d42 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1c6994ad70 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| refguide | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/5/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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] ZhaoBQ commented on pull request #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-833286386
At the beginning, I imitated other deprecated configurations and tried to use Configuration.addDeprecation(), I found that our use of Configuration.addDeprecation() was wrong. The key is that the order of calling is wrong, we should addDeprecation() before init Configuration object. We can see org.apache.hadoop.hdfs.HdfsConfiguration: https://github.com/apache/hadoop/blob/b93e448f9aa66689f1ce5059f6cdce8add130457/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/HdfsConfiguration.java#L34 .
So do we need to create another issue to correct this wrong usage first @ndimiduk ?
--
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 #3139: HBASE-25745 Deprecate/Rename config `hbase.normalizer.min.region.coun…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3139:
URL: https://github.com/apache/hbase/pull/3139#issuecomment-816595391
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 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 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 18s | master passed |
| +1 :green_heart: | compile | 1m 37s | master passed |
| +1 :green_heart: | shadedjars | 8m 41s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | 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 | 8m 55s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 42s | hbase-server generated 2 new + 18 unchanged - 2 fixed = 20 total (was 20) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 45s | hbase-common in the patch passed. |
| -1 :x: | unit | 157m 11s | hbase-server in the patch failed. |
| | | 197m 44s | |
| 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-3139/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3139 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 06c9f2f3a3f7 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 / 8e08952499 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/1/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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-3139/1/testReport/ |
| Max. process+thread count | 4947 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3139/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