Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HDDS-1363] ozone.metadata.dirs doesn't pick multiple dirs #691

Merged
merged 1 commit into from
Apr 12, 2019

Conversation

adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

ozone.metadata.dirs is a global fallback for per-component directory configurations, eg.ozone.om.db.dirs, ozone.scm.db.dirs, etc. All of these handle only a single location, but values with multiple comma-separated paths (eg. /data/dir1,/data/dir2) are treated in different ways:

  • rejected for the specific configs
  • used as a single value for the fallback config

The goal of this change is to reject comma-separated paths for ozone.metadata.dirs, too, by applying the same logic that is already used for the per-component configs.

In addition, the following minor fixes are included:

  1. Fix error message in ServerUtils#getDirectoryFromConfig, which referenced the component name (eg. SCM) instead of the config item name (eg. ozone.om.db.dirs) as "configuration setting".
  2. Move existing test cases for ServerUtils to the new ServerUtilsTest class from TestHddsServerUtils
  3. Eliminate duplicated logic in ScmUtils.getDBPath, reuse ServerUtils#getDirectoryFromConfig

https://issues.apache.org/jira/browse/HDDS-1363

How was this patch tested?

Unit tests (new and existing).

Tested manually using ozone docker-compose setup.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 24 Docker mode activated.
_ Prechecks _
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 1 The patch appears to include 2 new or modified test files.
_ trunk Compile Tests _
0 mvndep 63 Maven dependency ordering for branch
+1 mvninstall 1030 trunk passed
+1 compile 1069 trunk passed
+1 checkstyle 197 trunk passed
+1 mvnsite 238 trunk passed
+1 shadedclient 1132 branch has no errors when building and testing our client artifacts.
+1 findbugs 327 trunk passed
+1 javadoc 188 trunk passed
_ Patch Compile Tests _
0 mvndep 26 Maven dependency ordering for patch
+1 mvninstall 201 the patch passed
+1 compile 1003 the patch passed
+1 javac 1003 the patch passed
+1 checkstyle 192 the patch passed
+1 mvnsite 219 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 657 patch has no errors when building and testing our client artifacts.
+1 findbugs 387 the patch passed
+1 javadoc 198 the patch passed
_ Other Tests _
+1 unit 68 common in the patch passed.
-1 unit 77 container-service in the patch failed.
+1 unit 42 framework in the patch passed.
+1 unit 113 server-scm in the patch passed.
+1 unit 39 common in the patch passed.
+1 unit 41 ozone-recon in the patch passed.
+1 asflicense 36 The patch does not generate ASF License warnings.
7399
Reason Tests
Failed junit tests hadoop.ozone.container.common.TestDatanodeStateMachine
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/hadoop-multibranch/job/PR-691/1/artifact/out/Dockerfile
GITHUB PR #691
JIRA Issue HDDS-1363
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 5e915ebd1bbe 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2 17:16:02 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / eb03f7c
maven version: Apache Maven 3.3.9
Default Java 1.8.0_191
findbugs v3.1.0-RC1
unit https://builds.apache.org/job/hadoop-multibranch/job/PR-691/1/artifact/out/patch-unit-hadoop-hdds_container-service.txt
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-691/1/testReport/
Max. process+thread count 460 (vs. ulimit of 5500)
modules C: hadoop-hdds/common hadoop-hdds/container-service hadoop-hdds/framework hadoop-hdds/server-scm hadoop-ozone/common hadoop-ozone/ozone-recon U: .
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-691/1/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@adoroszlai
Copy link
Contributor Author

@nandakumar131 @hanishakoneru please review

@nandakumar131
Copy link
Contributor

Thanks @adoroszlai for the analysis and PR.

The change looks good.
Just one minor comment, can you rename ServerUtilsTest to TestServerUtils.
We follow the convention of prefixing "Test" to the class name.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
0 reexec 0 Docker mode activated.
-1 patch 7 #691 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.
Subsystem Report/Notes
GITHUB PR #691
JIRA Issue HDDS-1363
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-691/2/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
0 reexec 18 Docker mode activated.
_ Prechecks _
+1 @author 0 The patch does not contain any @author tags.
+1 test4tests 0 The patch appears to include 2 new or modified test files.
_ trunk Compile Tests _
0 mvndep 21 Maven dependency ordering for branch
+1 mvninstall 1089 trunk passed
+1 compile 988 trunk passed
+1 checkstyle 210 trunk passed
+1 mvnsite 273 trunk passed
+1 shadedclient 1252 branch has no errors when building and testing our client artifacts.
+1 findbugs 335 trunk passed
+1 javadoc 209 trunk passed
_ Patch Compile Tests _
0 mvndep 24 Maven dependency ordering for patch
+1 mvninstall 221 the patch passed
+1 compile 938 the patch passed
+1 javac 938 the patch passed
+1 checkstyle 213 the patch passed
+1 mvnsite 247 the patch passed
+1 whitespace 0 The patch has no whitespace issues.
+1 shadedclient 725 patch has no errors when building and testing our client artifacts.
+1 findbugs 385 the patch passed
+1 javadoc 208 the patch passed
_ Other Tests _
+1 unit 90 common in the patch passed.
+1 unit 70 container-service in the patch passed.
+1 unit 41 framework in the patch passed.
+1 unit 99 server-scm in the patch passed.
+1 unit 43 common in the patch passed.
+1 unit 48 ozone-recon in the patch passed.
+1 asflicense 42 The patch does not generate ASF License warnings.
7593
Subsystem Report/Notes
Docker Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/hadoop-multibranch/job/PR-691/3/artifact/out/Dockerfile
GITHUB PR #691
JIRA Issue HDDS-1363
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname Linux 794e2698972e 4.4.0-143-generic #169~14.04.2-Ubuntu SMP Wed Feb 13 15:00:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality personality/hadoop.sh
git revision trunk / dfb518b
maven version: Apache Maven 3.3.9
Default Java 1.8.0_191
findbugs v3.1.0-RC1
Test Results https://builds.apache.org/job/hadoop-multibranch/job/PR-691/3/testReport/
Max. process+thread count 429 (vs. ulimit of 5500)
modules C: hadoop-hdds/common hadoop-hdds/container-service hadoop-hdds/framework hadoop-hdds/server-scm hadoop-ozone/common hadoop-ozone/ozone-recon U: .
Console output https://builds.apache.org/job/hadoop-multibranch/job/PR-691/3/console
Powered by Apache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@nandakumar131 nandakumar131 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@nandakumar131 nandakumar131 merged commit 3b08ac4 into apache:trunk Apr 12, 2019
@adoroszlai adoroszlai deleted the HDDS-1363 branch April 12, 2019 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants