You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/08/30 22:28:56 UTC
[GitHub] [hadoop] goiri commented on a change in pull request #3346: HDFS-16188. RBF: Router to support resolving monitored namenodes with DNS
goiri commented on a change in pull request #3346:
URL: https://github.com/apache/hadoop/pull/3346#discussion_r698849492
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/NamenodeHeartbeatService.java
##########
@@ -113,47 +116,91 @@ public NamenodeHeartbeatService(
}
+ /**
+ * Create a new Namenode status updater.
+ *
+ * @param resolver Namenode resolver service to handle NN registration.
+ * @param nsId Identifier of the nameservice.
+ * @param nnId Identifier of the namenode in HA.
+ * @param resolvedHost resolvedHostname for this specific namenode.
+ */
+ public NamenodeHeartbeatService(
+ ActiveNamenodeResolver resolver, String nsId, String nnId, String resolvedHost) {
+ super(NamenodeHeartbeatService.class.getSimpleName() +
Review comment:
We probably want a getNnName() to make this more readable.
##########
File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java
##########
@@ -426,35 +426,42 @@ static String concatSuffixes(String... suffixes) {
Collection<String> nnIds = getNameNodeIds(conf, nsId);
Map<String, InetSocketAddress> ret = Maps.newLinkedHashMap();
for (String nnId : emptyAsSingletonNull(nnIds)) {
- String suffix = concatSuffixes(nsId, nnId);
- String address = checkKeysAndProcess(defaultValue, suffix, conf, keys);
- if (address != null) {
- InetSocketAddress isa = NetUtils.createSocketAddr(address);
- try {
- // Datanode should just use FQDN
- String[] resolvedHostNames = dnr
- .getAllResolvedHostnameByDomainName(isa.getHostName(), true);
- int port = isa.getPort();
- for (String hostname : resolvedHostNames) {
- InetSocketAddress inetSocketAddress = new InetSocketAddress(
- hostname, port);
- // Concat nn info with host info to make uniq ID
- String concatId;
- if (nnId == null || nnId.isEmpty()) {
- concatId = String
- .join("-", nsId, hostname, String.valueOf(port));
- } else {
- concatId = String
- .join("-", nsId, nnId, hostname, String.valueOf(port));
- }
- ret.put(concatId, inetSocketAddress);
+ getResolvedAddressesForNnId(
+ conf, nsId, nnId, dnr, defaultValue, ret, keys);
Review comment:
I think it is better to not have the "ret" as a parameter.
We should return it and use addAll().
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterNamenodeHeartbeat.java
##########
@@ -203,4 +210,64 @@ public void testHearbeat() throws InterruptedException, IOException {
standby = normalNss.get(1);
assertEquals(NAMENODES[1], standby.getNamenodeId());
}
+
+ @Test
+ public void testNamenodeHeartbeatServiceNNResolution() {
+ String nsId = "test-ns";
+ String nnId = "nn";
+ String rpcPort = "1000";
+ String servicePort = "1001";
+ String lifelinePort = "1002";
+ String webAddressPort = "1003";
+ Configuration conf = generateNamenodeConfiguration(nsId, nnId,
+ rpcPort, servicePort, lifelinePort, webAddressPort);
+
+ Router testRouter = new Router();
+ testRouter.setConf(conf);
+
+ Collection<NamenodeHeartbeatService> heartbeatServices =
+ testRouter.createNamenodeHeartbeatServices();
+
+ assertEquals(2, heartbeatServices.size());
+
+ Iterator<NamenodeHeartbeatService> iterator = heartbeatServices.iterator();
+ NamenodeHeartbeatService service = iterator.next();
+ service.init(conf);
+ assertEquals("test-ns-nn-host01.test:host01.test:1001",
+ service.getNamenodeDesc());
+
+ service = iterator.next();
+ service.init(conf);
+ assertEquals("test-ns-nn-host02.test:host02.test:1001",
+ service.getNamenodeDesc());
+
+ }
+
+ private Configuration generateNamenodeConfiguration(
+ String nsId, String nnId,
+ String rpcPort, String servicePort,
Review comment:
Make all the ports ints.
##########
File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java
##########
@@ -426,35 +426,42 @@ static String concatSuffixes(String... suffixes) {
Collection<String> nnIds = getNameNodeIds(conf, nsId);
Map<String, InetSocketAddress> ret = Maps.newLinkedHashMap();
for (String nnId : emptyAsSingletonNull(nnIds)) {
- String suffix = concatSuffixes(nsId, nnId);
- String address = checkKeysAndProcess(defaultValue, suffix, conf, keys);
- if (address != null) {
- InetSocketAddress isa = NetUtils.createSocketAddr(address);
- try {
- // Datanode should just use FQDN
- String[] resolvedHostNames = dnr
- .getAllResolvedHostnameByDomainName(isa.getHostName(), true);
- int port = isa.getPort();
- for (String hostname : resolvedHostNames) {
- InetSocketAddress inetSocketAddress = new InetSocketAddress(
- hostname, port);
- // Concat nn info with host info to make uniq ID
- String concatId;
- if (nnId == null || nnId.isEmpty()) {
- concatId = String
- .join("-", nsId, hostname, String.valueOf(port));
- } else {
- concatId = String
- .join("-", nsId, nnId, hostname, String.valueOf(port));
- }
- ret.put(concatId, inetSocketAddress);
+ getResolvedAddressesForNnId(
+ conf, nsId, nnId, dnr, defaultValue, ret, keys);
+ }
+ return ret;
+ }
+
+ public static void getResolvedAddressesForNnId(
+ Configuration conf, String nsId, String nnId,
+ DomainNameResolver dnr, String defaultValue,
+ Map<String, InetSocketAddress> ret, String... keys) {
+ String suffix = concatSuffixes(nsId, nnId);
+ String address = checkKeysAndProcess(defaultValue, suffix, conf, keys);
+ if (address != null) {
+ InetSocketAddress isa = NetUtils.createSocketAddr(address);
+ try {
+ String[] resolvedHostNames = dnr
+ .getAllResolvedHostnameByDomainName(isa.getHostName(), true);
+ int port = isa.getPort();
+ for (String hostname : resolvedHostNames) {
+ InetSocketAddress inetSocketAddress = new InetSocketAddress(
+ hostname, port);
+ // Concat nn info with host info to make uniq ID
+ String concatId;
+ if (nnId == null || nnId.isEmpty()) {
+ concatId = String
+ .join("-", nsId, hostname, String.valueOf(port));
+ } else {
+ concatId = String
+ .join("-", nsId, nnId, hostname, String.valueOf(port));
}
- } catch (UnknownHostException e) {
- LOG.error("Failed to resolve address: " + address);
+ ret.put(concatId, inetSocketAddress);
}
+ } catch (UnknownHostException e) {
+ LOG.error("Failed to resolve address: " + address);
Review comment:
Use logger {} now that we are moving this
##########
File path: hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/NamenodeHeartbeatService.java
##########
@@ -113,47 +116,91 @@ public NamenodeHeartbeatService(
}
+ /**
+ * Create a new Namenode status updater.
+ *
+ * @param resolver Namenode resolver service to handle NN registration.
+ * @param nsId Identifier of the nameservice.
+ * @param nnId Identifier of the namenode in HA.
+ * @param resolvedHost resolvedHostname for this specific namenode.
+ */
+ public NamenodeHeartbeatService(
+ ActiveNamenodeResolver resolver, String nsId, String nnId, String resolvedHost) {
+ super(NamenodeHeartbeatService.class.getSimpleName() +
+ (nsId == null ? "" : " " + nsId) +
+ (nnId == null ? "" : " " + nnId));
+
+ this.resolver = resolver;
+
+ this.nameserviceId = nsId;
+ // Concat a uniq id from original nnId and resolvedHost
+ this.namenodeId = nnId + "-" + resolvedHost;
+ this.resolvedHost = resolvedHost;
+ // Same the original nnid to get the ports from config.
+ this.originalNnId = nnId;
+
+ }
+
@Override
protected void serviceInit(Configuration configuration) throws Exception {
this.conf = DFSHAAdmin.addSecurityConfiguration(configuration);
String nnDesc = nameserviceId;
if (this.namenodeId != null && !this.namenodeId.isEmpty()) {
- this.localTarget = new NNHAServiceTarget(
- conf, nameserviceId, namenodeId);
nnDesc += "-" + namenodeId;
} else {
this.localTarget = null;
}
+ if (originalNnId == null) {
+ originalNnId = namenodeId;
+ }
+
// Get the RPC address for the clients to connect
- this.rpcAddress = getRpcAddress(conf, nameserviceId, namenodeId);
+ this.rpcAddress = getRpcAddress(conf, nameserviceId, originalNnId);
+ if (resolvedHost != null) {
+ rpcAddress = resolvedHost + ":" + rpcAddress.split(":")[1];
+ }
LOG.info("{} RPC address: {}", nnDesc, rpcAddress);
// Get the Service RPC address for monitoring
this.serviceAddress =
- DFSUtil.getNamenodeServiceAddr(conf, nameserviceId, namenodeId);
+ DFSUtil.getNamenodeServiceAddr(conf, nameserviceId, originalNnId);
if (this.serviceAddress == null) {
LOG.error("Cannot locate RPC service address for NN {}, " +
"using RPC address {}", nnDesc, this.rpcAddress);
this.serviceAddress = this.rpcAddress;
}
+ if (resolvedHost != null) {
+ serviceAddress = resolvedHost + ":" + serviceAddress.split(":")[1];
+ }
LOG.info("{} Service RPC address: {}", nnDesc, serviceAddress);
// Get the Lifeline RPC address for faster monitoring
this.lifelineAddress =
- DFSUtil.getNamenodeLifelineAddr(conf, nameserviceId, namenodeId);
+ DFSUtil.getNamenodeLifelineAddr(conf, nameserviceId, originalNnId);
if (this.lifelineAddress == null) {
this.lifelineAddress = this.serviceAddress;
}
+ if (resolvedHost != null) {
+ lifelineAddress = resolvedHost + ":" + lifelineAddress.split(":")[1];
+ }
LOG.info("{} Lifeline RPC address: {}", nnDesc, lifelineAddress);
// Get the Web address for UI
this.webAddress =
- DFSUtil.getNamenodeWebAddr(conf, nameserviceId, namenodeId);
+ DFSUtil.getNamenodeWebAddr(conf, nameserviceId, originalNnId);
+ if (resolvedHost != null) {
+ webAddress = resolvedHost + ":" + webAddress.split(":")[1];
Review comment:
Extract to port.
##########
File path: hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java
##########
@@ -426,35 +426,42 @@ static String concatSuffixes(String... suffixes) {
Collection<String> nnIds = getNameNodeIds(conf, nsId);
Map<String, InetSocketAddress> ret = Maps.newLinkedHashMap();
for (String nnId : emptyAsSingletonNull(nnIds)) {
- String suffix = concatSuffixes(nsId, nnId);
- String address = checkKeysAndProcess(defaultValue, suffix, conf, keys);
- if (address != null) {
- InetSocketAddress isa = NetUtils.createSocketAddr(address);
- try {
- // Datanode should just use FQDN
- String[] resolvedHostNames = dnr
- .getAllResolvedHostnameByDomainName(isa.getHostName(), true);
- int port = isa.getPort();
- for (String hostname : resolvedHostNames) {
- InetSocketAddress inetSocketAddress = new InetSocketAddress(
- hostname, port);
- // Concat nn info with host info to make uniq ID
- String concatId;
- if (nnId == null || nnId.isEmpty()) {
- concatId = String
- .join("-", nsId, hostname, String.valueOf(port));
- } else {
- concatId = String
- .join("-", nsId, nnId, hostname, String.valueOf(port));
- }
- ret.put(concatId, inetSocketAddress);
+ getResolvedAddressesForNnId(
+ conf, nsId, nnId, dnr, defaultValue, ret, keys);
+ }
+ return ret;
+ }
+
+ public static void getResolvedAddressesForNnId(
+ Configuration conf, String nsId, String nnId,
+ DomainNameResolver dnr, String defaultValue,
+ Map<String, InetSocketAddress> ret, String... keys) {
+ String suffix = concatSuffixes(nsId, nnId);
+ String address = checkKeysAndProcess(defaultValue, suffix, conf, keys);
+ if (address != null) {
+ InetSocketAddress isa = NetUtils.createSocketAddr(address);
+ try {
+ String[] resolvedHostNames = dnr
+ .getAllResolvedHostnameByDomainName(isa.getHostName(), true);
+ int port = isa.getPort();
+ for (String hostname : resolvedHostNames) {
+ InetSocketAddress inetSocketAddress = new InetSocketAddress(
+ hostname, port);
+ // Concat nn info with host info to make uniq ID
Review comment:
getConcatId() could be a static method.
--
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.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org