[ 
https://issues.apache.org/jira/browse/HDDS-2199?focusedWorklogId=322089&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-322089
 ]

ASF GitHub Bot logged work on HDDS-2199:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 02/Oct/19 17:44
            Start Date: 02/Oct/19 17:44
    Worklog Time Spent: 10m 
      Work Description: sodonnel commented on pull request #1551: HDDS-2199 In 
SCMNodeManager dnsToUuidMap cannot track multiple DNs on the same host
URL: https://github.com/apache/hadoop/pull/1551#discussion_r330682420
 
 

 ##########
 File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java
 ##########
 @@ -295,7 +297,33 @@ public ScmInfo getScmInfo() throws IOException {
     boolean auditSuccess = true;
     try{
       NodeManager nodeManager = scm.getScmNodeManager();
-      Node client = nodeManager.getNodeByAddress(clientMachine);
 
 Review comment:
   When there are multiple DNs on a given host (host1), you pass in the client 
as host1.
   
   We do a lookup for host1 and we get DN UUIDs (actually DatanodeDetails 
objects) DN1, DN6, DN10 and DN16 which are on this host.
   
   The pipeline passed in is DN4, DN5 and DN10. Ie only one of these hosts is 
on the client machine, but the client machine has other nodes not involved in 
this pipeline.
   
   If it was just the hostname/IP used for later comparison your logic would be 
fine. However ...
   
   The matched client DatanodeDetails object is passed to sortByDistanceCost() 
later in the same method, which calls this for each pipeline node:
   
   ```
   NetworkTopologyImpl.getDistanceCost(reader, nodes.get(i)); 
   ```
   Where reader is the client Node object we found.
   
   In get distance by cost, the first few lines do this:
   
   ```
   if ((node1 != null && node2 != null && node1.equals(node2)) ||
           (node1 == null && node2 == null))  {
         return 0;
       }
   ```
   
   Ie, it both parameters are non-null and **are the same object** returns a 
distance of zero, otherwise it goes through more logic to calculate a distance.
   
   Going back to the example above - your logic would return DN1 which would 
not give a zero distance cost in getDistanceByCost() when compared with any of 
the pipeline nodes.
   
   My more complex logic would return DN10 which would return a zero cost when 
compared with pipeline node DN10, as they are the same object.
   
   So after a rather long example the summary is that if the cost calculation 
was based on hostname your logic would be fine, but as it compares the actual 
node objects I think we need the more complex logic, unfortunately! I have not 
followed the rest of the logic in getDistanceByCost to see if a cost of zero 
would fall out in the end, but I suspect it will be some small non-zero value 
as both nodes will be at the same level.
 
----------------------------------------------------------------
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:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 322089)
    Time Spent: 4h  (was: 3h 50m)

> In SCMNodeManager dnsToUuidMap cannot track multiple DNs on the same host
> -------------------------------------------------------------------------
>
>                 Key: HDDS-2199
>                 URL: https://issues.apache.org/jira/browse/HDDS-2199
>             Project: Hadoop Distributed Data Store
>          Issue Type: Bug
>    Affects Versions: 0.5.0
>            Reporter: Stephen O'Donnell
>            Assignee: Stephen O'Donnell
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 4h
>  Remaining Estimate: 0h
>
> Often in test clusters and tests, we start multiple datanodes on the same 
> host.
> In SCMNodeManager.register() there is a map of hostname -> datanode UUID 
> called dnsToUuidMap.
> If several DNs register from the same host, the entry in the map will be 
> overwritten and the last DN to register will 'win'.
> This means that the method getNodeByAddress() does not return the correct 
> DatanodeDetails object when many hosts are registered from the same address.
> This method is only used in SCMBlockProtocolServer.sortDatanodes() to allow 
> it to see if one of the nodes matches the client, but it need to be used by 
> the Decommission code.
> Perhaps we could change the getNodeByAddress() method to returns a list of 
> DNs? In normal production clusters, there should only be one returned, but in 
> test clusters, there may be many. Any code looking for a specific DN entry 
> would need to iterate the list and match on the port number too, as host:port 
> would be the unique definition of a datanode.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to