[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers

[GitBox <gi...@apache.org>]

amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r546982531



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java
##########
@@ -173,9 +261,145 @@ public static SCMNodeDetails initStandAlone(
     SCMNodeDetails scmNodeDetails = new SCMNodeDetails.Builder()
         .setRatisPort(ratisPort)
         .setRpcAddress(rpcAddress)
-        .setSCMNodeId(localSCMNodeId)
-        .setSCMServiceId(localSCMServiceId)
+        .setDatanodeProtocolServerAddress(
+            HddsServerUtil.getScmDataNodeBindAddress(conf))
+        .setDatanodeAddressKey(OZONE_SCM_DATANODE_ADDRESS_KEY)
+        .setBlockProtocolServerAddress(
+            HddsServerUtil.getScmBlockClientBindAddress(conf))
+        .setBlockProtocolServerAddressKey(
+            ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY)
+        .setClientProtocolServerAddress(
+            HddsServerUtil.getScmClientBindAddress(conf))
+        .setClientProtocolServerAddressKey(
+            ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY)
         .build();
     return scmNodeDetails;
   }
+
+  public static SCMNodeDetails loadSCMHAConfig(
+      OzoneConfiguration conf) throws IOException {
+    SCMNodeDetails.Builder builder = new Builder();
+
+    String localScmServiceId = null;
+    String localScmNodeId = null;
+
+    Collection<String> scmServiceIds;
+
+    localScmServiceId = conf.getTrimmed(
+        ScmConfigKeys.OZONE_SCM_INTERNAL_SERVICE_ID);
+
+    LOG.info("ServiceID for StorageContainerManager is {}", localScmServiceId);
+
+    if (localScmServiceId == null) {
+      // There is no internal scm service id is being set, fall back to ozone
+      // .scm.service.ids.
+      LOG.info("{} is not defined, falling back to {} to find serviceID for "
+              + "StorageContainerManager if it is HA enabled cluster",
+          OZONE_SCM_INTERNAL_SERVICE_ID, OZONE_SCM_SERVICE_IDS_KEY);
+      scmServiceIds = conf.getTrimmedStringCollection(
+          OZONE_SCM_SERVICE_IDS_KEY);
+    } else {
+      LOG.info("ServiceID for StorageContainerManager is {}",
+          localScmServiceId);
+      builder.setSCMServiceId(localScmServiceId);
+      scmServiceIds = Collections.singleton(localScmServiceId);
+    }
+
+    localScmNodeId = conf.get(ScmConfigKeys.OZONE_SCM_NODE_ID_KEY);
+    boolean isScmHAConfigSet = false;
+    for (String serviceId : scmServiceIds) {
+      Collection<String> scmNodeIds = ScmUtils.getSCMNodeIds(conf, serviceId);
+
+      if (scmNodeIds.size() == 0) {
+        throw new IllegalArgumentException(
+            String.format("Configuration does not have any value set for %s " +
+            "for the service %s. List of OM Node ID's should be specified " +
+            "for an OM service", ScmConfigKeys.OZONE_SCM_NODES_KEY, serviceId));
+      }
+      // TODO: load Ratis peers configuration
+      isScmHAConfigSet = true;
+    }
+
+    if (!isScmHAConfigSet) {
+      // If HA config is not set properly, fall back to default configuration
+      return loadDefaultConfig(conf);
+    }
+
+    builder
+        .setBlockProtocolServerAddress(getScmBlockProtocolServerAddress(

Review comment:
       I haven't written test case for this. Want to wait for some feedback before I continue improving this PR. 




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org