You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/05 17:42:44 UTC
[GitHub] [ozone] aswinshakil opened a new pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
aswinshakil opened a new pull request #2506:
URL: https://github.com/apache/ozone/pull/2506
## What changes were proposed in this pull request?
To Merge SCM HA Configuration into ScmConfigKeys and remove unused configurations.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5054
## How was this patch tested?
The patch was tested using Unit tests, Integration tests and Acceptance tests using GitHub CI.
https://github.com/aswinshakil/ozone/actions/runs/1099362111
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel commented on pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #2506:
URL: https://github.com/apache/ozone/pull/2506#issuecomment-900695020
Merging this since TestBlockDeletion test is flaky - https://issues.apache.org/jira/browse/HDDS-5605
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel commented on pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #2506:
URL: https://github.com/apache/ozone/pull/2506#issuecomment-900695020
Merging this since TestBlockDeletion test is flaky - https://issues.apache.org/jira/browse/HDDS-5605
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel merged pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
vivekratnavel merged pull request #2506:
URL: https://github.com/apache/ozone/pull/2506
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel commented on a change in pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on a change in pull request #2506:
URL: https://github.com/apache/ozone/pull/2506#discussion_r684383897
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
##########
@@ -475,6 +475,71 @@
OZONE_SCM_EVENT_PREFIX + "ContainerReport.thread.pool.size";
public static final int OZONE_SCM_EVENT_THREAD_POOL_SIZE_DEFAULT = 10;
+ public static final String RATIS_RPC_TYPE =
Review comment:
Can we add appropriate prefixes to identify the keys easier?
For instance, this can be renamed to `OZONE_SCM_HA_RATIS_RPC_TYPE`. All the other configs can also be prefixed with "OZONE_SCM_HA"
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMHAConfiguration.java
##########
@@ -60,26 +60,17 @@ public void setup() {
@Test
public void testSetStorageDir() {
- SCMHAConfiguration scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- scmhaConfiguration.setRatisStorageDir("scm-ratis");
- conf.setFromObject(scmhaConfiguration);
-
- scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- Assert.assertEquals("scm-ratis", scmhaConfiguration.getRatisStorageDir());
+ conf.set(ScmConfigKeys.RATIS_STORAGE_DIR, "scm-ratis");
Review comment:
We can remove these tests as they aren't meaningful.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] bshashikant commented on pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2506:
URL: https://github.com/apache/ozone/pull/2506#issuecomment-899675536
> Thanks for working on this Aswin!
>
> A question for @bshashikant / @GlenGeng. Was the objective to move all the configs in SCM HA config to the old method of declaring configs (through ozone-default)? Or do we want to create a single SCMConfiguration class of type _ConfigGroup_ that has all the configs from both the old files?
I am ok with either of the approaches.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] vivekratnavel merged pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
vivekratnavel merged pull request #2506:
URL: https://github.com/apache/ozone/pull/2506
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #2506: HDDS-5054. Merge SCM HA configs to ScmConfigKeys.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #2506:
URL: https://github.com/apache/ozone/pull/2506#discussion_r684429888
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMHAConfiguration.java
##########
@@ -60,26 +60,17 @@ public void setup() {
@Test
public void testSetStorageDir() {
- SCMHAConfiguration scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- scmhaConfiguration.setRatisStorageDir("scm-ratis");
- conf.setFromObject(scmhaConfiguration);
-
- scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- Assert.assertEquals("scm-ratis", scmhaConfiguration.getRatisStorageDir());
+ conf.set(ScmConfigKeys.RATIS_STORAGE_DIR, "scm-ratis");
+ Assert.assertEquals("scm-ratis", conf.get(ScmConfigKeys.RATIS_STORAGE_DIR));
Review comment:
This test seems unnecessary now, since we removed the _SCMHAConfiguration_ class.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMHAConfiguration.java
##########
@@ -60,26 +60,17 @@ public void setup() {
@Test
public void testSetStorageDir() {
- SCMHAConfiguration scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- scmhaConfiguration.setRatisStorageDir("scm-ratis");
- conf.setFromObject(scmhaConfiguration);
-
- scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- Assert.assertEquals("scm-ratis", scmhaConfiguration.getRatisStorageDir());
+ conf.set(ScmConfigKeys.RATIS_STORAGE_DIR, "scm-ratis");
+ Assert.assertEquals("scm-ratis", conf.get(ScmConfigKeys.RATIS_STORAGE_DIR));
}
@Test
public void testRaftLogPurgeEnabled() {
- SCMHAConfiguration scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- scmhaConfiguration.setRaftLogPurgeEnabled(true);
- conf.setFromObject(scmhaConfiguration);
-
- scmhaConfiguration = conf.getObject(
- SCMHAConfiguration.class);
- Assert.assertEquals(true, scmhaConfiguration.getRaftLogPurgeEnabled());
+ conf.setBoolean(ScmConfigKeys.RAFT_LOG_PURGE_ENABLED, true);
+
+ Assert.assertEquals(true, conf.getBoolean(
Review comment:
Same as above.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org