hadoop-ozone-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From adorosz...@apache.org
Subject [hadoop-ozone] branch HDDS-1880-Decom updated: HDDS-2671. Have NodeManager.getNodeStatus throw NodeNotFoundException (#328)
Date Tue, 17 Dec 2019 18:22:07 GMT
This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch HDDS-1880-Decom
in repository https://gitbox.apache.org/repos/asf/hadoop-ozone.git


The following commit(s) were added to refs/heads/HDDS-1880-Decom by this push:
     new 692420f  HDDS-2671. Have NodeManager.getNodeStatus throw NodeNotFoundException (#328)
692420f is described below

commit 692420faeea70fe02ab805a448d8a08c33956c11
Author: Stephen O'Donnell <stephen.odonnell@gmail.com>
AuthorDate: Tue Dec 17 18:22:00 2019 +0000

    HDDS-2671. Have NodeManager.getNodeStatus throw NodeNotFoundException (#328)
---
 .../hdds/scm/container/ReplicationManager.java     | 20 +++++++++++++++++--
 .../hadoop/hdds/scm/node/DatanodeAdminMonitor.java |  1 -
 .../hdds/scm/node/DatanodeAdminMonitorImpl.java    |  9 +--------
 .../hdds/scm/node/NodeDecommissionManager.java     |  6 +-----
 .../apache/hadoop/hdds/scm/node/NodeManager.java   |  4 +++-
 .../hadoop/hdds/scm/node/SCMNodeManager.java       | 10 +++-------
 .../hdds/scm/server/SCMClientProtocolServer.java   | 23 +++++++++++++---------
 .../hadoop/hdds/scm/container/MockNodeManager.java |  3 ++-
 .../hdds/scm/container/SimpleMockNodeManager.java  |  3 ++-
 .../hdds/scm/node/TestDatanodeAdminMonitor.java    | 18 +++++++++++------
 .../hdds/scm/node/TestNodeDecommissionManager.java |  5 +++--
 11 files changed, 59 insertions(+), 43 deletions(-)

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
index b18024e..dfb5961 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ReplicationManager.java
@@ -42,6 +42,8 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolPro
 import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementPolicy;
 import org.apache.hadoop.hdds.scm.events.SCMEvents;
 import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.NodeStatus;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
 import org.apache.hadoop.hdds.server.events.EventPublisher;
 import org.apache.hadoop.metrics2.MetricsCollector;
 import org.apache.hadoop.metrics2.MetricsInfo;
@@ -544,7 +546,7 @@ public class ReplicationManager implements MetricsSource {
           // maintenance nodes, as the replicas will remain present in the
           // container manager, even when they go dead.
           .filter(r ->
-              nodeManager.getNodeStatus(r.getDatanodeDetails()).isHealthy())
+              getNodeStatus(r.getDatanodeDetails()).isHealthy())
           .filter(r -> !deletionInFlight.contains(r.getDatanodeDetails()))
           .sorted((r1, r2) -> r2.getSequenceId().compareTo(r1.getSequenceId()))
           .map(ContainerReplica::getDatanodeDetails)
@@ -574,7 +576,7 @@ public class ReplicationManager implements MetricsSource {
         LOG.warn("Cannot replicate container {}, no healthy replica found.",
             container.containerID());
       }
-    } catch (IOException ex) {
+    } catch (IOException | IllegalStateException ex) {
       LOG.warn("Exception while replicating container {}.",
           container.getContainerID(), ex);
     }
@@ -786,6 +788,20 @@ public class ReplicationManager implements MetricsSource {
   }
 
   /**
+   * Wrap the call to nodeManager.getNodeStatus, catching any
+   * NodeNotFoundException and instead throwing an IllegalStateException.
+   * @param dn The datanodeDetails to obtain the NodeStatus for
+   * @return NodeStatus corresponding to the given Datanode.
+   */
+  private NodeStatus getNodeStatus(DatanodeDetails dn) {
+    try {
+      return nodeManager.getNodeStatus(dn);
+    } catch (NodeNotFoundException e) {
+      throw new IllegalStateException("Unable to find NodeStatus for "+dn, e);
+    }
+  }
+
+  /**
    * Compares the container state with the replica state.
    *
    * @param containerState ContainerState
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitor.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitor.java
index 9ead66f..e78eeff 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitor.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitor.java
@@ -26,7 +26,6 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 public interface DatanodeAdminMonitor extends Runnable {
 
   void startMonitoring(DatanodeDetails dn, int endInHours);
-
   void stopMonitoring(DatanodeDetails dn);
 
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitorImpl.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitorImpl.java
index 8eb985b..4d2d895 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitorImpl.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeAdminMonitorImpl.java
@@ -360,16 +360,9 @@ public class DatanodeAdminMonitorImpl implements DatanodeAdminMonitor
{
     nodeManager.setNodeOperationalState(dn.getDatanodeDetails(), state);
   }
 
-  // TODO - The nodeManager.getNodeStatus call should really throw
-  //        NodeNotFoundException rather than having to handle it here as all
-  //        registered nodes must have a status.
   private NodeStatus getNodeStatus(DatanodeDetails dnd)
       throws NodeNotFoundException {
-    NodeStatus nodeStatus = nodeManager.getNodeStatus(dnd);
-    if (nodeStatus == null) {
-      throw new NodeNotFoundException("Unable to retrieve the nodeStatus");
-    }
-    return nodeStatus;
+    return nodeManager.getNodeStatus(dnd);
   }
 
 }
\ No newline at end of file
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 4c9bf9c..06fe270 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
@@ -331,11 +331,7 @@ public class NodeDecommissionManager {
 
   private NodeStatus getNodeStatus(DatanodeDetails dn)
       throws NodeNotFoundException {
-    NodeStatus nodeStatus = nodeManager.getNodeStatus(dn);
-    if (nodeStatus == null) {
-      throw new NodeNotFoundException();
-    }
-    return nodeStatus;
+    return nodeManager.getNodeStatus(dn);
   }
 
 }
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 252c38f..b595d00 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
@@ -128,8 +128,10 @@ public interface NodeManager extends StorageContainerNodeProtocol,
    * Returns the node status of a specific node.
    * @param datanodeDetails DatanodeDetails
    * @return NodeStatus for the node
+   * @throws NodeNotFoundException if the node does not exist
    */
-  NodeStatus getNodeStatus(DatanodeDetails datanodeDetails);
+  NodeStatus getNodeStatus(DatanodeDetails datanodeDetails)
+      throws NodeNotFoundException;
 
   /**
    * Set the operation state of a node.
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 131a0e4..3c5071f 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
@@ -218,13 +218,9 @@ public class SCMNodeManager implements NodeManager {
    * @return NodeStatus for the node
    */
   @Override
-  public NodeStatus getNodeStatus(DatanodeDetails datanodeDetails) {
-    try {
-      return nodeStateManager.getNodeStatus(datanodeDetails);
-    } catch (NodeNotFoundException e) {
-      // TODO: should we throw NodeNotFoundException?
-      return null;
-    }
+  public NodeStatus getNodeStatus(DatanodeDetails datanodeDetails)
+      throws NodeNotFoundException {
+    return nodeStateManager.getNodeStatus(datanodeDetails);
   }
 
   /**
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
index 76efb3a..8f6b356 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
@@ -36,6 +36,7 @@ import org.apache.hadoop.hdds.scm.ScmInfo;
 import org.apache.hadoop.hdds.scm.ScmUtils;
 import org.apache.hadoop.hdds.scm.node.NodeStatus;
 import org.apache.hadoop.hdds.scm.events.SCMEvents;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
 import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException;
 import org.apache.hadoop.hdds.scm.safemode.SafeModePrecheck;
 import org.apache.hadoop.hdds.scm.container.ContainerID;
@@ -357,15 +358,19 @@ public class SCMClientProtocolServer implements
     }
 
     List<HddsProtos.Node> result = new ArrayList<>();
-    queryNode(opState, state)
-        .forEach(node -> {
-          NodeStatus ns = scm.getScmNodeManager().getNodeStatus(node);
-          result.add(HddsProtos.Node.newBuilder()
-              .setNodeID(node.getProtoBufMessage())
-              .addNodeStates(ns.getHealth())
-              .addNodeOperationalStates(ns.getOperationalState())
-              .build());
-        });
+    for (DatanodeDetails node : queryNode(opState, state)) {
+      try {
+        NodeStatus ns = scm.getScmNodeManager().getNodeStatus(node);
+        result.add(HddsProtos.Node.newBuilder()
+            .setNodeID(node.getProtoBufMessage())
+            .addNodeStates(ns.getHealth())
+            .addNodeOperationalStates(ns.getOperationalState())
+            .build());
+      } catch (NodeNotFoundException e) {
+        throw new IOException(
+            "An unexpected error occurred querying the NodeStatus", e);
+      }
+    }
     return result;
   }
 
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 1a5db7f..45be60b 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
@@ -263,7 +263,8 @@ public class MockNodeManager implements NodeManager {
    * @return Healthy/Stale/Dead.
    */
   @Override
-  public NodeStatus getNodeStatus(DatanodeDetails dd) {
+  public NodeStatus getNodeStatus(DatanodeDetails dd)
+      throws NodeNotFoundException {
     return null;
   }
 
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 78d576c..883e910 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
@@ -87,7 +87,8 @@ public class SimpleMockNodeManager implements NodeManager {
    *         Inservice and Healthy NodeStatus.
    */
   @Override
-  public NodeStatus getNodeStatus(DatanodeDetails datanodeDetails) {
+  public NodeStatus getNodeStatus(DatanodeDetails datanodeDetails)
+      throws NodeNotFoundException {
     DatanodeInfo dni = nodeMap.get(datanodeDetails.getUuid());
     if (dni != null) {
       return dni.getNodeStatus();
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDatanodeAdminMonitor.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDatanodeAdminMonitor.java
index 5b74750..f3b7d8f 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDatanodeAdminMonitor.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestDatanodeAdminMonitor.java
@@ -98,7 +98,8 @@ public class TestDatanodeAdminMonitor {
    * must wait until the pipelines have closed before completing the flow.
    */
   @Test
-  public void testClosePipelinesEventFiredWhenAdminStarted() {
+  public void testClosePipelinesEventFiredWhenAdminStarted()
+      throws NodeNotFoundException{
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.DECOMMISSIONING,
@@ -136,7 +137,8 @@ public class TestDatanodeAdminMonitor {
    * state.
    */
   @Test
-  public void testDecommissionNodeTransitionsToCompleteWhenNoContainers() {
+  public void testDecommissionNodeTransitionsToCompleteWhenNoContainers()
+      throws NodeNotFoundException {
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.DECOMMISSIONING,
@@ -280,7 +282,8 @@ public class TestDatanodeAdminMonitor {
   }
 
   @Test
-  public void testMaintenanceWaitsForMaintenanceToComplete() {
+  public void testMaintenanceWaitsForMaintenanceToComplete()
+      throws NodeNotFoundException {
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
@@ -310,7 +313,8 @@ public class TestDatanodeAdminMonitor {
   }
 
   @Test
-  public void testMaintenanceEndsClosingPipelines() {
+  public void testMaintenanceEndsClosingPipelines()
+      throws NodeNotFoundException {
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
@@ -366,7 +370,8 @@ public class TestDatanodeAdminMonitor {
   }
 
   @Test
-  public void testDeadMaintenanceNodeDoesNotAbortWorkflow() {
+  public void testDeadMaintenanceNodeDoesNotAbortWorkflow()
+      throws NodeNotFoundException {
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
@@ -393,7 +398,8 @@ public class TestDatanodeAdminMonitor {
   }
 
   @Test
-  public void testCancelledNodesMovedToInService() {
+  public void testCancelledNodesMovedToInService()
+      throws NodeNotFoundException {
     DatanodeDetails dn1 = TestUtils.randomDatanodeDetails();
     nodeManager.register(dn1,
         new NodeStatus(HddsProtos.NodeOperationalState.ENTERING_MAINTENANCE,
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 47055f5..daf6731 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
@@ -23,6 +23,7 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.scm.HddsTestUtils;
 import org.apache.hadoop.hdds.scm.TestUtils;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
 import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
 import org.apache.hadoop.security.authentication.client.AuthenticationException;
 import org.apache.hadoop.test.GenericTestUtils;
@@ -134,7 +135,7 @@ public class TestNodeDecommissionManager {
 
   @Test
   public void testNodesCanBeDecommissionedAndRecommissioned()
-      throws InvalidHostStringException {
+      throws InvalidHostStringException, NodeNotFoundException {
     List<DatanodeDetails> dns = generateDatanodes();
 
     // Decommission 2 valid nodes
@@ -173,7 +174,7 @@ public class TestNodeDecommissionManager {
 
   @Test
   public void testNodesCanBePutIntoMaintenanceAndRecommissioned()
-      throws InvalidHostStringException {
+      throws InvalidHostStringException, NodeNotFoundException {
     List<DatanodeDetails> dns = generateDatanodes();
 
     // Put 2 valid nodes into maintenance


---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-commits-help@hadoop.apache.org


Mime
View raw message