bharatviswa504 commented on a change in pull request #1916:
URL: https://github.com/apache/ozone/pull/1916#discussion_r576559591
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -697,32 +698,36 @@ public static boolean scmInit(OzoneConfiguration conf,
if (clusterId != null && !clusterId.isEmpty()) {
scmStorageConfig.setClusterId(clusterId);
}
- scmStorageConfig.initialize();
- LOG.info("SCM initialization succeeded. Current cluster id for sd={}"
- + ";cid={};layoutVersion={}", scmStorageConfig.getStorageDir(),
- scmStorageConfig.getClusterID(),
- scmStorageConfig.getLayoutVersion());
- //TODO: this check need not be done if ratis is not used.
- // Initialize the raft group Id withe cluster Id.
+ // TODO: set the scm node info in the version file during upgrade
+ // from non-HA SCM to SCM HA set up.
if (SCMHAUtils.isSCMHAEnabled(conf)) {
RaftServer server = SCMRatisServerImpl
.newRaftServer(clusterId, scmStorageConfig.getScmId(), haDetails,
conf).build();
+ // ensure the ratis group exists
server.start();
server.close();
+ scmStorageConfig
Review comment:
Just a question can we also set scmNodeInfo in non-HA also?
So that all fresh installs of non-HA also will have this info.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -697,32 +698,36 @@ public static boolean scmInit(OzoneConfiguration conf,
if (clusterId != null && !clusterId.isEmpty()) {
scmStorageConfig.setClusterId(clusterId);
}
- scmStorageConfig.initialize();
- LOG.info("SCM initialization succeeded. Current cluster id for sd={}"
- + ";cid={};layoutVersion={}", scmStorageConfig.getStorageDir(),
- scmStorageConfig.getClusterID(),
- scmStorageConfig.getLayoutVersion());
- //TODO: this check need not be done if ratis is not used.
- // Initialize the raft group Id withe cluster Id.
+ // TODO: set the scm node info in the version file during upgrade
+ // from non-HA SCM to SCM HA set up.
if (SCMHAUtils.isSCMHAEnabled(conf)) {
RaftServer server = SCMRatisServerImpl
.newRaftServer(clusterId, scmStorageConfig.getScmId(), haDetails,
conf).build();
+ // ensure the ratis group exists
server.start();
server.close();
+ scmStorageConfig
+ .setScmNodeInfo(haDetails.getLocalNodeDetails().getHostName());
}
+ scmStorageConfig.initialize();
+ LOG.info("SCM initialization succeeded. Current cluster id for sd={}"
+ + ";cid={};layoutVersion={}", scmStorageConfig.getStorageDir(),
+ scmStorageConfig.getClusterID(),
Review comment:
Minor: Can we also add scmId also?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStorageConfig.java
##########
@@ -76,6 +98,11 @@ protected Properties getNodeProperties() {
}
Properties scmProperties = new Properties();
scmProperties.setProperty(SCM_ID, scmId);
+ String scmNodeInfo = getScmNodeInfo();
+ if (scmNodeInfo != null) {
+ // FOR NON-HA setup, SCM_NODES can be null
Review comment:
If we add nodeInfo for non-HA, this can be avoided.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
