[GitHub] [ozone] sodonnel commented on a change in pull request #2377: HDDS-5296. Replication Manager should not sent replica command to DN which is not in IN_SERVICE opState

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

sodonnel commented on a change in pull request #2377:
URL: https://github.com/apache/ozone/pull/2377#discussion_r662070011



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
##########
@@ -79,7 +86,10 @@ public void addNode(DatanodeDetails datanodeDetails, NodeStatus nodeStatus)
       if (nodeMap.containsKey(id)) {
         throw new NodeAlreadyExistsException("Node UUID: " + id);
       }
-      nodeMap.put(id, new DatanodeInfo(datanodeDetails, nodeStatus));
+      DatanodeInfo datanodeInfo = new DatanodeInfo(datanodeDetails, nodeStatus);
+      // Make sure nodes in topology tree has all DN info.
+      clusterMap.add(datanodeInfo);
+      nodeMap.put(id, datanodeInfo);

Review comment:
       I'm not sure about this. At the moment all calls to getNodeStatus (and all things related to datanodeInfo) are protected by a lock in nodeStateMap. If we just pass the DatanodeInfo object in here, then it means we are potentially accessing it concurrently, and that could result in hard to predict problems.




-- 
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: issues-unsubscribe@ozone.apache.org

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