HDDS-6695. Enable SCM Ratis by default for new clusters only #3499
Conversation
|
Thank you for the contribution @swamirishi, please include a link to the related issue on ASF Jira and fill in the required details in the description of the PR. This helps us track and understand the context for your changes. |
JyotinderSingh
changed the title
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/DefaultConfigManager.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java
Outdated
Show resolved
Hide resolved
errose28
left a comment
errose28
left a comment
There was a problem hiding this comment.
Thanks for working on this @swamirishi. Some comments in addition to those of @JyotinderSingh. Also, please document any manual testing performed and cite the acceptance tests(s) that are relevant for the change in the PR description for reference to future readers.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Looks like the Optional is only empty for tests right? Can we make storageConfig a required parameter and have the tests pass new SCMStorageConfig(conf)? This makes it clear to the production code that they must specify a storage config for correct behavior.
There was a problem hiding this comment.
I think we should make these messages more explicit. For the exception, let's tell the user why the config is invalid (they were previously using Ratis and now they are trying not to). For the warning, we should indicate that the config was not specified, so we are using this value based on the previous cluster state.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Instead of using reflection to clear the test state, can we just put each section that needs a clean slate in its own test? Another option could be an @VisibleForTesting clearDefaults method in the DefaultConfigManager.
errose28
left a comment
errose28
left a comment
There was a problem hiding this comment.
Thanks for the updates @swamirishi. Just some minor comments remaining.
...op-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java
Outdated
Show resolved
Hide resolved
...zone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManager.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHANodeDetails.java
Outdated
Show resolved
Hide resolved
|
Thanks for working on this @swamirishi. LGTM pending green CI. |
* master: (34 commits) HDDS-6868 Add S3Auth information to thread local (apache#3527) HDDS-6877. Keep replication port unchanged when restarting datanode in MiniOzoneCluster (apache#3510) HDDS-6907. OFS should create buckets with FILE_SYSTEM_OPTIMIZED layout. (apache#3528) HDDS-6875. Migrate parameterized tests in hdds-common to JUnit5 (apache#3513) HDDS-6924. OBJECT_STORE isn't flat namespaced (apache#3533) HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data. (apache#3522) HDDS-6695. Enable SCM Ratis by default for new clusters only (apache#3499) HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code (apache#3319) HDDS-6882. Correct exit code for invalid arguments passed to command-line tools. (apache#3517) HDDS-6890. EC: Fix potential wrong replica read with over-replicated container. (apache#3523) HDDS-6902. Duplicate mockito-core entries in pom.xml (apache#3525) HDDS-6752. Migrate tests with rules in hdds-server-scm to JUnit5 (apache#3442) HDDS-6806. EC: Implement the EC Reconstruction coordinator. (apache#3504) HDDS-6829. Limit the no of inflight replication tasks in SCM. (apache#3482) HDDS-6898. [SCM HA finalization] Modify acceptance test configuration to speed up test finalization (apache#3521) HDDS-6577. Configurations to reserve HDDS volume space. (apache#3484) HDDS-6870 Clean up isTenantAdmin to use UGI (apache#3503) HDDS-6872. TestAuthorizationV4QueryParser should pass offline (apache#3506) HDDS-6840. Add MetaData volume information to the SCM and OM - UI (apache#3488) HDDS-6697. EC: ReplicationManager - create class to detect EC container health issues (apache#3512) ...
HDDS-6695. Enable SCM Ratis by default for new clusters only (apache#3499) (cherry picked from commit 50dbf08) Change-Id: I0005cb86b4b7466174b2a875a168fdebf04be065
3 participants
What changes were proposed in this pull request?
The default value of the config "ozone.scm.ratis.enable" has been changed false to true.
The value of the config passed in is validated based on the current state of the SCM i.e. if the state is initialized then the property value of scmHA in the version file is picked and verified with config passed in. If they don't match we don't let the SCM come up. This is done as we are not currently supporting Non Ratis SCM -> Ratis SCM & Ratis SCM -> Non Ratis SCM.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6695
How was this patch tested?
Unit Tests & Acceptance Test
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)