kerneltime commented on a change in pull request #3186:
URL: https://github.com/apache/ozone/pull/3186#discussion_r840911811



##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -399,6 +400,44 @@ public RegisteredCommand register(
         LOG.error("Cannot find datanode {} from nodeStateManager",
             datanodeDetails.toString());
       }
+    } else {
+      // Update datanode if it is registered but the ip or hostname changes
+      try {
+        final DatanodeInfo datanodeInfo =
+                nodeStateManager.getNode(datanodeDetails);
+        if (!datanodeInfo.getIpAddress().equals(datanodeDetails.getIpAddress())
+                || !datanodeInfo.getHostName()
+                .equals(datanodeDetails.getHostName())) {
+          LOG.info("Updating data node {} from {} to {}",
+                  datanodeDetails.getUuidString(),
+                  datanodeInfo,
+                  datanodeDetails);
+          if (clusterMap.contains(datanodeInfo)) {

Review comment:
       It might be better to implement `clusterMap.update(datanodeDetails)`. 
This would keep the locking and concurrency issues in check.

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
##########
@@ -399,6 +400,44 @@ public RegisteredCommand register(
         LOG.error("Cannot find datanode {} from nodeStateManager",
             datanodeDetails.toString());
       }
+    } else {
+      // Update datanode if it is registered but the ip or hostname changes
+      try {
+        final DatanodeInfo datanodeInfo =
+                nodeStateManager.getNode(datanodeDetails);
+        if (!datanodeInfo.getIpAddress().equals(datanodeDetails.getIpAddress())
+                || !datanodeInfo.getHostName()
+                .equals(datanodeDetails.getHostName())) {
+          LOG.info("Updating data node {} from {} to {}",
+                  datanodeDetails.getUuidString(),
+                  datanodeInfo,
+                  datanodeDetails);
+          if (clusterMap.contains(datanodeInfo)) {
+            clusterMap.remove(datanodeInfo);
+          }
+          clusterMap.add(datanodeDetails);
+
+          String oldDnsName;
+          if (useHostname) {
+            oldDnsName = datanodeInfo.getHostName();
+          } else {
+            oldDnsName = datanodeInfo.getIpAddress();
+          }
+          removeEntryFromDnsToUuidMap(oldDnsName);
+          addEntryToDnsToUuidMap(dnsName, datanodeDetails.getUuidString());

Review comment:
       Same here better to implement a new method 
`updateEntryInDnsToUuisMap(oldDnsName, dnsName, datanodeDetails.getUuidString)`




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to