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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
