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 2020/12/17 22:20:53 UTC
[GitHub] [ozone] amaliujia opened a new pull request #1722: HDDS-4600. Use OM style Configuration to initialize SCM HA
amaliujia opened a new pull request #1722:
URL: https://github.com/apache/ozone/pull/1722
## What changes were proposed in this pull request?
Start to use SCMNodeDetails as the entry to retrieve address configs for servers (except Ratis server)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4600
## How was this patch tested?
UT
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-773786312
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-775469738
I have rebased this PR. Let's see now how does CI check goes.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r570718484
##########
File path: hadoop-hdds/server-scm/pom.xml
##########
@@ -48,6 +48,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-hdds-client</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-ozone-common</artifactId>
+ <version>1.1.0-SNAPSHOT</version>
Review comment:
It is wired to me that my local checkstyle contains when there is no `version` in here.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. Use OM style Configuration to initialize SCM HA
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r546210998
##########
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);
Review comment:
Currently SCMNodeDetails will always be built from default configuration to pass existing UT.
The HA configuration is left to be used by MiniOzoneHACluster.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. Use OM style Configuration to initialize SCM HA
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-747815008
R: @nandakumar131
----------------------------------------------------------------
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
[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
Posted by 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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-756524049
cc @ChenSammi
----------------------------------------------------------------
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
[GitHub] [ozone] bshashikant commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r570876906
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java
##########
@@ -17,59 +17,61 @@
package org.apache.hadoop.hdds.scm.ha;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.ozone.ha.NodeDetails;
+import org.apache.ratis.protocol.RaftGroup;
+import org.apache.ratis.protocol.RaftPeerId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
-import java.net.InetAddress;
import java.net.InetSocketAddress;
-import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_INTERNAL_SERVICE_ID;
-import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SERVICE_IDS_KEY;
-
/**
* Construct SCM node details.
*/
-public final class SCMNodeDetails {
- private String scmServiceId;
- private String scmNodeId;
- private InetSocketAddress rpcAddress;
- private int rpcPort;
- private int ratisPort;
- private String httpAddress;
- private String httpsAddress;
+public final class SCMNodeDetails extends NodeDetails {
+ private InetSocketAddress blockProtocolServerAddress;
+ private String blockProtocolServerAddressKey;
+ private InetSocketAddress clientProtocolServerAddress;
+ private String clientProtocolServerAddressKey;
+ private InetSocketAddress datanodeProtocolServerAddress;
+ private String datanodeAddressKey;
public static final Logger LOG =
LoggerFactory.getLogger(SCMNodeDetails.class);
/**
* Constructs SCMNodeDetails object.
*/
+ @SuppressWarnings("checkstyle:ParameterNumber")
private SCMNodeDetails(String serviceId, String nodeId,
- InetSocketAddress rpcAddr, int rpcPort, int ratisPort,
- String httpAddress, String httpsAddress) {
- this.scmServiceId = serviceId;
- this.scmNodeId = nodeId;
- this.rpcAddress = rpcAddr;
- this.rpcPort = rpcPort;
- this.ratisPort = ratisPort;
- this.httpAddress = httpAddress;
- this.httpsAddress = httpsAddress;
+ InetSocketAddress rpcAddr, int ratisPort, String httpAddress,
+ String httpsAddress, InetSocketAddress blockProtocolServerAddress,
+ InetSocketAddress clientProtocolServerAddress,
+ InetSocketAddress datanodeProtocolServerAddress,
+ RaftGroup group, RaftPeerId selfPeerId, String datanodeAddressKey,
+ String blockProtocolServerAddressKey,
+ String clientProtocolServerAddressAddressKey) {
+ super(serviceId, nodeId, rpcAddr, ratisPort,
+ httpAddress, httpsAddress);
+ this.blockProtocolServerAddress = blockProtocolServerAddress;
+ this.clientProtocolServerAddress = clientProtocolServerAddress;
+ this.datanodeProtocolServerAddress = datanodeProtocolServerAddress;
+ this.datanodeAddressKey = datanodeAddressKey;
+ this.blockProtocolServerAddressKey = blockProtocolServerAddressKey;
+ this.clientProtocolServerAddressKey = clientProtocolServerAddressAddressKey;
}
@Override
public String toString() {
+ // TODO: add new fields to toString
Review comment:
Can we move this to NodeDetails class itself?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ScmUtils.java
##########
@@ -60,4 +71,111 @@ public static File createSCMDir(String dirPath) {
}
return dirFile;
}
+
+ public static Collection<String> getSCMNodeIds(ConfigurationSource conf,
+ String scmServiceId) {
+ String key = addSuffix(ScmConfigKeys.OZONE_SCM_NODES_KEY, scmServiceId);
+ return conf.getTrimmedStringCollection(key);
+ }
+
+ public static InetSocketAddress getScmBlockProtocolServerAddress(
+ OzoneConfiguration conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_BIND_HOST_KEY,
+ localScmServiceId, nodeId);
+ final Optional<String> host = getHostNameFromConfigKeys(conf, bindHostKey);
+
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY,
+ localScmServiceId, nodeId);
+ final OptionalInt port = getPortNumberFromConfigKeys(conf, addressKey);
+
+ return NetUtils.createSocketAddr(
+ host.orElse(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_BIND_HOST_DEFAULT) + ":" +
+ port.orElse(ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT));
+ }
+
+ public static String getScmBlockProtocolServerAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ public static InetSocketAddress getClientProtocolServerAddress(
+ OzoneConfiguration conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_KEY,
+ localScmServiceId, nodeId);
+
+ final String host = getHostNameFromConfigKeys(conf, bindHostKey)
+ .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_DEFAULT);
+
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY,
+ localScmServiceId, nodeId);
+
+ final int port = getPortNumberFromConfigKeys(conf, addressKey)
+ .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT);
+
+ return NetUtils.createSocketAddr(host + ":" + port);
+ }
+
+ public static String getClientProtocolServerAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ public static InetSocketAddress getScmDataNodeBindAddress(
+ ConfigurationSource conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_BIND_HOST_KEY,
+ localScmServiceId, nodeId
+ );
+ final Optional<String> host = getHostNameFromConfigKeys(conf, bindHostKey);
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY,
+ localScmServiceId, nodeId
+ );
+ final OptionalInt port = getPortNumberFromConfigKeys(conf, addressKey);
+
+ return NetUtils.createSocketAddr(
+ host.orElse(ScmConfigKeys.OZONE_SCM_DATANODE_BIND_HOST_DEFAULT) + ":" +
+ port.orElse(ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT));
+ }
+
+ public static String getScmDataNodeBindAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ private static String addSuffix(String key, String suffix) {
+ if (suffix == null || suffix.isEmpty()) {
+ return key;
+ }
Review comment:
These functions can be moved to a utility and used by both OM and SCM as required. Nothing seems specific ti SCM here.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-775584290
@bshashikant Checks passed.
----------------------------------------------------------------
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
[GitHub] [ozone] bshashikant commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r572612741
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMNodeDetails.java
##########
@@ -31,43 +30,36 @@
/**
* This class stores OM node details.
*/
-public final class OMNodeDetails {
- private String omServiceId;
- private String omNodeId;
- private InetSocketAddress rpcAddress;
+public final class OMNodeDetails extends NodeDetails {
private int rpcPort;
- private int ratisPort;
- private String httpAddress;
- private String httpsAddress;
/**
* Constructs OMNodeDetails object.
*/
private OMNodeDetails(String serviceId, String nodeId,
InetSocketAddress rpcAddr, int rpcPort, int ratisPort,
String httpAddress, String httpsAddress) {
- this.omServiceId = serviceId;
- this.omNodeId = nodeId;
- this.rpcAddress = rpcAddr;
+ super(serviceId, nodeId, rpcAddr, ratisPort, httpAddress, httpsAddress);
this.rpcPort = rpcPort;
Review comment:
is there any specific reason to maintain rpc port specifically in OMNodeDetails? I guess, it can be common for both SCM and OM.
----------------------------------------------------------------
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
[GitHub] [ozone] bshashikant commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r572612741
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMNodeDetails.java
##########
@@ -31,43 +30,36 @@
/**
* This class stores OM node details.
*/
-public final class OMNodeDetails {
- private String omServiceId;
- private String omNodeId;
- private InetSocketAddress rpcAddress;
+public final class OMNodeDetails extends NodeDetails {
private int rpcPort;
- private int ratisPort;
- private String httpAddress;
- private String httpsAddress;
/**
* Constructs OMNodeDetails object.
*/
private OMNodeDetails(String serviceId, String nodeId,
InetSocketAddress rpcAddr, int rpcPort, int ratisPort,
String httpAddress, String httpsAddress) {
- this.omServiceId = serviceId;
- this.omNodeId = nodeId;
- this.rpcAddress = rpcAddr;
+ super(serviceId, nodeId, rpcAddr, ratisPort, httpAddress, httpsAddress);
this.rpcPort = rpcPort;
Review comment:
is there any specific reason to maintain rpc port specifically in OMNodeDetails? I guess, it can be common for both SCM and OM and we can maintain it in NodeDetails itself.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-773830649
Regarding the findsbug, it is wired to me
```
[INFO] Total bugs: 1
[ERROR] org.apache.hadoop.hdds.scm.pipeline.choose.algorithms.PipelineChoosePolicyFactory.createPipelineChoosePolicyFromClass(Class) may return null, but is declared @Nonnull [org.apache.hadoop.hdds.scm.pipeline.choose.algorithms.PipelineChoosePolicyFactory, org.apache.hadoop.hdds.scm.pipeline.choose.algorithms.PipelineChoosePolicyFactory] Returned at PipelineChoosePolicyFactory.java:[line 86]Known null at PipelineChoosePolicyFactory.java:[line 86] NP_NONNULL_RETURN_VIOLATION
[INFO]
```
I guess I didn't touch this code at all.
----------------------------------------------------------------
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
[GitHub] [ozone] bshashikant commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-775732532
Thanks @amaliujia for the contribution.
----------------------------------------------------------------
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
[GitHub] [ozone] bshashikant merged pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
bshashikant merged pull request #1722:
URL: https://github.com/apache/ozone/pull/1722
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r571378372
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ScmUtils.java
##########
@@ -60,4 +71,111 @@ public static File createSCMDir(String dirPath) {
}
return dirFile;
}
+
+ public static Collection<String> getSCMNodeIds(ConfigurationSource conf,
+ String scmServiceId) {
+ String key = addSuffix(ScmConfigKeys.OZONE_SCM_NODES_KEY, scmServiceId);
+ return conf.getTrimmedStringCollection(key);
+ }
+
+ public static InetSocketAddress getScmBlockProtocolServerAddress(
+ OzoneConfiguration conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_BIND_HOST_KEY,
+ localScmServiceId, nodeId);
+ final Optional<String> host = getHostNameFromConfigKeys(conf, bindHostKey);
+
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY,
+ localScmServiceId, nodeId);
+ final OptionalInt port = getPortNumberFromConfigKeys(conf, addressKey);
+
+ return NetUtils.createSocketAddr(
+ host.orElse(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_BIND_HOST_DEFAULT) + ":" +
+ port.orElse(ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT));
+ }
+
+ public static String getScmBlockProtocolServerAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_BLOCK_CLIENT_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ public static InetSocketAddress getClientProtocolServerAddress(
+ OzoneConfiguration conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_KEY,
+ localScmServiceId, nodeId);
+
+ final String host = getHostNameFromConfigKeys(conf, bindHostKey)
+ .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_BIND_HOST_DEFAULT);
+
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY,
+ localScmServiceId, nodeId);
+
+ final int port = getPortNumberFromConfigKeys(conf, addressKey)
+ .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT);
+
+ return NetUtils.createSocketAddr(host + ":" + port);
+ }
+
+ public static String getClientProtocolServerAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ public static InetSocketAddress getScmDataNodeBindAddress(
+ ConfigurationSource conf, String localScmServiceId, String nodeId) {
+ String bindHostKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_BIND_HOST_KEY,
+ localScmServiceId, nodeId
+ );
+ final Optional<String> host = getHostNameFromConfigKeys(conf, bindHostKey);
+ String addressKey = ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY,
+ localScmServiceId, nodeId
+ );
+ final OptionalInt port = getPortNumberFromConfigKeys(conf, addressKey);
+
+ return NetUtils.createSocketAddr(
+ host.orElse(ScmConfigKeys.OZONE_SCM_DATANODE_BIND_HOST_DEFAULT) + ":" +
+ port.orElse(ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT));
+ }
+
+ public static String getScmDataNodeBindAddressKey(
+ String serviceId, String nodeId) {
+ return ScmUtils.addKeySuffixes(
+ ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY,
+ serviceId, nodeId);
+ }
+
+ private static String addSuffix(String key, String suffix) {
+ if (suffix == null || suffix.isEmpty()) {
+ return key;
+ }
Review comment:
I created a common utility and moved some functions there.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r570718484
##########
File path: hadoop-hdds/server-scm/pom.xml
##########
@@ -48,6 +48,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-hdds-client</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.hadoop</groupId>
+ <artifactId>hadoop-ozone-common</artifactId>
+ <version>1.1.0-SNAPSHOT</version>
Review comment:
It is wired to me that my local checkstyle contains when there is no `version` in here.
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-775645796
also R: @mukul1987
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r572615441
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ha/OMNodeDetails.java
##########
@@ -31,43 +30,36 @@
/**
* This class stores OM node details.
*/
-public final class OMNodeDetails {
- private String omServiceId;
- private String omNodeId;
- private InetSocketAddress rpcAddress;
+public final class OMNodeDetails extends NodeDetails {
private int rpcPort;
- private int ratisPort;
- private String httpAddress;
- private String httpsAddress;
/**
* Constructs OMNodeDetails object.
*/
private OMNodeDetails(String serviceId, String nodeId,
InetSocketAddress rpcAddr, int rpcPort, int ratisPort,
String httpAddress, String httpsAddress) {
- this.omServiceId = serviceId;
- this.omNodeId = nodeId;
- this.rpcAddress = rpcAddr;
+ super(serviceId, nodeId, rpcAddr, ratisPort, httpAddress, httpsAddress);
this.rpcPort = rpcPort;
Review comment:
I might not have a correct understanding:
SCM does not need a single RPC port, it has three: datanode, client and block server.
That why this diverges: OM maintains its rpc port while SCM maintains three ports.
If this is not correct, I can make a change in a subsequence 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
[GitHub] [ozone] amaliujia commented on pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#issuecomment-773786312
R @bshashikant
----------------------------------------------------------------
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
[GitHub] [ozone] amaliujia commented on a change in pull request #1722: HDDS-4600. SCM HA Config Phase 1: Use SCMNodeDetails as the entry to initialize SCM except for Ratis servers
Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1722:
URL: https://github.com/apache/ozone/pull/1722#discussion_r571378346
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java
##########
@@ -17,59 +17,61 @@
package org.apache.hadoop.hdds.scm.ha;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.ozone.ha.NodeDetails;
+import org.apache.ratis.protocol.RaftGroup;
+import org.apache.ratis.protocol.RaftPeerId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
-import java.net.InetAddress;
import java.net.InetSocketAddress;
-import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_INTERNAL_SERVICE_ID;
-import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_SERVICE_IDS_KEY;
-
/**
* Construct SCM node details.
*/
-public final class SCMNodeDetails {
- private String scmServiceId;
- private String scmNodeId;
- private InetSocketAddress rpcAddress;
- private int rpcPort;
- private int ratisPort;
- private String httpAddress;
- private String httpsAddress;
+public final class SCMNodeDetails extends NodeDetails {
+ private InetSocketAddress blockProtocolServerAddress;
+ private String blockProtocolServerAddressKey;
+ private InetSocketAddress clientProtocolServerAddress;
+ private String clientProtocolServerAddressKey;
+ private InetSocketAddress datanodeProtocolServerAddress;
+ private String datanodeAddressKey;
public static final Logger LOG =
LoggerFactory.getLogger(SCMNodeDetails.class);
/**
* Constructs SCMNodeDetails object.
*/
+ @SuppressWarnings("checkstyle:ParameterNumber")
private SCMNodeDetails(String serviceId, String nodeId,
- InetSocketAddress rpcAddr, int rpcPort, int ratisPort,
- String httpAddress, String httpsAddress) {
- this.scmServiceId = serviceId;
- this.scmNodeId = nodeId;
- this.rpcAddress = rpcAddr;
- this.rpcPort = rpcPort;
- this.ratisPort = ratisPort;
- this.httpAddress = httpAddress;
- this.httpsAddress = httpsAddress;
+ InetSocketAddress rpcAddr, int ratisPort, String httpAddress,
+ String httpsAddress, InetSocketAddress blockProtocolServerAddress,
+ InetSocketAddress clientProtocolServerAddress,
+ InetSocketAddress datanodeProtocolServerAddress,
+ RaftGroup group, RaftPeerId selfPeerId, String datanodeAddressKey,
+ String blockProtocolServerAddressKey,
+ String clientProtocolServerAddressAddressKey) {
+ super(serviceId, nodeId, rpcAddr, ratisPort,
+ httpAddress, httpsAddress);
+ this.blockProtocolServerAddress = blockProtocolServerAddress;
+ this.clientProtocolServerAddress = clientProtocolServerAddress;
+ this.datanodeProtocolServerAddress = datanodeProtocolServerAddress;
+ this.datanodeAddressKey = datanodeAddressKey;
+ this.blockProtocolServerAddressKey = blockProtocolServerAddressKey;
+ this.clientProtocolServerAddressKey = clientProtocolServerAddressAddressKey;
}
@Override
public String toString() {
+ // TODO: add new fields to toString
Review comment:
Probably not.
SCMNodeDetails and OMNodeDetails do not share the same fields. E.g. OMNodeDetails uses RPC port while SCMNodeDetails has three different server port.
----------------------------------------------------------------
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