Skip to content

HDDS-3556 Refactor conf in SCMRatisServer to Java-based conf. #907

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.

Sign up for GitHub

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

Merged
ChenSammi merged 3 commits into from
May 20, 2020
Merged

HDDS-3556 Refactor conf in SCMRatisServer to Java-based conf. #907

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a777392
HDDS-3556 Refactor configuration in SCMRatisServer to Java-based conf…
timmylicheng May 9, 2020
ba67c76
Make SCM constructor private and have SCMRatisServer constructed in s…
timmylicheng May 13, 2020
affe58c
Update java doc for createSCM.
timmylicheng May 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ public final class ScmConfigKeys {
// SCM Ratis Leader Election configurations
public static final String
OZONE_SCM_LEADER_ELECTION_MINIMUM_TIMEOUT_DURATION_KEY =
"ozone.scm.leader.election.minimum.timeout.duration";
"ozone.scm.ratis.leader.election.minimum.timeout.duration";
public static final TimeDuration
OZONE_SCM_LEADER_ELECTION_MINIMUM_TIMEOUT_DURATION_DEFAULT =
TimeDuration.valueOf(1, TimeUnit.SECONDS);
Expand Down
4 changes: 2 additions & 2 deletions hadoop-hdds/common/src/main/resources/ozone-default.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@
<name>ozone.scm.ratis.server.request.timeout</name>
<value>3s</value>
<tag>OZONE, SCM, HA, RATIS</tag>
<description>The timeout duration for SCM's ratis server request .</description>
<description>The timeout duration for SCM's ratis server request.</description>
</property>

<property>
Expand All @@ -2054,7 +2054,7 @@
</property>

<property>
<name>ozone.scm.leader.election.minimum.timeout.duration</name>
<name>ozone.scm.ratis.leader.election.minimum.timeout.duration</name>
<value>1s</value>
<tag>OZONE, SCM, HA, RATIS</tag>
<description>The minimum timeout duration for SCM ratis leader election.
Expand Down
5 changes: 5 additions & 0 deletions hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,10 @@
*/
TimeUnit timeUnit() default TimeUnit.MILLISECONDS;

/**
* If type == SIZE the unit should be defined with this attribute.
*/
StorageUnit sizeUnit() default StorageUnit.BYTES;

ConfigTag[] tags();
}
3 changes: 2 additions & 1 deletion hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigTag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@ public enum ConfigTag {
STANDALONE,
S3GATEWAY,
DATANODE,
RECON
RECON,
HA
}
4 changes: 4 additions & 0 deletions ...op-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public static <T> void injectConfigurationToObject(ConfigurationSource from,
forcedFieldSet(field, configuration,
from.getTimeDuration(key, "0s", configAnnotation.timeUnit()));
break;
case SIZE:
forcedFieldSet(field, configuration,
from.getStorageSize(key, "0B", configAnnotation.sizeUnit()));
break;
default:
throw new ConfigurationException(
"Unsupported ConfigType " + type + " on " + fieldLocation);
Expand Down
9 changes: 6 additions & 3 deletions ...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ public StorageContainerManager(OzoneConfiguration conf,
loginAsSCMUser(conf);
}

// Initialize SCM Ratis Server
if (SCMHAUtils.isSCMHAEnabled(conf)) {
this.scmRatisSnapshotInfo = new SCMRatisSnapshotInfo(
scmStorageConfig.getCurrentDir());
Expand Down Expand Up @@ -1141,9 +1142,11 @@ private void initializeRatisServer() throws IOException {
if (scmRatisServer == null) {
SCMNodeDetails scmNodeDetails = SCMNodeDetails
.initStandAlone(configuration);
//TODO enable Ratis ring
scmRatisServer = SCMRatisServer.newSCMRatisServer(configuration, this,
scmNodeDetails, Collections.EMPTY_LIST);
//TODO enable Ratis group
scmRatisServer = SCMRatisServer.newSCMRatisServer(configuration
.getObject(SCMRatisServer.SCMRatisServerConfiguration.class),
this, scmNodeDetails, Collections.EMPTY_LIST,

@xiaoyuyao xiaoyuyao May 12, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Passing half created object (this) to SCMRatisServer constructor could cause subtle bugs that are tricky to get right. We should avoid it if possible.

@timmylicheng timmylicheng May 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have made updates to have SCM use static method for createSCM and make constructor private. SCMRatisServer will be constructed after SCM is constructed.

SCMRatisServer.getSCMRatisDirectory(configuration));
if (scmRatisServer != null) {
LOG.info("SCM Ratis server initialized at port {}",
scmRatisServer.getServerPort());
Expand Down
Loading