This is an automated email from the ASF dual-hosted git repository.
sodonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new a7c71f2eba HDDS-9481. A reformatted datanode node cannot be
decommissioned (#5458)
a7c71f2eba is described below
commit a7c71f2eba2426a3bc4eb587d6a33bb1a074bca3
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Wed Oct 18 15:04:11 2023 +0100
HDDS-9481. A reformatted datanode node cannot be decommissioned (#5458)
---
.../hdds/scm/node/NodeDecommissionManager.java | 87 +++++++++++++--
.../apache/hadoop/hdds/scm/node/NodeManager.java | 8 ++
.../hadoop/hdds/scm/node/SCMNodeManager.java | 16 +++
.../hadoop/hdds/scm/container/MockNodeManager.java | 5 +
.../hdds/scm/container/SimpleMockNodeManager.java | 5 +
.../hdds/scm/node/TestNodeDecommissionManager.java | 122 ++++++++++++++++++++-
.../testutils/ReplicationNodeManagerMock.java | 5 +
7 files changed, 233 insertions(+), 15 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java
index a84b07d513..cedca7d3bf 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeDecommissionManager.java
@@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm.node;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState;
@@ -37,11 +38,13 @@ import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.util.ArrayList;
+import java.util.Comparator;
import java.util.LinkedList;
import java.util.List;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
/**
* Class used to manage datanodes scheduled for maintenance or decommission.
@@ -125,11 +128,10 @@ public class NodeDecommissionManager {
dnsName = addr.getHostAddress();
}
List<DatanodeDetails> found = nodeManager.getNodesByAddress(dnsName);
- if (found.size() == 0) {
+ if (found.isEmpty()) {
throw new InvalidHostStringException("Host " + host.getRawHostname()
+ " (" + dnsName + ") is not running any datanodes registered"
- + " with SCM."
- + " Please check the host name.");
+ + " with SCM. Please check the host name.");
} else if (found.size() == 1) {
if (host.getPort() != -1 &&
!validateDNPortMatch(host.getPort(), found.get(0))) {
@@ -139,26 +141,87 @@ public class NodeDecommissionManager {
+ " Please check the port number.");
}
results.add(found.get(0));
- } else if (found.size() > 1) {
- DatanodeDetails match = null;
- for (DatanodeDetails dn : found) {
- if (validateDNPortMatch(host.getPort(), dn)) {
- match = dn;
- break;
- }
+ } else {
+ // Here we either have multiple DNs on the same host / IP, and they
+ // should have different ports. Or, we have a case where a DN was
+ // registered from a host, then stopped and formatted, changing its
+ // UUID and registered again. In that case, the ports of all hosts
+ // should be the same, and we should just use the one with the most
+ // recent heartbeat.
+ if (host.getPort() != -1) {
+ found.removeIf(dn -> !validateDNPortMatch(host.getPort(), dn));
}
- if (match == null) {
+ if (found.isEmpty()) {
+ throw new InvalidHostStringException("Host " + host.getRawHostname()
+ + " is running multiple datanodes registered with SCM,"
+ + " but no port numbers match."
+ + " Please check the port number.");
+ } else if (found.size() == 1) {
+ results.add(found.get(0));
+ continue;
+ }
+ // Here we have at least 2 DNs matching the passed in port, or no port
+ // was passed so we may have all the same ports in SCM or a mix of
+ // ports.
+ if (allPortsMatch(found)) {
+ // All ports match, so just use the most recent heartbeat as it is
+ // not possible for a host to have 2 DNs coming from the same port.
+ DatanodeDetails mostRecent = findDnWithMostRecentHeartbeat(found);
+ if (mostRecent == null) {
+ throw new InvalidHostStringException("Host " +
host.getRawHostname()
+ + " has multiple datanodes registered with SCM."
+ + " All have identical ports, but none have a newest"
+ + " heartbeat.");
+ }
+ results.add(mostRecent);
+ } else {
+ // We have no passed in port or the ports in SCM do not all match, so
+ // we cannot decide which DN to use.
throw new InvalidHostStringException("Host " + host.getRawHostname()
+ " is running multiple datanodes registered with SCM,"
+ " but no port numbers match."
+ " Please check the port number.");
}
- results.add(match);
}
}
return results;
}
+ private boolean allPortsMatch(List<DatanodeDetails> dns) {
+ if (dns.size() < 2) {
+ return true;
+ }
+ int port = dns.get(0).getPort(DatanodeDetails.Port.Name.RATIS).getValue();
+ for (int i = 1; i < dns.size(); i++) {
+ if (dns.get(i).getPort(DatanodeDetails.Port.Name.RATIS).getValue()
+ != port) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private DatanodeDetails findDnWithMostRecentHeartbeat(
+ List<DatanodeDetails> dns) {
+ if (dns.size() < 2) {
+ return dns.isEmpty() ? null : dns.get(0);
+ }
+ List<Pair<DatanodeDetails, Long>> dnsWithHeartbeat = dns.stream()
+ .map(dn -> Pair.of(dn, nodeManager.getLastHeartbeat(dn)))
+ .sorted(Comparator.comparingLong(Pair::getRight))
+ .collect(Collectors.toList());
+ // The last element should have the largest (newest) heartbeat. But also
+ // check it is not identical to the last but 1 element, as then we cannot
+ // determine which node to decommission.
+ Pair<DatanodeDetails, Long> last = dnsWithHeartbeat.get(
+ dnsWithHeartbeat.size() - 1);
+ if (last.getRight() > dnsWithHeartbeat.get(
+ dnsWithHeartbeat.size() - 2).getRight()) {
+ return last.getLeft();
+ }
+ return null;
+ }
+
/**
* Check if the passed port is used by the given DatanodeDetails object. If
* it is, return true, otherwise return false.
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 b35a19b402..011b361d62 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
@@ -380,6 +380,14 @@ public interface NodeManager extends
StorageContainerNodeProtocol,
*/
List<DatanodeDetails> getNodesByAddress(String address);
+ /**
+ * For the given node, retried the last heartbeat time.
+ * @param datanodeDetails DatanodeDetails of the node.
+ * @return The last heartbeat time in milliseconds or -1 if the node does not
+ * existing in the nodeManager.
+ */
+ long getLastHeartbeat(DatanodeDetails datanodeDetails);
+
/**
* Get cluster map as in network topology for this node manager.
* @return cluster map
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 3466fa4234..0d323b996c 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
@@ -1393,6 +1393,22 @@ public class SCMNodeManager implements NodeManager {
return clusterMap;
}
+ /**
+ * For the given node, retried the last heartbeat time.
+ * @param datanodeDetails DatanodeDetails of the node.
+ * @return The last heartbeat time in milliseconds or -1 if the node does not
+ * exist.
+ */
+ @Override
+ public long getLastHeartbeat(DatanodeDetails datanodeDetails) {
+ try {
+ DatanodeInfo node = nodeStateManager.getNode(datanodeDetails);
+ return node.getLastHeartbeatTime();
+ } catch (NodeNotFoundException e) {
+ return -1;
+ }
+ }
+
private String nodeResolve(String hostname) {
List<String> hosts = new ArrayList<>(1);
hosts.add(hostname);
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 6ce5caa21e..98638ebe00 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
@@ -922,6 +922,11 @@ public class MockNodeManager implements NodeManager {
return numPipelinePerDatanode;
}
+ @Override
+ public long getLastHeartbeat(DatanodeDetails datanodeDetails) {
+ return -1;
+ }
+
public void setNumPipelinePerDatanode(int value) {
numPipelinePerDatanode = value;
}
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java
index 8c3d027d1d..2bd13d4489 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/SimpleMockNodeManager.java
@@ -381,6 +381,11 @@ public class SimpleMockNodeManager implements NodeManager {
return 0;
}
+ @Override
+ public long getLastHeartbeat(DatanodeDetails datanodeDetails) {
+ return -1;
+ }
+
@Override
public void close() throws IOException {
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java
index f4e02a495a..c0e3b804bf 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestNodeDecommissionManager.java
@@ -39,6 +39,8 @@ import java.util.List;
import java.util.UUID;
import java.util.Arrays;
import java.util.ArrayList;
+
+import static
org.apache.hadoop.ozone.container.upgrade.UpgradeUtils.defaultLayoutVersionProto;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.assertj.core.api.Fail.fail;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -163,9 +165,18 @@ public class TestNodeDecommissionManager {
assertEquals(HddsProtos.NodeOperationalState.DECOMMISSIONING,
nodeManager.getNodeStatus(multiDn).getOperationalState());
+ // Attempt to decommission on dn(9) which has another instance at
+ // dn(11) with identical ports.
+ nodeManager.processHeartbeat(dns.get(9), defaultLayoutVersionProto());
+ DatanodeDetails duplicatePorts = dns.get(9);
+ decom.decommissionNodes(Arrays.asList(duplicatePorts.getIpAddress()));
+ assertEquals(HddsProtos.NodeOperationalState.DECOMMISSIONING,
+ nodeManager.getNodeStatus(duplicatePorts).getOperationalState());
+
// Recommission all 3 hosts
decom.recommissionNodes(Arrays.asList(
- multiAddr, dns.get(1).getIpAddress(), dns.get(2).getIpAddress()));
+ multiAddr, dns.get(1).getIpAddress(), dns.get(2).getIpAddress(),
+ duplicatePorts.getIpAddress()));
decom.getMonitor().run();
assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
nodeManager.getNodeStatus(dns.get(1)).getOperationalState());
@@ -173,6 +184,76 @@ public class TestNodeDecommissionManager {
nodeManager.getNodeStatus(dns.get(2)).getOperationalState());
assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
nodeManager.getNodeStatus(dns.get(10)).getOperationalState());
+ assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
+ nodeManager.getNodeStatus(duplicatePorts).getOperationalState());
+ }
+
+ @Test
+ public void testNodesCanBeDecommissionedAndRecommissionedMixedPorts()
+ throws InvalidHostStringException, NodeNotFoundException {
+ List<DatanodeDetails> dns = generateDatanodes();
+
+ // From the generateDatanodes method we have DNs at index 9 and 11 with the
+ // same IP and port. We can add another DN with a different port on the
+ // same IP so we have 3 registered from the same host and 2 distinct ports.
+ DatanodeDetails sourceDN = dns.get(9);
+ int ratisPort = sourceDN
+ .getPort(DatanodeDetails.Port.Name.RATIS).getValue();
+ DatanodeDetails.Builder builder = DatanodeDetails.newBuilder();
+ builder.setUuid(UUID.randomUUID())
+ .setHostName(sourceDN.getHostName())
+ .setIpAddress(sourceDN.getIpAddress())
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.STANDALONE,
+ sourceDN.getPort(DatanodeDetails.Port.Name.STANDALONE)
+ .getValue() + 1))
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.RATIS,
+ ratisPort + 1))
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.REST,
+ sourceDN.getPort(DatanodeDetails.Port.Name.REST).getValue() + 1))
+ .setNetworkLocation(sourceDN.getNetworkLocation());
+ DatanodeDetails extraDN = builder.build();
+ dns.add(extraDN);
+ nodeManager.register(extraDN, null, null);
+
+ // Attempt to decommission with just the IP, which should fail.
+ try {
+ decom.decommissionNodes(Arrays.asList(extraDN.getIpAddress()));
+ fail("InvalidHostStringException expected");
+ } catch (InvalidHostStringException e) {
+ }
+ // Now try the one with the unique port
+ decom.decommissionNodes(Arrays.asList(
+ extraDN.getIpAddress() + ":" + ratisPort + 1));
+
+ assertEquals(HddsProtos.NodeOperationalState.DECOMMISSIONING,
+ nodeManager.getNodeStatus(extraDN).getOperationalState());
+
+ decom.recommissionNodes(Arrays.asList(
+ extraDN.getIpAddress() + ":" + ratisPort + 1));
+ decom.getMonitor().run();
+ assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
+ nodeManager.getNodeStatus(extraDN).getOperationalState());
+
+ // Now decommission one of the DNs with the duplicate port
+ DatanodeDetails expectedDN = dns.get(9);
+ nodeManager.processHeartbeat(expectedDN, defaultLayoutVersionProto());
+
+ decom.decommissionNodes(Arrays.asList(
+ expectedDN.getIpAddress() + ":" + ratisPort));
+ assertEquals(HddsProtos.NodeOperationalState.DECOMMISSIONING,
+ nodeManager.getNodeStatus(expectedDN).getOperationalState());
+ // The other duplicate is still in service
+ assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
+ nodeManager.getNodeStatus(dns.get(11)).getOperationalState());
+
+ decom.recommissionNodes(Arrays.asList(
+ expectedDN.getIpAddress() + ":" + ratisPort));
+ decom.getMonitor().run();
+ assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
+ nodeManager.getNodeStatus(expectedDN).getOperationalState());
}
@Test
@@ -206,9 +287,19 @@ public class TestNodeDecommissionManager {
assertEquals(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
nodeManager.getNodeStatus(multiDn).getOperationalState());
+ // Attempt to enable maintenance on dn(9) which has another instance at
+ // dn(11) with identical ports.
+ nodeManager.processHeartbeat(dns.get(9), defaultLayoutVersionProto());
+ DatanodeDetails duplicatePorts = dns.get(9);
+ decom.startMaintenanceNodes(Arrays.asList(duplicatePorts.getIpAddress()),
+ 100);
+ assertEquals(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
+ nodeManager.getNodeStatus(duplicatePorts).getOperationalState());
+
// Recommission all 3 hosts
decom.recommissionNodes(Arrays.asList(
- multiAddr, dns.get(1).getIpAddress(), dns.get(2).getIpAddress()));
+ multiAddr, dns.get(1).getIpAddress(), dns.get(2).getIpAddress(),
+ duplicatePorts.getIpAddress()));
decom.getMonitor().run();
assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
nodeManager.getNodeStatus(dns.get(1)).getOperationalState());
@@ -216,6 +307,8 @@ public class TestNodeDecommissionManager {
nodeManager.getNodeStatus(dns.get(2)).getOperationalState());
assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
nodeManager.getNodeStatus(dns.get(10)).getOperationalState());
+ assertEquals(HddsProtos.NodeOperationalState.IN_SERVICE,
+ nodeManager.getNodeStatus(duplicatePorts).getOperationalState());
}
@Test
@@ -301,7 +394,7 @@ public class TestNodeDecommissionManager {
nodeManager.register(dn, null, null);
}
// We have 10 random DNs, we want to create another one that is on the same
- // host as some of the others.
+ // host as some of the others, but with a different port
DatanodeDetails multiDn = dns.get(0);
DatanodeDetails.Builder builder = DatanodeDetails.newBuilder();
@@ -317,8 +410,31 @@ public class TestNodeDecommissionManager {
.setNetworkLocation(multiDn.getNetworkLocation());
DatanodeDetails dn = builder.build();
+ dns.add(dn);
nodeManager.register(dn, null, null);
+
+ // Now add another DN with the same host and IP as dns(9), and with the
+ // same port.
+ DatanodeDetails duplicatePorts = dns.get(9);
+ builder = DatanodeDetails.newBuilder();
+ builder.setUuid(UUID.randomUUID())
+ .setHostName(duplicatePorts.getHostName())
+ .setIpAddress(duplicatePorts.getIpAddress())
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.STANDALONE,
+ duplicatePorts.getPort(DatanodeDetails.Port.Name.STANDALONE)
+ .getValue()))
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.RATIS,
+
duplicatePorts.getPort(DatanodeDetails.Port.Name.RATIS).getValue()))
+ .addPort(DatanodeDetails.newPort(
+ DatanodeDetails.Port.Name.REST,
+ duplicatePorts.getPort(DatanodeDetails.Port.Name.REST).getValue()))
+ .setNetworkLocation(multiDn.getNetworkLocation());
+ dn = builder.build();
dns.add(dn);
+ nodeManager.register(dn, null, null);
+
return dns;
}
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 21defdb08b..0a86504335 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
@@ -534,4 +534,9 @@ public class ReplicationNodeManagerMock implements
NodeManager {
public int minPipelineLimit(List<DatanodeDetails> dn) {
return 0;
}
+
+ @Override
+ public long getLastHeartbeat(DatanodeDetails datanodeDetails) {
+ return -1;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
