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 2021/04/22 09:52:35 UTC
[GitHub] [ozone] bharatviswa504 opened a new pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
bharatviswa504 opened a new pull request #2173:
URL: https://github.com/apache/ozone/pull/2173
## What changes were proposed in this pull request?
Make HA config when SCM HA is enabled to figure out SCM client address.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5127
## How was this patch tested?
Added test.
--
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 #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r618928330
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- final int port = getPortNumberFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY)
- .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT);
+ if (!host.isPresent()) {
Review comment:
I think in case of non_HA setup , scm client address is not defined , client should still be bale to fall back to get the scm address from ozone.scm.names as before.
--
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 #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r618926163
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
Review comment:
NIT: Optional< String > host -> Optional<String> host
--
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] bharatviswa504 edited a comment on pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#issuecomment-826537822
As when re-work on review comments, seen that we need HDDS-5140 also, so included HDDS-5140 also in this. (It is just a cleanup change). I will close HDDS-5140 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] bshashikant commented on a change in pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r618928623
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java
##########
@@ -173,34 +222,6 @@ public void testBlockClientFallbackToScmNamesWithPort() {
assertEquals(OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT, address.getPort());
}
- @Test
- public void testClientFallbackToScmNamesNoPort() {
- // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, it should fallback
Review comment:
I think, once we restore back to failback logic to ozone.scm.names, these tests will still be valid.
--
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] bharatviswa504 commented on a change in pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r619999781
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
Review comment:
Refactored.
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- final int port = getPortNumberFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY)
- .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT);
+ if (!host.isPresent()) {
Review comment:
Done
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java
##########
@@ -173,34 +222,6 @@ public void testBlockClientFallbackToScmNamesWithPort() {
assertEquals(OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT, address.getPort());
}
- @Test
- public void testClientFallbackToScmNamesNoPort() {
- // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, it should fallback
Review comment:
Yes added them back
--
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] bharatviswa504 commented on pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#issuecomment-826537822
As when re-work on review comments, seen that we need HDDS-5140 also, so included HDDS-5140 also in this. (It is just a cleanup change)
--
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] bharatviswa504 commented on a change in pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r619999885
##########
File path: hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/TestHddsClientUtils.java
##########
@@ -173,34 +222,6 @@ public void testBlockClientFallbackToScmNamesWithPort() {
assertEquals(OZONE_SCM_BLOCK_CLIENT_PORT_DEFAULT, address.getPort());
}
- @Test
- public void testClientFallbackToScmNamesNoPort() {
- // When OZONE_SCM_CLIENT_ADDRESS_KEY is undefined, it should fallback
Review comment:
Yes added them back
--
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] bharatviswa504 edited a comment on pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#issuecomment-826537822
As when re-work on review comments, seen that we need HDDS-5140 also, so included HDDS-5140 also in this. (It is just a cleanup change). I will close HDDS-5140 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] bharatviswa504 merged pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2173:
URL: https://github.com/apache/ozone/pull/2173
--
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] bharatviswa504 commented on a change in pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#discussion_r619999781
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
Review comment:
Refactored.
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java
##########
@@ -95,21 +97,38 @@ private HddsUtils() {
*
* @return Target {@code InetSocketAddress} for the SCM client endpoint.
*/
- public static InetSocketAddress getScmAddressForClients(
+ public static Collection<InetSocketAddress> getScmAddressForClients(
ConfigurationSource conf) {
- Optional<String> host = getHostNameFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- if (!host.isPresent()) {
- // Fallback to Ozone SCM name
- host = Optional.of(getSingleSCMAddress(conf).getHostName());
- }
+ if (SCMHAUtils.getScmServiceId(conf) != null) {
+ List<SCMNodeInfo> scmNodeInfoList = SCMNodeInfo.buildNodeInfo(conf);
+ Collection<InetSocketAddress> scmAddressList =
+ new HashSet<>(scmNodeInfoList.size());
+ for (SCMNodeInfo scmNodeInfo : scmNodeInfoList) {
+ if (scmNodeInfo.getScmClientAddress() == null) {
+ throw new ConfigurationException("Ozone scm client address is not " +
+ "set for SCM service-id " + scmNodeInfo.getServiceId() +
+ "node-id" + scmNodeInfo.getNodeId());
+ }
+ scmAddressList.add(
+ NetUtils.createSocketAddr(scmNodeInfo.getScmClientAddress()));
+ }
+ return scmAddressList;
+ } else {
+ Optional< String > host = getHostNameFromConfigKeys(conf,
+ ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY);
- final int port = getPortNumberFromConfigKeys(conf,
- ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY)
- .orElse(ScmConfigKeys.OZONE_SCM_CLIENT_PORT_DEFAULT);
+ if (!host.isPresent()) {
Review comment:
Done
--
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] bharatviswa504 commented on pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#issuecomment-826717317
Thank You @bshashikant for the review.
--
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] bharatviswa504 commented on pull request #2173: HDDS-5127. Fix getServiceList when SCM HA is enabled
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #2173:
URL: https://github.com/apache/ozone/pull/2173#issuecomment-826537822
As when re-work on review comments, seen that we need HDDS-5140 also, so included HDDS-5140 also in this. (It is just a cleanup change)
--
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