From a488e3068d36a66297493dd305753d20a2a2ece7 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Mon, 30 Sep 2019 15:48:26 +0100 Subject: [PATCH 01/10] Changed SCMNodeManager.getNodeByAddress to returns a list of nodes on the given host / address rather than a single node --- .../hadoop/hdds/scm/node/NodeManager.java | 8 +-- .../hadoop/hdds/scm/node/SCMNodeManager.java | 47 +++++++++---- .../scm/server/SCMBlockProtocolServer.java | 30 ++++++++- .../hdds/scm/container/MockNodeManager.java | 36 ++++++++-- .../hdds/scm/node/TestSCMNodeManager.java | 67 ++++++++++++++++++- .../testutils/ReplicationNodeManagerMock.java | 5 +- 6 files changed, 168 insertions(+), 25 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java index d8890fb0b920e..fd8bb87ceb12e 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java @@ -192,11 +192,11 @@ void processNodeReport(DatanodeDetails datanodeDetails, DatanodeDetails getNodeByUuid(String uuid); /** - * Given datanode address(Ipaddress or hostname), returns the DatanodeDetails - * for the node. + * Given datanode address(Ipaddress or hostname), returns a list of + * DatanodeDetails for the datanodes running at that address. * * @param address datanode address - * @return the given datanode, or null if not found + * @return the given datanode, or empty list if none found */ - DatanodeDetails getNodeByAddress(String address); + List getNodesByAddress(String address); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index d3df858e6e6e1..7d9a47a117d85 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -25,6 +25,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.HashSet; +import java.util.LinkedList; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; @@ -98,7 +100,7 @@ public class SCMNodeManager implements NodeManager { private final NetworkTopology clusterMap; private final DNSToSwitchMapping dnsToSwitchMapping; private final boolean useHostname; - private final ConcurrentHashMap dnsToUuidMap = + private final ConcurrentHashMap> dnsToUuidMap = new ConcurrentHashMap<>(); /** @@ -260,7 +262,7 @@ public RegisteredCommand register( } nodeStateManager.addNode(datanodeDetails); clusterMap.add(datanodeDetails); - dnsToUuidMap.put(dnsName, datanodeDetails.getUuidString()); + addEntryTodnsToUuidMap(dnsName, datanodeDetails.getUuidString()); // Updating Node Report, as registration is successful processNodeReport(datanodeDetails, nodeReport); LOG.info("Registered Data node : {}", datanodeDetails); @@ -275,6 +277,22 @@ public RegisteredCommand register( .build(); } + /** + * Add an entry to the dnsToUuidMap, which maps hostname / IP to the DNs + * running on that host. As each address can have many DNs running on it, + * this is a one to many mapping. + * @param dnsName String representing the hostname or IP of the node + * @param uuid String representing the UUID of the registered node. + */ + private void addEntryTodnsToUuidMap(String dnsName, String uuid) { + Set dnList = dnsToUuidMap.get(dnsName); + if (dnList == null) { + dnList = new HashSet<>(); + dnsToUuidMap.put(dnsName, dnList); + } + dnList.add(uuid); + } + /** * Send heartbeat to indicate the datanode is alive and doing well. * @@ -584,29 +602,34 @@ public DatanodeDetails getNodeByUuid(String uuid) { } /** - * Given datanode address(Ipaddress or hostname), returns the DatanodeDetails - * for the node. + * Given datanode address(Ipaddress or hostname), return a list of + * DatanodeDetails for the datanodes registed on that address * * @param address datanode address - * @return the given datanode, or null if not found + * @return the given datanode, or empty list if none found */ @Override - public DatanodeDetails getNodeByAddress(String address) { + public List getNodesByAddress(String address) { + List results = new LinkedList<>(); if (Strings.isNullOrEmpty(address)) { LOG.warn("address is null"); - return null; + return results; } - String uuid = dnsToUuidMap.get(address); - if (uuid != null) { + Set uuids = dnsToUuidMap.get(address); + if (uuids == null) { + LOG.warn("Cannot find node for address {}", address); + return results; + } + + for (String uuid : uuids) { DatanodeDetails temp = DatanodeDetails.newBuilder().setUuid(uuid).build(); try { - return nodeStateManager.getNode(temp); + results.add(nodeStateManager.getNode(temp)); } catch (NodeNotFoundException e) { LOG.warn("Cannot find node for uuid {}", uuid); } } - LOG.warn("Cannot find node for address {}", address); - return null; + return results; } private String nodeResolve(String hostname) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index 5500891d98e95..13913545b15f0 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -26,6 +26,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Random; +import java.util.stream.Collectors; import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.hdds.client.BlockID; @@ -295,7 +297,33 @@ public List sortDatanodes(List nodes, boolean auditSuccess = true; try{ NodeManager nodeManager = scm.getScmNodeManager(); - Node client = nodeManager.getNodeByAddress(clientMachine); + Node client; + List possibleClients = + nodeManager.getNodesByAddress(clientMachine); + if (possibleClients.size() == 1) { + client = possibleClients.get(0); + } else if (possibleClients.size() == 0) { + client = null; + } else { + // Generally a test cluster with many DNs on the same host so check + // if any of the passed hosts match one of the client nodes + List matchedNodes = possibleClients.stream() + .filter(nodes::contains) + .collect(Collectors.toList()); + if (matchedNodes.size() == 0) { + // None of the passed in nodes match a node running on the client + // host, so just set the client as the first one. + client = possibleClients.get(0); + } else if (matchedNodes.size() == 1) { + // Only one of the passed nodes matches a DN on the client, so use it + client = matchedNodes.get(0); + } else { + // Several of the passed nodes are running on the client address, so + // pick a random one. + Random rand = new Random(); + client = matchedNodes.get(rand.nextInt(matchedNodes.size())); + } + } List nodeList = new ArrayList(); nodes.stream().forEach(uuid -> { DatanodeDetails node = nodeManager.getNodeByUuid(uuid); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java index b7a9813483d1d..d5d6fe9158a83 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java @@ -53,6 +53,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.HashSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -88,7 +89,7 @@ public class MockNodeManager implements NodeManager { private final Node2PipelineMap node2PipelineMap; private final Node2ContainerMap node2ContainerMap; private NetworkTopology clusterMap; - private ConcurrentHashMap dnsToUuidMap; + private ConcurrentHashMap> dnsToUuidMap; public MockNodeManager(boolean initializeFakeNodes, int nodeCount) { this.healthyNodes = new LinkedList<>(); @@ -386,7 +387,7 @@ public RegisteredCommand register(DatanodeDetails datanodeDetails, try { node2ContainerMap.insertNewDatanode(datanodeDetails.getUuid(), Collections.emptySet()); - dnsToUuidMap.put(datanodeDetails.getIpAddress(), + addEntryTodnsToUuidMap(datanodeDetails.getIpAddress(), datanodeDetails.getUuidString()); if (clusterMap != null) { datanodeDetails.setNetworkName(datanodeDetails.getUuidString()); @@ -398,6 +399,22 @@ public RegisteredCommand register(DatanodeDetails datanodeDetails, return null; } + /** + * Add an entry to the dnsToUuidMap, which maps hostname / IP to the DNs + * running on that host. As each address can have many DNs running on it, + * this is a one to many mapping. + * @param dnsName String representing the hostname or IP of the node + * @param uuid String representing the UUID of the registered node. + */ + private void addEntryTodnsToUuidMap(String dnsName, String uuid) { + Set dnList = dnsToUuidMap.get(dnsName); + if (dnList == null) { + dnList = new HashSet<>(); + dnsToUuidMap.put(dnsName, dnList); + } + dnList.add(uuid); + } + /** * Send heartbeat to indicate the datanode is alive and doing well. * @@ -484,8 +501,19 @@ public DatanodeDetails getNodeByUuid(String uuid) { } @Override - public DatanodeDetails getNodeByAddress(String address) { - return getNodeByUuid(dnsToUuidMap.get(address)); + public List getNodesByAddress(String address) { + List results = new LinkedList<>(); + Set uuids = dnsToUuidMap.get(address); + if (uuids == null) { + return results; + } + for(String uuid : uuids) { + DatanodeDetails dn = getNodeByUuid(uuid); + if (dn != null) { + results.add(dn); + } + } + return results; } public void setNetworkTopology(NetworkTopology topology) { diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java index d028851168576..db76d6678789a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java @@ -1064,6 +1064,25 @@ public void testScmRegisterNodeWithHostname() testScmRegisterNodeWithNetworkTopology(true); } + /** + * Test getNodesByAddress when using IPs. + * + */ + @Test + public void testgetNodesByAddressWithIpAddress() + throws IOException, InterruptedException, AuthenticationException { + testGetNodesByAddress(false); + } + + /** + * Test getNodesByAddress when using hostnames. + */ + @Test + public void testgetNodesByAddressWithHostname() + throws IOException, InterruptedException, AuthenticationException { + testGetNodesByAddress(true); + } + /** * Test add node into a 4-layer network topology during node register. */ @@ -1152,11 +1171,55 @@ private void testScmRegisterNodeWithNetworkTopology(boolean useHostname) // test get node if (useHostname) { Arrays.stream(hostNames).forEach(hostname -> - Assert.assertNotNull(nodeManager.getNodeByAddress(hostname))); + Assert.assertNotEquals(0, nodeManager.getNodesByAddress(hostname) + .size())); } else { Arrays.stream(ipAddress).forEach(ip -> - Assert.assertNotNull(nodeManager.getNodeByAddress(ip))); + Assert.assertNotEquals(0, nodeManager.getNodesByAddress(ip) + .size())); } } } + + /** + * Test add node into a 4-layer network topology during node register. + */ + private void testGetNodesByAddress(boolean useHostname) + throws IOException, InterruptedException, AuthenticationException { + OzoneConfiguration conf = getConf(); + conf.setTimeDuration(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL, 1000, + MILLISECONDS); + + // create a set of hosts - note two hosts on "host1" + String[] hostNames = {"host1", "host1", "host2", "host3", "host4"}; + String[] ipAddress = + {"1.2.3.4", "1.2.3.4", "2.3.4.5", "3.4.5.6", "4.5.6.7"}; + + if (useHostname) { + conf.set(DFSConfigKeys.DFS_DATANODE_USE_DN_HOSTNAME, "true"); + } + final int nodeCount = hostNames.length; + try (SCMNodeManager nodeManager = createNodeManager(conf)) { + DatanodeDetails[] nodes = new DatanodeDetails[nodeCount]; + for (int i = 0; i < nodeCount; i++) { + DatanodeDetails node = TestUtils.createDatanodeDetails( + UUID.randomUUID().toString(), hostNames[i], ipAddress[i], null); + nodeManager.register(node, null, null); + } + // test get node + Assert.assertEquals(0, nodeManager.getNodesByAddress(null).size()); + if (useHostname) { + Assert.assertEquals(2, + nodeManager.getNodesByAddress("host1").size()); + Assert.assertEquals(1, nodeManager.getNodesByAddress("host2").size()); + Assert.assertEquals(0, nodeManager.getNodesByAddress("unknown").size()); + } else { + Assert.assertEquals(2, + nodeManager.getNodesByAddress("1.2.3.4").size()); + Assert.assertEquals(1, nodeManager.getNodesByAddress("2.3.4.5").size()); + Assert.assertEquals(0, nodeManager.getNodesByAddress("1.9.8.7").size()); + } + } + } + } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/testutils/ReplicationNodeManagerMock.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/testutils/ReplicationNodeManagerMock.java index 30a75efb19424..0ecff3f541a7c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/testutils/ReplicationNodeManagerMock.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/testutils/ReplicationNodeManagerMock.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.LinkedList; /** * A Node Manager to test replication. @@ -323,7 +324,7 @@ public DatanodeDetails getNodeByUuid(String address) { } @Override - public DatanodeDetails getNodeByAddress(String address) { - return null; + public List getNodesByAddress(String address) { + return new LinkedList<>(); } } From 58026eb74038a87852416d9dac826a373119e3e9 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Mon, 30 Sep 2019 16:56:16 +0100 Subject: [PATCH 02/10] Fixed style issue --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 7d9a47a117d85..a8767bd17cb1a 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -603,7 +603,7 @@ public DatanodeDetails getNodeByUuid(String uuid) { /** * Given datanode address(Ipaddress or hostname), return a list of - * DatanodeDetails for the datanodes registed on that address + * DatanodeDetails for the datanodes registered on that address. * * @param address datanode address * @return the given datanode, or empty list if none found From 7e995650ba3ae8ef5e289b9e37573be0a57a7207 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Tue, 1 Oct 2019 12:29:10 +0100 Subject: [PATCH 03/10] Make addEntryTodnsToUuidMap synchronized --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index a8767bd17cb1a..4ef36c0b37353 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -284,7 +284,8 @@ public RegisteredCommand register( * @param dnsName String representing the hostname or IP of the node * @param uuid String representing the UUID of the registered node. */ - private void addEntryTodnsToUuidMap(String dnsName, String uuid) { + private synchronized void addEntryTodnsToUuidMap( + String dnsName, String uuid) { Set dnList = dnsToUuidMap.get(dnsName); if (dnList == null) { dnList = new HashSet<>(); From b7c5d2a3beeeb7107931a705cb23510b84b666ac Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 2 Oct 2019 11:12:04 +0100 Subject: [PATCH 04/10] Added findbugs ignore annotation --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index 4ef36c0b37353..ef5fe01074fc1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -32,6 +32,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.stream.Collectors; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState; @@ -284,6 +285,9 @@ public RegisteredCommand register( * @param dnsName String representing the hostname or IP of the node * @param uuid String representing the UUID of the registered node. */ + @SuppressFBWarnings(value="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION", + justification="The method is synchronized and this is the only place "+ + "dnsToUuidMap is modified") private synchronized void addEntryTodnsToUuidMap( String dnsName, String uuid) { Set dnList = dnsToUuidMap.get(dnsName); From 07df5521e87cf7892f908ed873c394f3f63d6a8c Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 2 Oct 2019 12:01:08 +0100 Subject: [PATCH 05/10] Moved HashSet to a Concurrent version and ensured the MockNodeManager is also synchronized when modifying dnsToUuidMap --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 2 +- .../org/apache/hadoop/hdds/scm/container/MockNodeManager.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index ef5fe01074fc1..e3c78e27a0d6d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -292,7 +292,7 @@ private synchronized void addEntryTodnsToUuidMap( String dnsName, String uuid) { Set dnList = dnsToUuidMap.get(dnsName); if (dnList == null) { - dnList = new HashSet<>(); + dnList = ConcurrentHashMap.newKeySet(); dnsToUuidMap.put(dnsName, dnList); } dnList.add(uuid); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java index d5d6fe9158a83..75c9eb86b6f9e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java @@ -406,10 +406,10 @@ public RegisteredCommand register(DatanodeDetails datanodeDetails, * @param dnsName String representing the hostname or IP of the node * @param uuid String representing the UUID of the registered node. */ - private void addEntryTodnsToUuidMap(String dnsName, String uuid) { + private synchronized void addEntryTodnsToUuidMap(String dnsName, String uuid) { Set dnList = dnsToUuidMap.get(dnsName); if (dnList == null) { - dnList = new HashSet<>(); + dnList = ConcurrentHashMap.newKeySet(); dnsToUuidMap.put(dnsName, dnList); } dnList.add(uuid); From 7fe483a58ff97a9998cf9c6bb5252ad5ee1e5e59 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 2 Oct 2019 14:53:21 +0100 Subject: [PATCH 06/10] Fixed further style issues --- .../java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java | 1 - .../org/apache/hadoop/hdds/scm/container/MockNodeManager.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java index e3c78e27a0d6d..ed65ed3555321 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.HashSet; import java.util.LinkedList; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java index 75c9eb86b6f9e..6f5d4356fb3dc 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java @@ -53,7 +53,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.HashSet; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -406,7 +405,8 @@ public RegisteredCommand register(DatanodeDetails datanodeDetails, * @param dnsName String representing the hostname or IP of the node * @param uuid String representing the UUID of the registered node. */ - private synchronized void addEntryTodnsToUuidMap(String dnsName, String uuid) { + private synchronized void addEntryTodnsToUuidMap( + String dnsName, String uuid) { Set dnList = dnsToUuidMap.get(dnsName); if (dnList == null) { dnList = ConcurrentHashMap.newKeySet(); From 768ae91aa88a09b6b1fb5235b599cefcf44bad5a Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 2 Oct 2019 15:44:23 +0100 Subject: [PATCH 07/10] Refactored the the change in sortDatanodes() to make it simpler and remove the random lookup --- .../scm/server/SCMBlockProtocolServer.java | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index 13913545b15f0..7530ffab38255 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -297,14 +297,12 @@ public List sortDatanodes(List nodes, boolean auditSuccess = true; try{ NodeManager nodeManager = scm.getScmNodeManager(); - Node client; + Node client = null; List possibleClients = nodeManager.getNodesByAddress(clientMachine); if (possibleClients.size() == 1) { client = possibleClients.get(0); - } else if (possibleClients.size() == 0) { - client = null; - } else { + } else if (possibleClients.size() > 1) { // Generally a test cluster with many DNs on the same host so check // if any of the passed hosts match one of the client nodes List matchedNodes = possibleClients.stream() @@ -312,16 +310,13 @@ public List sortDatanodes(List nodes, .collect(Collectors.toList()); if (matchedNodes.size() == 0) { // None of the passed in nodes match a node running on the client - // host, so just set the client as the first one. + // host, so just set the client as the first one originally found as + // the client is part of the cluster and the nodes are sorted by + // distance from the client. client = possibleClients.get(0); - } else if (matchedNodes.size() == 1) { - // Only one of the passed nodes matches a DN on the client, so use it - client = matchedNodes.get(0); } else { - // Several of the passed nodes are running on the client address, so - // pick a random one. - Random rand = new Random(); - client = matchedNodes.get(rand.nextInt(matchedNodes.size())); + // One or more matches the client, so just use the first one. + client = matchedNodes.get(0); } } List nodeList = new ArrayList(); From dc1e393302c1468ee84520f3eddda1cbe28f4a8e Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 2 Oct 2019 18:47:28 +0100 Subject: [PATCH 08/10] Fixed another style issue (unused import) after removing the random logic --- .../apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index 7530ffab38255..de2de3f7dcbb9 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Random; import java.util.stream.Collectors; import org.apache.hadoop.fs.CommonConfigurationKeys; From 4421b5a9d1f8328349cbb4b3c170150f64f3b663 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Thu, 3 Oct 2019 18:29:28 +0100 Subject: [PATCH 09/10] Simplify logic used to map Client host string to a DatanodeDetails object --- .../scm/server/SCMBlockProtocolServer.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index de2de3f7dcbb9..d2567ef332338 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -299,24 +299,8 @@ public List sortDatanodes(List nodes, Node client = null; List possibleClients = nodeManager.getNodesByAddress(clientMachine); - if (possibleClients.size() == 1) { + if (possibleClients.size()>0){ client = possibleClients.get(0); - } else if (possibleClients.size() > 1) { - // Generally a test cluster with many DNs on the same host so check - // if any of the passed hosts match one of the client nodes - List matchedNodes = possibleClients.stream() - .filter(nodes::contains) - .collect(Collectors.toList()); - if (matchedNodes.size() == 0) { - // None of the passed in nodes match a node running on the client - // host, so just set the client as the first one originally found as - // the client is part of the cluster and the nodes are sorted by - // distance from the client. - client = possibleClients.get(0); - } else { - // One or more matches the client, so just use the first one. - client = matchedNodes.get(0); - } } List nodeList = new ArrayList(); nodes.stream().forEach(uuid -> { From 5d97878aacf91812d55c0178d91110562a76fc13 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Thu, 3 Oct 2019 18:37:04 +0100 Subject: [PATCH 10/10] Remove unused import --- .../apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index d2567ef332338..9c69758d5a0a8 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.hdds.client.BlockID;