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 4fb7f84394f3..30b661291f9b 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 @@ -113,7 +113,7 @@ default void registerSendCommandNotify(SCMCommandProto.Type type, * @param health - The health of the node * @return List of Datanodes that are Heartbeating SCM. */ - List getNodes( + List getNodes( NodeOperationalState opState, NodeState health); /** @@ -134,10 +134,8 @@ List getNodes( int getNodeCount( NodeOperationalState opState, NodeState health); - /** - * @return all datanodes known to SCM. - */ - List getAllNodes(); + /** @return a shadow copied list of all datanodes, sorted by {@link DatanodeID}. */ + List getAllNodes(); /** @return the number of datanodes. */ default int getAllNodeCount() { @@ -175,15 +173,6 @@ default int getAllNodeCount() { */ DatanodeUsageInfo getUsageInfo(DatanodeDetails dn); - /** - * Get the datanode info of a specified datanode. - * - * @param dn the usage of which we want to get - * @return DatanodeInfo of the specified datanode - */ - @Nullable - DatanodeInfo getDatanodeInfo(DatanodeDetails dn); - /** * True if the node can accept another container of the given size. */ @@ -381,7 +370,7 @@ Map getTotalDatanodeCommandCounts( List> getCommandQueue(DatanodeID dnID); /** @return the datanode of the given id if it exists; otherwise, return null. */ - @Nullable DatanodeDetails getNode(@Nullable DatanodeID id); + @Nullable DatanodeInfo getNode(@Nullable DatanodeID id); /** * Given datanode address(Ipaddress or hostname), returns a list of diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java index 9e4b96999df0..2e67f719905d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java @@ -531,12 +531,8 @@ public int getVolumeFailuresNodeCount() { return getVolumeFailuresNodes().size(); } - /** - * Returns all the nodes which have registered to NodeStateManager. - * - * @return all the managed nodes - */ - public List getAllNodes() { + /** @return a shadow copied list of all datanodes, sorted by {@link DatanodeID}. */ + List getAllNodes() { return nodeStateMap.getAllDatanodeInfos(); } 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 ad392a247d53..09c3f19024b7 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 @@ -27,7 +27,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import jakarta.annotation.Nullable; import java.io.IOException; import java.math.RoundingMode; import java.net.InetAddress; @@ -260,11 +259,9 @@ public List getNodes(NodeStatus nodeStatus) { * @return List of Datanodes that are known to SCM in the requested states. */ @Override - public List getNodes( + public List getNodes( NodeOperationalState opState, NodeState health) { - return nodeStateManager.getNodes(opState, health) - .stream() - .map(node -> (DatanodeDetails)node).collect(Collectors.toList()); + return nodeStateManager.getNodes(opState, health); } @Override @@ -1005,8 +1002,7 @@ public Map getNodeStats() { @Override public List getMostOrLeastUsedDatanodes( boolean mostUsed) { - List healthyNodes = - getNodes(IN_SERVICE, NodeState.HEALTHY); + final List healthyNodes = getNodes(IN_SERVICE, NodeState.HEALTHY); List datanodeUsageInfoList = new ArrayList<>(healthyNodes.size()); @@ -1053,24 +1049,6 @@ public DatanodeUsageInfo getUsageInfo(DatanodeDetails dn) { return usageInfo; } - /** - * Get the usage info of a specified datanode. - * - * @param dn the usage of which we want to get - * @return DatanodeUsageInfo of the specified datanode - */ - @Override - @Nullable - public DatanodeInfo getDatanodeInfo(DatanodeDetails dn) { - try { - return nodeStateManager.getNode(dn); - } catch (NodeNotFoundException e) { - LOG.warn("Cannot retrieve DatanodeInfo, datanode {} not found.", - dn.getUuid()); - return null; - } - } - /** * Effective space check aligned with container allocation: per-disk slot model minus * SCM pending allocations. diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java index 8aea57b23ab0..67c487e3ba1c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java @@ -17,10 +17,10 @@ package org.apache.hadoop.hdds.scm.node.states; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; @@ -41,7 +41,7 @@ */ public class NodeStateMap { /** Map: {@link DatanodeID} -> ({@link DatanodeInfo}, {@link ContainerID}s). */ - private final Map nodeMap = new HashMap<>(); + private final Map nodeMap = new TreeMap<>(); private final ReadWriteLock lock = new ReentrantReadWriteLock(); @@ -166,11 +166,7 @@ public int getNodeCount() { } } - /** - * Returns the list of all the nodes as DatanodeInfo objects. - * - * @return list of all the node ids - */ + /** @return a shadow copied list of all datanodes, sorted by {@link DatanodeID}. */ public List getAllDatanodeInfos() { lock.readLock().lock(); try { 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 156eed688d81..382eeb0c1b59 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 @@ -45,7 +45,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.TreeSet; import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -678,9 +677,9 @@ public List queryNode( } try { List result = new ArrayList<>(); - for (DatanodeDetails node : queryNode(opState, state)) { + for (DatanodeDetails node : scm.getScmNodeManager().getNodes(opState, state)) { NodeStatus ns = scm.getScmNodeManager().getNodeStatus(node); - DatanodeInfo datanodeInfo = scm.getScmNodeManager().getDatanodeInfo(node); + DatanodeInfo datanodeInfo = node instanceof DatanodeInfo ? (DatanodeInfo) node : null; HddsProtos.Node.Builder nodeBuilder = HddsProtos.Node.newBuilder() .setNodeID(node.toProto(clientVersion)) .addNodeStates(ns.getHealth()) @@ -704,34 +703,22 @@ public List queryNode( } @Override - public HddsProtos.Node queryNode(UUID uuid) - throws IOException { + public HddsProtos.Node queryNode(UUID uuid) { final Map auditMap = Maps.newHashMap(); auditMap.put("uuid", String.valueOf(uuid)); HddsProtos.Node result = null; - try { - DatanodeDetails node = scm.getScmNodeManager().getNode(DatanodeID.of(uuid)); - if (node != null) { - NodeStatus ns = scm.getScmNodeManager().getNodeStatus(node); - DatanodeInfo datanodeInfo = scm.getScmNodeManager().getDatanodeInfo(node); - HddsProtos.Node.Builder nodeBuilder = HddsProtos.Node.newBuilder() - .setNodeID(node.getProtoBufMessage()) - .addNodeStates(ns.getHealth()) - .addNodeOperationalStates(ns.getOperationalState()); - - if (datanodeInfo != null) { - nodeBuilder.setTotalVolumeCount(datanodeInfo.getStorageReports().size()); - nodeBuilder.setHealthyVolumeCount(datanodeInfo.getHealthyVolumeCount()); - addFailedVolumes(nodeBuilder, datanodeInfo); - } - result = nodeBuilder.build(); - } - } catch (NodeNotFoundException e) { - IOException ex = new IOException( - "An unexpected error occurred querying the NodeStatus", e); - AUDIT.logReadFailure(buildAuditMessageForFailure( - SCMAction.QUERY_NODE, auditMap, ex)); - throw ex; + DatanodeInfo datanodeInfo = scm.getScmNodeManager().getNode(DatanodeID.of(uuid)); + if (datanodeInfo != null) { + NodeStatus ns = datanodeInfo.getNodeStatus(); + HddsProtos.Node.Builder nodeBuilder = HddsProtos.Node.newBuilder() + .setNodeID(datanodeInfo.getProtoBufMessage()) + .addNodeStates(ns.getHealth()) + .addNodeOperationalStates(ns.getOperationalState()); + + nodeBuilder.setTotalVolumeCount(datanodeInfo.getStorageReports().size()); + nodeBuilder.setHealthyVolumeCount(datanodeInfo.getHealthyVolumeCount()); + addFailedVolumes(nodeBuilder, datanodeInfo); + result = nodeBuilder.build(); } AUDIT.logReadSuccess(buildAuditMessageForSuccess( SCMAction.QUERY_NODE, auditMap)); @@ -1580,26 +1567,6 @@ public List getListOfContainerIDs( } } - /** - * Queries a list of Node that match a set of statuses. - * - *

For example, if the nodeStatuses is HEALTHY and RAFT_MEMBER, then - * this call will return all - * healthy nodes which members in Raft pipeline. - * - *

Right now we don't support operations, so we assume it is an AND - * operation between the - * operators. - * - * @param opState - NodeOperational State - * @param state - NodeState. - * @return List of Datanodes. - */ - public List queryNode( - HddsProtos.NodeOperationalState opState, HddsProtos.NodeState state) { - return new ArrayList<>(queryNodeState(opState, state)); - } - @VisibleForTesting public StorageContainerManager getScm() { return scm; @@ -1612,24 +1579,6 @@ public boolean getSafeModeStatus() { return scm.getScmContext().isInSafeMode(); } - /** - * Query the System for Nodes. - * - * @params opState - The node operational state - * @param nodeState - NodeState that we are interested in matching. - * @return Set of Datanodes that match the NodeState. - */ - private Set queryNodeState( - HddsProtos.NodeOperationalState opState, HddsProtos.NodeState nodeState) { - Set returnSet = new TreeSet<>(); - List tmp = scm.getScmNodeManager() - .getNodes(opState, nodeState); - if ((tmp != null) && (!tmp.isEmpty())) { - returnSet.addAll(tmp); - } - return returnSet; - } - @Override public AuditMessage buildAuditMessageForSuccess( AuditAction op, Map auditMap) { diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java index dba2d60b98ce..b18a9dad287d 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java @@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; import java.io.File; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -85,11 +86,15 @@ void setup(@TempDir File testDir) { conf = SCMTestUtils.getConf(testDir); } + static List getAllNodes(NodeManager nm) { + return new ArrayList<>(nm.getAllNodes()); + } + @Test public void testGetResultSet() throws SCMException { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List result = dummyPlacementPolicy.getResultSet(3, list); Set resultSet = new HashSet<>(result); assertNotEquals(1, resultSet.size()); @@ -137,7 +142,7 @@ public void testReplicasToFixMisreplicationWithOneMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 5) .map(list::get).collect(Collectors.toList()); List replicas = @@ -158,7 +163,7 @@ public void testReplicasToFixMisreplicationWithTwoMisreplication() { 3, ImmutableList.of(3, 8), 4, ImmutableList.of(4, 9))), 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 5) .map(list::get).collect(Collectors.toList()); List replicas = @@ -179,7 +184,7 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { 3, ImmutableList.of(3, 8), 4, ImmutableList.of(4, 9))), 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 5) .map(list::get).collect(Collectors.toList()); List replicas = @@ -201,7 +206,7 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { 3, ImmutableList.of(3, 4, 8), 4, ImmutableList.of(9))), 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); //Creating Replicas without replica Index @@ -224,7 +229,7 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { 3, ImmutableList.of(3, 4, 8), 4, ImmutableList.of(9))), 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 3, 4) .map(list::get).collect(Collectors.toList()); //Creating Replicas without replica Index for replicas < number of racks @@ -247,7 +252,7 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { 3, ImmutableList.of(3, 4, 8), 4, ImmutableList.of(9))), 5); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 4, 6) .map(list::get).collect(Collectors.toList()); //Creating Replicas without replica Index for replicas >number of racks @@ -262,7 +267,7 @@ public void testReplicasToFixMisreplicationMaxReplicaPerRack() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 2); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 2, 4, 6, 8) .map(list::get).collect(Collectors.toList()); List replicas = @@ -278,7 +283,7 @@ public void testReplicasToFixMisreplicationMaxReplicaPerRack() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 2); List racks = dummyPlacementPolicy.racks; - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 2, 4, 6, 8) .map(list::get).collect(Collectors.toList()); List replicas = @@ -297,7 +302,7 @@ public void testReplicasToFixMisreplicationMaxReplicaPerRack() { public void testReplicasWithoutMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); List replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); Map replicas = @@ -314,7 +319,7 @@ public void testReplicasWithoutMisreplication() { public void testReplicasToRemoveWithOneOverreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet( HddsTestUtils.getReplicasWithReplicaIndex( ContainerID.valueOf(1), CLOSED, 0, 0, 0, list.subList(1, 6))); @@ -335,7 +340,7 @@ public void testReplicasToRemoveWithOneOverreplication() { public void testReplicasToRemoveWithTwoOverreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet( HddsTestUtils.getReplicasWithReplicaIndex( @@ -356,7 +361,7 @@ public void testReplicasToRemoveWithTwoOverreplication() { public void testReplicasToRemoveWith2CountPerUniqueReplica() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 3); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet( HddsTestUtils.getReplicasWithReplicaIndex( @@ -382,7 +387,7 @@ public void testReplicasToRemoveWith2CountPerUniqueReplica() { public void testReplicasToRemoveWithoutReplicaIndex() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 3); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet(HddsTestUtils.getReplicas( ContainerID.valueOf(1), CLOSED, 0, list.subList(0, 5))); @@ -402,7 +407,7 @@ public void testReplicasToRemoveWithoutReplicaIndex() { public void testReplicasToRemoveWithOverreplicationWithinSameRack() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 3); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet( HddsTestUtils.getReplicasWithReplicaIndex( @@ -441,7 +446,7 @@ public void testReplicasToRemoveWithOverreplicationWithinSameRack() { public void testReplicasToRemoveWithNoOverreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = nodeManager.getAllNodes(); + List list = getAllNodes(nodeManager); Set replicas = Sets.newHashSet( HddsTestUtils.getReplicasWithReplicaIndex( ContainerID.valueOf(1), CLOSED, 0, 0, 0, list.subList(1, 6))); @@ -602,8 +607,9 @@ private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { when(node.getNetworkFullPath()).thenReturn(String.valueOf(i)); return node; }).collect(Collectors.toList()); - final List datanodeDetails = nodeManager.getAllNodes(); - rackMap = datanodeRackMap.entrySet().stream() + final List datanodeDetails = getAllNodes(nodeManager); + rackMap = datanodeRackMap + .entrySet().stream() .collect(Collectors.toMap( entry -> datanodeDetails.get(entry.getKey()), entry -> racks.get(entry.getValue()))); 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 57d38ece3dd6..8ddcb9d164b3 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 @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; @@ -105,7 +106,7 @@ public class MockNodeManager implements NodeManager { private final List healthyNodes; private final List staleNodes; private final List deadNodes; - private final Map nodeMetricMap; + private final Map nodeMetricMap = new TreeMap<>(); private final SCMNodeStat aggregateStat; private final Map>> commandMap; private Node2PipelineMap node2PipelineMap; @@ -121,7 +122,6 @@ public class MockNodeManager implements NodeManager { this.healthyNodes = new LinkedList<>(); this.staleNodes = new LinkedList<>(); this.deadNodes = new LinkedList<>(); - this.nodeMetricMap = new HashMap<>(); this.node2PipelineMap = new Node2PipelineMap(); this.node2ContainerMap = new NodeStateMap(); this.dnsToUuidMap = new ConcurrentHashMap<>(); @@ -250,7 +250,7 @@ private void populateNodeMetric(DatanodeDetails datanodeDetails, int x) { */ @Override public List getNodes(NodeStatus status) { - return getNodes(status.getOperationalState(), status.getHealth()); + return getDatanodeDetails(status.getOperationalState(), status.getHealth()); } /** @@ -261,7 +261,15 @@ public List getNodes(NodeStatus status) { * @return List of Datanodes that are Heartbeating SCM. */ @Override - public List getNodes( + public List getNodes(HddsProtos.NodeOperationalState opState, HddsProtos.NodeState nodestate) { + final List details = getDatanodeDetails(opState, nodestate); + if (details == null) { + return null; + } + return details.stream().map(this::getDatanodeInfo).collect(Collectors.toList()); + } + + private List getDatanodeDetails( HddsProtos.NodeOperationalState opState, HddsProtos.NodeState nodestate) { if (nodestate == HEALTHY) { // mock storage reports for SCMCommonPlacementPolicy.hasEnoughSpace() @@ -322,22 +330,17 @@ public int getNodeCount(NodeStatus status) { @Override public int getNodeCount( HddsProtos.NodeOperationalState opState, HddsProtos.NodeState nodestate) { - List nodes = getNodes(opState, nodestate); + List nodes = getDatanodeDetails(opState, nodestate); if (nodes != null) { return nodes.size(); } return 0; } - /** - * Get all datanodes known to SCM. - * - * @return List of DatanodeDetails known to SCM. - */ @Override - public List getAllNodes() { + public List getAllNodes() { // mock storage reports for TestDiskBalancer - List healthyNodesWithInfo = new ArrayList<>(); + List healthyNodesWithInfo = new ArrayList<>(); for (Map.Entry entry: nodeMetricMap.entrySet()) { NodeStatus nodeStatus = NodeStatus.inServiceHealthy(); @@ -399,7 +402,7 @@ public Map getNodeStats() { public List getMostOrLeastUsedDatanodes( boolean mostUsed) { List datanodeDetailsList = - getNodes(NodeOperationalState.IN_SERVICE, HEALTHY); + getDatanodeDetails(NodeOperationalState.IN_SERVICE, HEALTHY); if (datanodeDetailsList == null) { return new ArrayList<>(); } @@ -428,9 +431,11 @@ public DatanodeUsageInfo getUsageInfo(DatanodeDetails datanodeDetails) { return new DatanodeUsageInfo(datanodeDetails, stat); } - @Override @Nullable public DatanodeInfo getDatanodeInfo(DatanodeDetails dd) { + if (dd instanceof DatanodeInfo) { + return (DatanodeInfo) dd; + } if (nodeMetricMap.get(dd) == null) { return null; } @@ -897,9 +902,9 @@ public List> getCommandQueue(DatanodeID dnID) { } @Override - public DatanodeDetails getNode(DatanodeID id) { + public DatanodeInfo getNode(DatanodeID id) { Node node = clusterMap.getNode(NetConstants.DEFAULT_RACK + "/" + id); - return node == null ? null : (DatanodeDetails)node; + return node == null ? null : getDatanodeInfo((DatanodeDetails)node); } @Override 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 f2da8fd2878b..73a9608401ec 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 @@ -17,7 +17,6 @@ package org.apache.hadoop.hdds.scm.container; -import jakarta.annotation.Nullable; import java.io.IOException; import java.util.Collections; import java.util.HashSet; @@ -193,7 +192,7 @@ public List getNodes(NodeStatus nodeStatus) { } @Override - public List getNodes( + public List getNodes( NodeOperationalState opState, HddsProtos.NodeState health) { return null; } @@ -210,8 +209,8 @@ public int getNodeCount(NodeOperationalState opState, } @Override - public List getAllNodes() { - return null; + public List getAllNodes() { + return Collections.emptyList(); } @Override @@ -243,12 +242,6 @@ public DatanodeUsageInfo getUsageInfo(DatanodeDetails datanodeDetails) { return null; } - @Override - @Nullable - public DatanodeInfo getDatanodeInfo(DatanodeDetails dn) { - return null; - } - @Override public void recordPendingAllocationForDatanode(DatanodeID datanodeID, ContainerID containerID) { } @@ -360,7 +353,7 @@ public List> getCommandQueue(DatanodeID dnID) { } @Override - public DatanodeDetails getNode(DatanodeID id) { + public DatanodeInfo getNode(DatanodeID id) { return null; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/reconciliation/TestReconcileContainerEventHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/reconciliation/TestReconcileContainerEventHandler.java index 49fab0afe225..e4ca940cf561 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/reconciliation/TestReconcileContainerEventHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/reconciliation/TestReconcileContainerEventHandler.java @@ -39,7 +39,6 @@ import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; -import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.DatanodeID; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State; @@ -54,6 +53,7 @@ import org.apache.hadoop.hdds.scm.container.reconciliation.ReconciliationEligibilityHandler.EligibilityResult; import org.apache.hadoop.hdds.scm.container.reconciliation.ReconciliationEligibilityHandler.Result; import org.apache.hadoop.hdds.scm.ha.SCMContext; +import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.server.events.EventPublisher; import org.apache.hadoop.ozone.protocol.commands.CommandForDatanode; import org.apache.hadoop.ozone.protocol.commands.SCMCommand; @@ -300,7 +300,7 @@ private Set addReplicasToContainer(State... replicaStates) thr // If no states are specified, replica list will be empty. Set replicas = new HashSet<>(); try (MockNodeManager nodeManager = new MockNodeManager(true, replicaStates.length)) { - List nodes = nodeManager.getAllNodes(); + List nodes = nodeManager.getAllNodes(); for (int i = 0; i < replicaStates.length; i++) { replicas.addAll(HddsTestUtils.getReplicas(CONTAINER_ID, replicaStates[i], nodes.get(i))); } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java index 341bbedf42d9..2871af693d70 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/node/TestDecommissionAndMaintenance.java @@ -213,7 +213,7 @@ public void testNodeWithOpenPipelineCanBeDecommissionedAndRecommissioned() waitForDnToReachOpState(nm, toDecommission, DECOMMISSIONED); // Ensure one node transitioned to DECOMMISSIONING - List decomNodes = nm.getNodes( + List decomNodes = nm.getNodes( DECOMMISSIONED, HEALTHY); assertEquals(1, decomNodes.size()); @@ -325,7 +325,7 @@ public void testInsufficientNodesCannotBeDecommissioned() toDecommission.get(3).getIpAddress(), toDecommission.get(4).getIpAddress()), false); // Ensure no nodes transitioned to DECOMMISSIONING or DECOMMISSIONED - List decomNodes = nm.getNodes( + List decomNodes = nm.getNodes( DECOMMISSIONING, HEALTHY); assertEquals(0, decomNodes.size()); @@ -717,7 +717,7 @@ public void testInsufficientNodesCannotBePutInMaintenance() getDNHostAndPort(toMaintenance.get(5))), 0, false); // Ensure no nodes transitioned to MAINTENANCE - List maintenanceNodes = nm.getNodes( + List maintenanceNodes = nm.getNodes( ENTERING_MAINTENANCE, HEALTHY); assertEquals(0, maintenanceNodes.size()); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancer.java index 6df761e45f5a..3435886259ed 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancer.java @@ -221,7 +221,7 @@ public void testDatanodeDiskBalancerStatus() throws IOException, InterruptedExce } // Query status from remaining IN_SERVICE DNs and verify they still show RUNNING - List inServiceDatanodes = nm.getNodes(IN_SERVICE, HddsProtos.NodeState.HEALTHY); + final List inServiceDatanodes = nm.getNodes(IN_SERVICE, HddsProtos.NodeState.HEALTHY); statusProtoList.clear(); for (DatanodeDetails dn : inServiceDatanodes) { try (DiskBalancerProtocol proxy = getDiskBalancerProxy(dn)) { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancerDuringDecommissionAndMaintenance.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancerDuringDecommissionAndMaintenance.java index 04c6273b8ae1..353240f89d15 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancerDuringDecommissionAndMaintenance.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/node/TestDiskBalancerDuringDecommissionAndMaintenance.java @@ -138,13 +138,6 @@ private DiskBalancerProtocol getDiskBalancerProxy(DatanodeDetails dn) throws IOE return new DiskBalancerProtocolClientSideTranslatorPB(nodeAddr, user, conf); } - /** - * Helper method to get all IN_SERVICE datanodes. - */ - private List getInServiceDatanodes(NodeManager nm) { - return nm.getNodes(IN_SERVICE, HddsProtos.NodeState.HEALTHY); - } - /** * Helper method to query DiskBalancer info from all IN_SERVICE datanodes. * Similar to --in-service-datanodes option in CLI. @@ -152,7 +145,7 @@ private List getInServiceDatanodes(NodeManager nm) { private List queryAllInServiceDatanodes( DiskBalancerQuery query) throws IOException { NodeManager nm = cluster.getStorageContainerManager().getScmNodeManager(); - List inServiceDatanodes = getInServiceDatanodes(nm); + final List inServiceDatanodes = nm.getNodes(IN_SERVICE, HddsProtos.NodeState.HEALTHY); List results = new ArrayList<>(); for (DatanodeDetails dn : inServiceDatanodes) { diff --git a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/PipelineSyncTask.java b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/PipelineSyncTask.java index f8985a0a8671..5e145625e141 100644 --- a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/PipelineSyncTask.java +++ b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/PipelineSyncTask.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; @@ -109,13 +110,14 @@ protected void runTask() throws IOException, NodeNotFoundException { */ private void syncOperationalStateOnDeadNodes() throws IOException, NodeNotFoundException { - List deadNodesOnRecon = nodeManager.getNodes(null, DEAD); + final Set deadNodesOnRecon = nodeManager.getNodes(null, DEAD).stream() + .map(info -> info.getID().getID()) + .collect(Collectors.toSet()); if (!deadNodesOnRecon.isEmpty()) { List scmNodes = scmClient.getNodes(); List filteredScmNodes = scmNodes.stream() - .filter(n -> deadNodesOnRecon.contains( - DatanodeDetails.getFromProtoBuf(n.getNodeID()))) + .filter(n -> deadNodesOnRecon.contains(n.getNodeID().getUuid())) .collect(Collectors.toList()); for (Node deadNode : filteredScmNodes) { diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java index 78f9bbceafcd..9a9618dc3d24 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconIncrementalContainerReportHandler.java @@ -46,7 +46,9 @@ import org.apache.hadoop.hdds.scm.ha.SCMContext; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl; +import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.node.NodeStatus; import org.apache.hadoop.hdds.scm.node.SCMNodeManager; import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException; import org.apache.hadoop.hdds.scm.server.SCMDatanodeHeartbeatDispatcher.IncrementalContainerReportFromDatanode; @@ -133,11 +135,12 @@ public void testProcessICRStateMismatch() ReconContainerManager containerManager = getContainerManager(); containerManager.addNewContainer(containerWithPipeline); - DatanodeDetails datanodeDetails = - containerWithPipeline.getPipeline().getFirstNode(); + DatanodeInfo datanodeInfo = new DatanodeInfo( + containerWithPipeline.getPipeline().getFirstNode(), + NodeStatus.inServiceHealthy(), null, 1000); NodeManager nodeManagerMock = mock(NodeManager.class); when(nodeManagerMock.getNode(any(DatanodeID.class))) - .thenReturn(datanodeDetails); + .thenReturn(datanodeInfo); IncrementalContainerReportFromDatanode reportMock = mock(IncrementalContainerReportFromDatanode.class); when(reportMock.getDatanodeDetails()) @@ -145,7 +148,7 @@ public void testProcessICRStateMismatch() IncrementalContainerReportProto containerReport = getIncrementalContainerReportProto(containerID, state, - datanodeDetails.getUuidString()); + datanodeInfo.getUuidString()); when(reportMock.getReport()).thenReturn(containerReport); ReconIncrementalContainerReportHandler reconIcr = new ReconIncrementalContainerReportHandler(nodeManagerMock, diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconNodeManager.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconNodeManager.java index e0eb2f020ccb..838c4308de2c 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconNodeManager.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/scm/TestReconNodeManager.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl; +import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException; import org.apache.hadoop.hdds.server.events.EventQueue; import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager; @@ -229,7 +230,7 @@ public void testUpdateNodeOperationalStateFromScm() throws Exception { reconNodeManager.updateNodeOperationalStateFromScm(node, datanodeDetails); assertEquals(DECOMMISSIONING, reconNodeManager .getNode(datanodeDetails.getID()).getPersistedOpState()); - List nodes = + List nodes = reconNodeManager.getNodes(DECOMMISSIONING, null); assertEquals(1, nodes.size()); assertEquals(datanodeDetails.getUuid(), nodes.get(0).getUuid());