From f6b0d03f75337fd4216f6393a255c34493f5bb51 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 28 Nov 2022 02:14:34 -0800 Subject: [PATCH 01/23] HDDS-7492. Placement Policy Interface changes to handle misreplication changes --- .../hadoop/hdds/scm/PlacementPolicy.java | 22 ++- .../hdds/scm/SCMCommonPlacementPolicy.java | 154 +++++++++++++++++- .../SCMContainerPlacementCapacity.java | 11 +- .../SCMContainerPlacementRackAware.java | 11 +- .../SCMContainerPlacementRackScatter.java | 11 +- .../SCMContainerPlacementRandom.java | 13 +- .../scm/pipeline/PipelinePlacementPolicy.java | 11 +- .../scm/pipeline/RatisPipelineProvider.java | 3 +- .../scm/TestSCMCommonPlacementPolicy.java | 79 ++++++++- .../TestContainerPlacementFactory.java | 26 ++- .../TestSCMContainerPlacementCapacity.java | 7 +- .../TestSCMContainerPlacementRackAware.java | 19 ++- .../TestSCMContainerPlacementRackScatter.java | 14 +- .../TestSCMContainerPlacementRandom.java | 19 ++- .../replication/ReplicationTestUtil.java | 9 +- .../pipeline/TestPipelinePlacementPolicy.java | 29 ++-- .../placement/TestContainerPlacement.java | 15 +- .../recon/fsck/TestContainerHealthTask.java | 24 ++- 18 files changed, 398 insertions(+), 79 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index b240e5c3b789..c630118a13c4 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -22,12 +22,14 @@ import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.Set; /** * A PlacementPolicy support choosing datanodes to build * pipelines or containers with specified constraints. */ -public interface PlacementPolicy { +public interface PlacementPolicy { default List chooseDatanodes( List excludedNodes, @@ -60,9 +62,23 @@ List chooseDatanodes(List usedNodes, * Given a list of datanode and the number of replicas required, return * a PlacementPolicyStatus object indicating if the container meets the * placement policy - ie is it on the correct number of racks, etc. - * @param dns List of datanodes holding a replica of the container + * @param dns List of replica holding a replica of the container * @param replicas The expected number of replicas */ ContainerPlacementStatus validateContainerPlacement( - List dns, int replicas); + List dns, int replicas); + Map replicasToCopy(Set replicas, + int expectedCountPerUniqueReplica, + int expectedUniqueGroups); + + Set replicasToRemove(Set replicas, + int expectedCountPerUniqueReplica, + int expectedUniqueGroups); + + + /** Gets the group of from the datanode based on the placement. + * @param dn + * @return PlacementGroup + */ + PlacementGroup getPlacementGroup(DatanodeDetails dn); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 5f7e944235c2..602a2c18f0cc 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -20,19 +20,30 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.PriorityQueue; +import java.util.Queue; import java.util.Random; +import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import com.google.common.base.Preconditions; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementStatusDefault; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.scm.node.NodeStatus; @@ -46,7 +57,8 @@ * for all basic placement policies, acts as the repository of helper * functions which are common to placement policies. */ -public abstract class SCMCommonPlacementPolicy implements PlacementPolicy { +public abstract class SCMCommonPlacementPolicy implements + PlacementPolicy { @VisibleForTesting static final Logger LOG = LoggerFactory.getLogger(SCMCommonPlacementPolicy.class); @@ -57,6 +69,8 @@ public abstract class SCMCommonPlacementPolicy implements PlacementPolicy { private final ConfigurationSource conf; private final boolean shouldRemovePeers; + private Function replicaIdentifierFunction; + /** * Return for replication factor 1 containers where the placement policy * is always met, or not met (zero replicas available) rather than creating a @@ -75,10 +89,12 @@ public abstract class SCMCommonPlacementPolicy implements PlacementPolicy { * @param conf Configuration class. */ public SCMCommonPlacementPolicy(NodeManager nodeManager, - ConfigurationSource conf) { + ConfigurationSource conf, + Function replicaIdentifierFunction) { this.nodeManager = nodeManager; this.conf = conf; this.shouldRemovePeers = ScmUtils.shouldRemovePeers(conf); + this.replicaIdentifierFunction = replicaIdentifierFunction; } /** @@ -379,7 +395,7 @@ public ContainerPlacementStatus validateContainerPlacement( // leafLevel - 1 is the rack count numRacks = topology.getNumOfNodes(maxLevel - 1); final long currentRackCount = dns.stream() - .map(d -> topology.getAncestor(d, 1)) + .map(this::getPlacementGroup) .distinct() .count(); @@ -426,4 +442,136 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } return false; } + @Override + public Set replicasToRemove(Set replicas, + int expectedCountPerUniqueReplica, int expectedUniqueGroups) { + Map> replicaIdMap = new HashMap<>(); + Map>> placementGroupReplicaIdMap + = new HashMap<>(); + Map placementGroupCntMap = new HashMap<>(); + for (ContainerReplica replica:replicas) { + RID replicaId = replicaIdentifierFunction.apply(replica); + Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); + if (!replicaIdMap.containsKey(replicaId)) { + replicaIdMap.put(replicaId, Sets.newHashSet()); + } + if (!placementGroupReplicaIdMap.containsKey(placementGroup)) { + placementGroupReplicaIdMap.put(placementGroup, Maps.newHashMap()); + } + placementGroupCntMap.compute(placementGroup, + (group, cnt) -> (cnt == null ? 0 : cnt) + 1); + replicaIdMap.get(replicaId).add(replica); + Map> placementGroupReplicaIDMap = + placementGroupReplicaIdMap.get(placementGroup); + placementGroupReplicaIDMap.compute(replicaId, + (rid, placementGroupReplicas) -> { + if (placementGroupReplicas == null) { + placementGroupReplicas = Sets.newHashSet(); + } + placementGroupReplicas.add(replica); + return placementGroupReplicas; + }); + } + + Set replicasToRemove = new HashSet<>(); + List sortedRIDList = replicaIdMap.keySet().stream().sorted((o1, o2) -> + Integer.compare(replicaIdMap.get(o2).size(), + replicaIdMap.get(o1).size())).collect(Collectors.toList()); + for (RID rid : sortedRIDList) { + Queue pq = new PriorityQueue<>((o1, o2) -> + Integer.compare(placementGroupCntMap.get(o2), + placementGroupCntMap.get(o1))); + pq.addAll(placementGroupReplicaIdMap.entrySet() + .stream() + .filter(nodeMapEntry -> nodeMapEntry.getValue().containsKey(rid)) + .map(Map.Entry::getKey) + .collect(Collectors.toList())); + + while (replicaIdMap.get(rid).size() > expectedCountPerUniqueReplica) { + Node rack = pq.poll(); + Set replicaSet = + placementGroupReplicaIdMap.get(rack).get(rid); + if (replicaSet.size() > 0) { + ContainerReplica r = replicaSet.stream().findFirst().get(); + replicasToRemove.add(r); + replicaSet.remove(r); + replicaIdMap.get(rid).remove(r); + placementGroupCntMap.compute(rack, + (group, cnt) -> (cnt == null ? 0 : cnt) - 1); + if (replicaSet.size() == 0) { + placementGroupReplicaIdMap.get(rack).remove(rid); + } else { + pq.add(rack); + } + } + } + } + return replicasToRemove; + } + + @Override + public Map replicasToCopy( + Set replicas, int expectedCountPerUniqueReplicas, + int expectedUniqueGroups) { + Map replicaIdMap = new HashMap<>(); + Map replicaIdCntMap = new HashMap<>(); + Map> placementGroupReplicaIdMap + = new HashMap<>(); + for (ContainerReplica replica:replicas) { + RID replicaId = replicaIdentifierFunction.apply(replica); + Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); + if (!replicaIdMap.containsKey(replicaId)) { + replicaIdMap.put(replicaId, replica); + } + replicaIdCntMap.compute(replicaId, (rid, cnt) -> + (cnt == null ? 0 : cnt) + 1); + placementGroupReplicaIdMap.compute(placementGroup, + (rid, placementGroupReplicas) -> { + if (placementGroupReplicas == null) { + placementGroupReplicas = Sets.newHashSet(); + } + placementGroupReplicas.add(replica); + return placementGroupReplicas; + }); + } + int misreplicationCnt = Math.max(getRequiredRackCount( + expectedUniqueGroups * expectedCountPerUniqueReplicas) + - placementGroupReplicaIdMap.size(), 0); + Map copyRIDMap = new HashMap<>(); + for (RID rid : replicaIdMap.keySet()) { + if (replicaIdCntMap.get(rid) < expectedCountPerUniqueReplicas) { + int additionalReplica = expectedCountPerUniqueReplicas - + replicaIdCntMap.get(rid); + copyRIDMap.compute(rid, (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) + + additionalReplica); + misreplicationCnt -= additionalReplica; + } + } + + for (Set replicaSet: placementGroupReplicaIdMap + .values()) { + if (misreplicationCnt > 0 && replicaSet.size() > 1) { + Map cntMap = replicaSet.stream() + .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) + .collect(Collectors.groupingBy(replicaIdentifierFunction, + Collectors.counting())); + for (RID rid : cntMap.keySet()) { + int additionalReplica = Math.toIntExact(cntMap.get(rid)); + copyRIDMap.compute(rid, (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) + + additionalReplica); + misreplicationCnt -= additionalReplica; + } + } + } + return copyRIDMap.entrySet().stream() + .collect(Collectors.toMap(e -> replicaIdMap.get(e.getKey()), + Map.Entry::getValue)); + } + + @Override + public Node getPlacementGroup(DatanodeDetails dn) { + return nodeManager.getClusterNetworkTopologyMap().getAncestor(dn, 1); + } + + } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java index 3ac5bcbe5522..8f618ea47155 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java @@ -18,10 +18,12 @@ package org.apache.hadoop.hdds.scm.container.placement.algorithms; import java.util.List; +import java.util.function.Function; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -66,8 +68,8 @@ * little or no work and the cluster will achieve a balanced distribution * over time. */ -public final class SCMContainerPlacementCapacity - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementCapacity + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementCapacity.class); @@ -81,8 +83,9 @@ public final class SCMContainerPlacementCapacity */ public SCMContainerPlacementCapacity(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics) { - super(nodeManager, conf); + final boolean fallback, final SCMContainerPlacementMetrics metrics, + final Function replicaIdentifierFunction) { + super(nodeManager, conf, replicaIdentifierFunction); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java index 4ee7408fbb5f..c2c6c528f8c8 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java @@ -22,6 +22,7 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -33,6 +34,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -47,8 +49,8 @@ * recommend to use this if the network topology has more layers. *

*/ -public final class SCMContainerPlacementRackAware - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementRackAware + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRackAware.class); @@ -72,8 +74,9 @@ public final class SCMContainerPlacementRackAware */ public SCMContainerPlacementRackAware(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics) { - super(nodeManager, conf); + final boolean fallback, final SCMContainerPlacementMetrics metrics, + final Function containerReplicaFunction) { + super(nodeManager, conf, containerReplicaFunction); this.networkTopology = networkTopology; this.fallback = fallback; this.metrics = metrics; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java index eff2dc86c426..a9f01425f631 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java @@ -22,6 +22,7 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.Node; @@ -36,6 +37,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -50,8 +52,8 @@ * recommend to use this if the network topology has more layers. *

*/ -public final class SCMContainerPlacementRackScatter - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementRackScatter + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRackScatter.class); @@ -71,8 +73,9 @@ public final class SCMContainerPlacementRackScatter */ public SCMContainerPlacementRackScatter(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - boolean fallback, final SCMContainerPlacementMetrics metrics) { - super(nodeManager, conf); + boolean fallback, final SCMContainerPlacementMetrics metrics, + Function replicaIdentifierFunction) { + super(nodeManager, conf, replicaIdentifierFunction); this.networkTopology = networkTopology; this.metrics = metrics; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java index cdfd57d1d09b..351ff78f9f67 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java @@ -21,14 +21,17 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.scm.PlacementPolicy; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.List; +import java.util.function.Function; /** * Container placement policy that randomly chooses healthy datanodes. @@ -39,8 +42,9 @@ * Balancer will need to support containers as a feature before this class * can be practically used. */ -public final class SCMContainerPlacementRandom extends SCMCommonPlacementPolicy - implements PlacementPolicy { +public final class SCMContainerPlacementRandom extends + SCMCommonPlacementPolicy implements + PlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRandom.class); @@ -53,8 +57,9 @@ public final class SCMContainerPlacementRandom extends SCMCommonPlacementPolicy */ public SCMContainerPlacementRandom(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics) { - super(nodeManager, conf); + final boolean fallback, final SCMContainerPlacementMetrics metrics, + Function replicaIdentifierFunction) { + super(nodeManager, conf, replicaIdentifierFunction); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java index befc0543a357..bf3814c5f28e 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.node.NodeManager; @@ -37,6 +38,7 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -49,7 +51,8 @@ * 4. Choose an anchor node among the viable nodes. * 5. Choose other nodes around the anchor node based on network topology */ -public final class PipelinePlacementPolicy extends SCMCommonPlacementPolicy { +public final class PipelinePlacementPolicy extends + SCMCommonPlacementPolicy { @VisibleForTesting static final Logger LOG = LoggerFactory.getLogger(PipelinePlacementPolicy.class); @@ -73,9 +76,9 @@ public final class PipelinePlacementPolicy extends SCMCommonPlacementPolicy { * @param conf Configuration */ public PipelinePlacementPolicy(final NodeManager nodeManager, - final PipelineStateManager stateManager, - final ConfigurationSource conf) { - super(nodeManager, conf); + final PipelineStateManager stateManager, final ConfigurationSource conf, + final Function replicaIdentifierFunction) { + super(nodeManager, conf, replicaIdentifierFunction); this.nodeManager = nodeManager; this.conf = conf; this.stateManager = stateManager; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java index 43b2e01c9140..9b70f17f9876 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java @@ -78,7 +78,8 @@ public RatisPipelineProvider(NodeManager nodeManager, this.eventPublisher = eventPublisher; this.scmContext = scmContext; this.placementPolicy = - new PipelinePlacementPolicy(nodeManager, stateManager, conf); + new PipelinePlacementPolicy<>(nodeManager, stateManager, conf, + ContainerReplica::getReplicaIndex); this.pipelineNumberLimit = conf.getInt( ScmConfigKeys.OZONE_SCM_RATIS_PIPELINE_LIMIT, ScmConfigKeys.OZONE_SCM_RATIS_PIPELINE_LIMIT_DEFAULT); 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 d11ca91b369e..672fdae573c3 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 @@ -21,8 +21,11 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.exceptions.SCMException; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.scm.node.NodeStatus; import org.apache.hadoop.ozone.container.common.SCMTestUtils; @@ -30,9 +33,15 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; /** * Test functions of SCMCommonPlacementPolicy. @@ -59,17 +68,83 @@ public void testGetResultSet() throws SCMException { Assertions.assertNotEquals(1, resultSet.size()); } - private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { + @Test + public void testReplicasToCopy() { + DummyPlacementPolicy dummyPlacementPolicy = + new DummyPlacementPolicy(nodeManager, conf); + List list = + nodeManager.getNodes(NodeStatus.inServiceHealthy()); + Set replicas = + IntStream.range(1, 6).mapToObj(i -> + ContainerReplica.newBuilder() + .setContainerID(new ContainerID(1)) + .setContainerState(CLOSED) + .setReplicaIndex(i) + .setDatanodeDetails(list.get(i)).build()) + .collect(Collectors.toSet()); + + + Map replicasToCopy = + dummyPlacementPolicy.replicasToCopy(replicas, + 2, 5); + Assertions.assertEquals(replicas, replicasToCopy.keySet()); + Assertions.assertTrue(replicasToCopy.values().stream() + .allMatch(i -> i == 1)); + } + + @Test + public void testReplicasToRemove() { + DummyPlacementPolicy dummyPlacementPolicy = + new DummyPlacementPolicy(nodeManager, conf); + List list = + nodeManager.getNodes(NodeStatus.inServiceHealthy()); + Set replicas = + IntStream.range(1, 6).mapToObj(i -> + ContainerReplica.newBuilder() + .setContainerID(new ContainerID(1)) + .setContainerState(CLOSED) + .setReplicaIndex(i) + .setDatanodeDetails(list.get(i)).build()) + .collect(Collectors.toSet()); + ContainerReplica replica = ContainerReplica.newBuilder() + .setContainerID(new ContainerID(1)) + .setContainerState(CLOSED) + .setReplicaIndex(1) + .setDatanodeDetails(list.get(7)).build(); + replicas.add(replica); + + Set replicasToRemove = + dummyPlacementPolicy.replicasToRemove(replicas, + 1, 5); + Assertions.assertEquals(replicasToRemove.size(), 1); + Assertions.assertEquals(replicasToRemove.stream().findFirst().get(), + replica); + } + + private static class DummyPlacementPolicy extends + SCMCommonPlacementPolicy { + private Map dns; DummyPlacementPolicy( NodeManager nodeManager, ConfigurationSource conf) { - super(nodeManager, conf); + super(nodeManager, conf, ContainerReplica::getReplicaIndex); + dns = new HashMap<>(); + List datanodeDetails = + nodeManager.getNodes(NodeStatus.inServiceHealthy()); + for (int idx = 0; idx < datanodeDetails.size(); idx++) { + dns.put(datanodeDetails.get(idx), datanodeDetails.get(idx % 5)); + } } @Override public DatanodeDetails chooseNode(List healthyNodes) { return healthyNodes.get(0); } + + @Override + public Node getPlacementGroup(DatanodeDetails dn) { + return dns.get(dn); + } } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index c35cb2b4551f..9083107e6620 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -20,6 +20,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.StorageUnit; @@ -31,9 +33,11 @@ import org.apache.hadoop.hdds.scm.PlacementPolicy; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.net.NodeSchema; import org.apache.hadoop.hdds.scm.net.NodeSchemaManager; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; @@ -176,7 +180,8 @@ public void testECPolicy() throws IOException { /** * A dummy container placement implementation for test. */ - public static class DummyImpl implements PlacementPolicy { + public static class DummyImpl implements + PlacementPolicy { @Override public List chooseDatanodes( List usedNodes, @@ -191,6 +196,25 @@ public List chooseDatanodes( validateContainerPlacement(List dns, int replicas) { return new ContainerPlacementStatusDefault(1, 1, 1); } + + @Override + public Map replicasToCopy( + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { + return null; + } + + @Override + public Set replicasToRemove( + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { + return null; + } + + @Override + public Node getPlacementGroup(DatanodeDetails dn) { + return dn.getParent(); + } } @Test diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java index fb169c7feac5..6352f088199d 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java @@ -29,6 +29,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; @@ -120,9 +121,9 @@ public void chooseDatanodes() throws SCMException { .orElse(null); }); - SCMContainerPlacementCapacity scmContainerPlacementRandom = - new SCMContainerPlacementCapacity(mockNodeManager, conf, null, true, - null); + SCMContainerPlacementCapacity scmContainerPlacementRandom = + new SCMContainerPlacementCapacity<>(mockNodeManager, conf, null, true, + null, ContainerReplica::getReplicaIndex); List existingNodes = new ArrayList<>(); existingNodes.add(datanodes.get(0)); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java index 3342c402d7aa..56a1976fea2a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -75,9 +76,9 @@ public class TestSCMContainerPlacementRackAware { private final List datanodes = new ArrayList<>(); private final List dnInfos = new ArrayList<>(); // policy with fallback capability - private SCMContainerPlacementRackAware policy; + private SCMContainerPlacementRackAware policy; // policy prohibit fallback - private SCMContainerPlacementRackAware policyNoFallback; + private SCMContainerPlacementRackAware policyNoFallback; // node storage capacity private static final long STORAGE_CAPACITY = 100L; private SCMContainerPlacementMetrics metrics; @@ -180,10 +181,10 @@ private void setup(int datanodeCount) { .thenReturn(cluster); // create placement policy instances - policy = new SCMContainerPlacementRackAware( - nodeManager, conf, cluster, true, metrics); - policyNoFallback = new SCMContainerPlacementRackAware( - nodeManager, conf, cluster, false, metrics); + policy = new SCMContainerPlacementRackAware<>(nodeManager, conf, cluster, + true, metrics, ContainerReplica::getReplicaIndex); + policyNoFallback = new SCMContainerPlacementRackAware<>(nodeManager, conf, + cluster, false, metrics, ContainerReplica::getReplicaIndex); } @BeforeEach @@ -481,9 +482,9 @@ public void testDatanodeWithDefaultNetworkLocation(int datanodeCount) // choose nodes to host 3 replica int nodeNum = 3; - SCMContainerPlacementRackAware newPolicy = - new SCMContainerPlacementRackAware(nodeManager, conf, clusterMap, true, - metrics); + SCMContainerPlacementRackAware newPolicy = + new SCMContainerPlacementRackAware<>(nodeManager, conf, clusterMap, + true, metrics, ContainerReplica::getReplicaIndex); List datanodeDetails = newPolicy.chooseDatanodes(null, null, nodeNum, 0, 15); Assertions.assertEquals(nodeNum, datanodeDetails.size()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java index 46bf8031effc..604571a89fc8 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java @@ -25,6 +25,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -79,7 +80,7 @@ public class TestSCMContainerPlacementRackScatter { private final List datanodes = new ArrayList<>(); private final List dnInfos = new ArrayList<>(); // policy with fallback capability - private SCMContainerPlacementRackScatter policy; + private SCMContainerPlacementRackScatter policy; // node storage capacity private static final long STORAGE_CAPACITY = 100L; private SCMContainerPlacementMetrics metrics; @@ -196,8 +197,9 @@ private void setup(int datanodeCount, int nodesPerRack) { .thenReturn(cluster); // create placement policy instances - policy = new SCMContainerPlacementRackScatter( - nodeManager, conf, cluster, true, metrics); + policy = new SCMContainerPlacementRackScatter<>( + nodeManager, conf, cluster, true, metrics, + ContainerReplica::getReplicaIndex); } @BeforeEach @@ -465,9 +467,9 @@ public void testDatanodeWithDefaultNetworkLocation(int datanodeCount) // choose nodes to host 5 replica int nodeNum = 5; - SCMContainerPlacementRackScatter newPolicy = - new SCMContainerPlacementRackScatter(nodeManager, conf, clusterMap, - true, metrics); + SCMContainerPlacementRackScatter newPolicy = + new SCMContainerPlacementRackScatter<>(nodeManager, conf, clusterMap, + true, metrics, ContainerReplica::getReplicaIndex); List datanodeDetails = newPolicy.chooseDatanodes(null, null, nodeNum, 0, 15); Assertions.assertEquals(nodeNum, datanodeDetails.size()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java index b7c152b06d35..bab6d7c1310e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java @@ -28,6 +28,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.scm.node.NodeManager; @@ -89,9 +90,9 @@ public void chooseDatanodes() throws SCMException { when(mockNodeManager.getNodes(NodeStatus.inServiceHealthy())) .thenReturn(new ArrayList<>(datanodes)); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, - null); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, + null, ContainerReplica::getReplicaIndex); List existingNodes = new ArrayList<>(); existingNodes.add(datanodes.get(0)); @@ -132,9 +133,9 @@ public void testPlacementPolicySatisified() { } NodeManager mockNodeManager = Mockito.mock(NodeManager.class); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, - null); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, + null, ContainerReplica::getReplicaIndex); ContainerPlacementStatus status = scmContainerPlacementRandom.validateContainerPlacement(datanodes, 3); assertTrue(status.isPolicySatisfied()); @@ -211,9 +212,9 @@ public void testIsValidNode() throws SCMException { when(mockNodeManager.getNodeByUuid(datanodes.get(2).getUuidString())) .thenReturn(datanodes.get(2)); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, - null); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, + null, ContainerReplica::getReplicaIndex); Assertions.assertTrue( scmContainerPlacementRandom.isValidNode(datanodes.get(0), 15L, 15L)); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java index f87d2851586a..132bfda33929 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java @@ -198,7 +198,8 @@ public static Set createReplicas( public static PlacementPolicy getSimpleTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf) { - return new SCMCommonPlacementPolicy(nodeManager, conf) { + return new SCMCommonPlacementPolicy(nodeManager, conf, + ContainerReplica::getReplicaIndex) { @Override protected List chooseDatanodesInternal( List usedNodes, @@ -222,7 +223,8 @@ public DatanodeDetails chooseNode(List healthyNodes) { public static PlacementPolicy getSameNodeTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf, DatanodeDetails nodeToReturn) { - return new SCMCommonPlacementPolicy(nodeManager, conf) { + return new SCMCommonPlacementPolicy(nodeManager, conf, + ContainerReplica::getReplicaIndex) { @Override protected List chooseDatanodesInternal( List usedNodes, @@ -251,7 +253,8 @@ public DatanodeDetails chooseNode(List healthyNodes) { public static PlacementPolicy getNoNodesTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf) { - return new SCMCommonPlacementPolicy(nodeManager, conf) { + return new SCMCommonPlacementPolicy(nodeManager, conf, + ContainerReplica::getReplicaIndex) { @Override protected List chooseDatanodesInternal( List usedNodes, diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java index 9d5cadeb2d38..cc82fafd2d5e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java @@ -39,6 +39,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.container.TestContainerManagerImpl; import org.apache.hadoop.hdds.scm.exceptions.SCMException; @@ -118,8 +119,8 @@ public void init() throws Exception { .setNodeManager(nodeManager) .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - placementPolicy = new PipelinePlacementPolicy( - nodeManager, stateManager, conf); + placementPolicy = new PipelinePlacementPolicy<>( + nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); } @AfterEach @@ -201,8 +202,10 @@ public void testChooseNodeWithSingleNodeRack() throws IOException { .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - PipelinePlacementPolicy localPlacementPolicy = new PipelinePlacementPolicy( - localNodeManager, tempPipelineStateManager, conf); + PipelinePlacementPolicy localPlacementPolicy = + new PipelinePlacementPolicy<>(localNodeManager, + tempPipelineStateManager, conf, + ContainerReplica::getReplicaIndex); int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber(); List results = localPlacementPolicy.chooseDatanodes( new ArrayList<>(datanodes.size()), @@ -238,8 +241,10 @@ public void testChooseNodeNotEnoughSpace() throws IOException { .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - PipelinePlacementPolicy localPlacementPolicy = new PipelinePlacementPolicy( - localNodeManager, tempPipelineStateManager, conf); + PipelinePlacementPolicy localPlacementPolicy = + new PipelinePlacementPolicy<>(localNodeManager, + tempPipelineStateManager, conf, + ContainerReplica::getReplicaIndex); int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber(); String expectedMessageSubstring = "Unable to find enough nodes that meet " + @@ -477,8 +482,8 @@ public void testValidatePlacementPolicyOK() { cluster = initTopology(); nodeManager = new MockNodeManager(cluster, getNodesWithRackAwareness(), false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy( - nodeManager, stateManager, conf); + placementPolicy = new PipelinePlacementPolicy<>( + nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); List dns = new ArrayList<>(); dns.add(MockDatanodeDetails @@ -524,8 +529,8 @@ public void testValidatePlacementPolicySingleRackInCluster() { cluster = initTopology(); nodeManager = new MockNodeManager(cluster, new ArrayList<>(), false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy( - nodeManager, stateManager, conf); + placementPolicy = new PipelinePlacementPolicy<>( + nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); List dns = new ArrayList<>(); dns.add(MockDatanodeDetails @@ -616,8 +621,8 @@ private List setupSkewedRacks() { nodeManager = new MockNodeManager(cluster, dns, false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy( - nodeManager, stateManager, conf); + placementPolicy = new PipelinePlacementPolicy<>( + nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); return dns; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java index 6bb641e62b2c..1043751ce32f 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java @@ -22,6 +22,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementCapacity; import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRandom; @@ -76,13 +77,15 @@ public void testCapacityPlacementYieldsBetterDataDistribution() throws assertEquals(beforeCapacity.getStandardDeviation(), beforeRandom .getStandardDeviation(), 0.001); - SCMContainerPlacementCapacity capacityPlacer = new - SCMContainerPlacementCapacity(nodeManagerCapacity, + SCMContainerPlacementCapacity capacityPlacer = new + SCMContainerPlacementCapacity<>(nodeManagerCapacity, new OzoneConfiguration(), - null, true, null); - SCMContainerPlacementRandom randomPlacer = new - SCMContainerPlacementRandom(nodeManagerRandom, new OzoneConfiguration(), - null, true, null); + null, true, null, + ContainerReplica::getReplicaIndex); + SCMContainerPlacementRandom randomPlacer = new + SCMContainerPlacementRandom<>(nodeManagerRandom, + new OzoneConfiguration(), null, true, null, + ContainerReplica::getReplicaIndex); for (int x = 0; x < opsCount; x++) { long containerSize = random.nextInt(10) * OzoneConsts.GB; diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index d60bbe980162..a06433bc09cc 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -30,6 +30,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; @@ -46,6 +47,7 @@ import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementStatusDefault; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.ozone.recon.persistence.ContainerHealthSchemaManager; import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider; @@ -343,7 +345,8 @@ private ContainerInfo getMockDeletedContainer(int containerID) { * of a datanode via setMisRepWhenDnPresent. If a DN with that UUID is passed * to validateContainerPlacement, then it will return an invalid placement. */ - private static class MockPlacementPolicy implements PlacementPolicy { + private static class MockPlacementPolicy implements + PlacementPolicy { private UUID misRepWhenDnPresent = null; @@ -370,6 +373,25 @@ public ContainerPlacementStatus validateContainerPlacement( } } + @Override + public Map replicasToCopy( + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { + return null; + } + + @Override + public Set replicasToRemove( + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { + return null; + } + + @Override + public Node getPlacementGroup(DatanodeDetails dn) { + return dn.getParent(); + } + private boolean isDnPresent(List dns) { for (DatanodeDetails dn : dns) { if (misRepWhenDnPresent != null From daeddb412ef9eb9a93a361d1832a5329809eade4 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 28 Nov 2022 09:39:27 -0800 Subject: [PATCH 02/23] HDDS-7492. Address Review Comments --- .../hdds/scm/SCMCommonPlacementPolicy.java | 55 ++++++++++--------- .../SCMContainerPlacementCapacity.java | 11 ++-- .../SCMContainerPlacementRackAware.java | 11 ++-- .../SCMContainerPlacementRackScatter.java | 11 ++-- .../SCMContainerPlacementRandom.java | 11 ++-- .../scm/pipeline/PipelinePlacementPolicy.java | 11 ++-- .../scm/pipeline/RatisPipelineProvider.java | 3 +- .../scm/TestSCMCommonPlacementPolicy.java | 35 ++++++++---- .../TestSCMContainerPlacementCapacity.java | 7 +-- .../TestSCMContainerPlacementRackAware.java | 19 +++---- .../TestSCMContainerPlacementRackScatter.java | 14 ++--- .../TestSCMContainerPlacementRandom.java | 19 +++---- .../replication/ReplicationTestUtil.java | 9 +-- .../pipeline/TestPipelinePlacementPolicy.java | 29 ++++------ .../placement/TestContainerPlacement.java | 15 ++--- 15 files changed, 122 insertions(+), 138 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 602a2c18f0cc..d2f80f06c211 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -57,7 +57,7 @@ * for all basic placement policies, acts as the repository of helper * functions which are common to placement policies. */ -public abstract class SCMCommonPlacementPolicy implements +public abstract class SCMCommonPlacementPolicy implements PlacementPolicy { @VisibleForTesting static final Logger LOG = @@ -69,7 +69,7 @@ public abstract class SCMCommonPlacementPolicy implements private final ConfigurationSource conf; private final boolean shouldRemovePeers; - private Function replicaIdentifierFunction; + private Function replicaIdentifierFunction; /** * Return for replication factor 1 containers where the placement policy @@ -90,13 +90,26 @@ public abstract class SCMCommonPlacementPolicy implements */ public SCMCommonPlacementPolicy(NodeManager nodeManager, ConfigurationSource conf, - Function replicaIdentifierFunction) { + Function replicaIdentifierFunction) { this.nodeManager = nodeManager; this.conf = conf; this.shouldRemovePeers = ScmUtils.shouldRemovePeers(conf); this.replicaIdentifierFunction = replicaIdentifierFunction; } + /** + * Constructor. + * + * @param nodeManager NodeManager + * @param conf Configuration class. + */ + public SCMCommonPlacementPolicy(NodeManager nodeManager, + ConfigurationSource conf) { + this(nodeManager, conf, ContainerReplica::getReplicaIndex); + } + + + /** * Return node manager. * @@ -445,12 +458,12 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, @Override public Set replicasToRemove(Set replicas, int expectedCountPerUniqueReplica, int expectedUniqueGroups) { - Map> replicaIdMap = new HashMap<>(); - Map>> placementGroupReplicaIdMap + Map> replicaIdMap = new HashMap<>(); + Map>> placementGroupReplicaIdMap = new HashMap<>(); Map placementGroupCntMap = new HashMap<>(); for (ContainerReplica replica:replicas) { - RID replicaId = replicaIdentifierFunction.apply(replica); + Integer replicaId = replicaIdentifierFunction.apply(replica); Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); if (!replicaIdMap.containsKey(replicaId)) { replicaIdMap.put(replicaId, Sets.newHashSet()); @@ -461,7 +474,7 @@ public Set replicasToRemove(Set replicas, placementGroupCntMap.compute(placementGroup, (group, cnt) -> (cnt == null ? 0 : cnt) + 1); replicaIdMap.get(replicaId).add(replica); - Map> placementGroupReplicaIDMap = + Map> placementGroupReplicaIDMap = placementGroupReplicaIdMap.get(placementGroup); placementGroupReplicaIDMap.compute(replicaId, (rid, placementGroupReplicas) -> { @@ -474,10 +487,10 @@ public Set replicasToRemove(Set replicas, } Set replicasToRemove = new HashSet<>(); - List sortedRIDList = replicaIdMap.keySet().stream().sorted((o1, o2) -> - Integer.compare(replicaIdMap.get(o2).size(), + List sortedRIDList = replicaIdMap.keySet().stream() + .sorted((o1, o2) -> Integer.compare(replicaIdMap.get(o2).size(), replicaIdMap.get(o1).size())).collect(Collectors.toList()); - for (RID rid : sortedRIDList) { + for (Integer rid : sortedRIDList) { Queue pq = new PriorityQueue<>((o1, o2) -> Integer.compare(placementGroupCntMap.get(o2), placementGroupCntMap.get(o1))); @@ -513,18 +526,15 @@ public Set replicasToRemove(Set replicas, public Map replicasToCopy( Set replicas, int expectedCountPerUniqueReplicas, int expectedUniqueGroups) { - Map replicaIdMap = new HashMap<>(); - Map replicaIdCntMap = new HashMap<>(); + Map replicaIdMap = new HashMap<>(); Map> placementGroupReplicaIdMap = new HashMap<>(); for (ContainerReplica replica:replicas) { - RID replicaId = replicaIdentifierFunction.apply(replica); + Integer replicaId = replicaIdentifierFunction.apply(replica); Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); if (!replicaIdMap.containsKey(replicaId)) { replicaIdMap.put(replicaId, replica); } - replicaIdCntMap.compute(replicaId, (rid, cnt) -> - (cnt == null ? 0 : cnt) + 1); placementGroupReplicaIdMap.compute(placementGroup, (rid, placementGroupReplicas) -> { if (placementGroupReplicas == null) { @@ -537,25 +547,16 @@ public Map replicasToCopy( int misreplicationCnt = Math.max(getRequiredRackCount( expectedUniqueGroups * expectedCountPerUniqueReplicas) - placementGroupReplicaIdMap.size(), 0); - Map copyRIDMap = new HashMap<>(); - for (RID rid : replicaIdMap.keySet()) { - if (replicaIdCntMap.get(rid) < expectedCountPerUniqueReplicas) { - int additionalReplica = expectedCountPerUniqueReplicas - - replicaIdCntMap.get(rid); - copyRIDMap.compute(rid, (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) - + additionalReplica); - misreplicationCnt -= additionalReplica; - } - } + Map copyRIDMap = new HashMap<>(); for (Set replicaSet: placementGroupReplicaIdMap .values()) { if (misreplicationCnt > 0 && replicaSet.size() > 1) { - Map cntMap = replicaSet.stream() + Map cntMap = replicaSet.stream() .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) .collect(Collectors.groupingBy(replicaIdentifierFunction, Collectors.counting())); - for (RID rid : cntMap.keySet()) { + for (Integer rid : cntMap.keySet()) { int additionalReplica = Math.toIntExact(cntMap.get(rid)); copyRIDMap.compute(rid, (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) + additionalReplica); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java index 8f618ea47155..3ac5bcbe5522 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementCapacity.java @@ -18,12 +18,10 @@ package org.apache.hadoop.hdds.scm.container.placement.algorithms; import java.util.List; -import java.util.function.Function; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -68,8 +66,8 @@ * little or no work and the cluster will achieve a balanced distribution * over time. */ -public final class SCMContainerPlacementCapacity - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementCapacity + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementCapacity.class); @@ -83,9 +81,8 @@ public final class SCMContainerPlacementCapacity */ public SCMContainerPlacementCapacity(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics, - final Function replicaIdentifierFunction) { - super(nodeManager, conf, replicaIdentifierFunction); + final boolean fallback, final SCMContainerPlacementMetrics metrics) { + super(nodeManager, conf); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java index c2c6c528f8c8..4ee7408fbb5f 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java @@ -22,7 +22,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -34,7 +33,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.function.Function; import java.util.stream.Collectors; /** @@ -49,8 +47,8 @@ * recommend to use this if the network topology has more layers. *

*/ -public final class SCMContainerPlacementRackAware - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementRackAware + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRackAware.class); @@ -74,9 +72,8 @@ public final class SCMContainerPlacementRackAware */ public SCMContainerPlacementRackAware(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics, - final Function containerReplicaFunction) { - super(nodeManager, conf, containerReplicaFunction); + final boolean fallback, final SCMContainerPlacementMetrics metrics) { + super(nodeManager, conf); this.networkTopology = networkTopology; this.fallback = fallback; this.metrics = metrics; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java index a9f01425f631..eff2dc86c426 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java @@ -22,7 +22,6 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.Node; @@ -37,7 +36,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -52,8 +50,8 @@ * recommend to use this if the network topology has more layers. *

*/ -public final class SCMContainerPlacementRackScatter - extends SCMCommonPlacementPolicy { +public final class SCMContainerPlacementRackScatter + extends SCMCommonPlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRackScatter.class); @@ -73,9 +71,8 @@ public final class SCMContainerPlacementRackScatter */ public SCMContainerPlacementRackScatter(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - boolean fallback, final SCMContainerPlacementMetrics metrics, - Function replicaIdentifierFunction) { - super(nodeManager, conf, replicaIdentifierFunction); + boolean fallback, final SCMContainerPlacementMetrics metrics) { + super(nodeManager, conf); this.networkTopology = networkTopology; this.metrics = metrics; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java index 351ff78f9f67..45fe9dd90576 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java @@ -31,7 +31,6 @@ import org.slf4j.LoggerFactory; import java.util.List; -import java.util.function.Function; /** * Container placement policy that randomly chooses healthy datanodes. @@ -42,9 +41,8 @@ * Balancer will need to support containers as a feature before this class * can be practically used. */ -public final class SCMContainerPlacementRandom extends - SCMCommonPlacementPolicy implements - PlacementPolicy { +public final class SCMContainerPlacementRandom extends SCMCommonPlacementPolicy + implements PlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRandom.class); @@ -57,9 +55,8 @@ public final class SCMContainerPlacementRandom extends */ public SCMContainerPlacementRandom(final NodeManager nodeManager, final ConfigurationSource conf, final NetworkTopology networkTopology, - final boolean fallback, final SCMContainerPlacementMetrics metrics, - Function replicaIdentifierFunction) { - super(nodeManager, conf, replicaIdentifierFunction); + final boolean fallback, final SCMContainerPlacementMetrics metrics) { + super(nodeManager, conf); } /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java index bf3814c5f28e..befc0543a357 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java @@ -27,7 +27,6 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.SCMCommonPlacementPolicy; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.node.NodeManager; @@ -38,7 +37,6 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.List; -import java.util.function.Function; import java.util.stream.Collectors; /** @@ -51,8 +49,7 @@ * 4. Choose an anchor node among the viable nodes. * 5. Choose other nodes around the anchor node based on network topology */ -public final class PipelinePlacementPolicy extends - SCMCommonPlacementPolicy { +public final class PipelinePlacementPolicy extends SCMCommonPlacementPolicy { @VisibleForTesting static final Logger LOG = LoggerFactory.getLogger(PipelinePlacementPolicy.class); @@ -76,9 +73,9 @@ public final class PipelinePlacementPolicy extends * @param conf Configuration */ public PipelinePlacementPolicy(final NodeManager nodeManager, - final PipelineStateManager stateManager, final ConfigurationSource conf, - final Function replicaIdentifierFunction) { - super(nodeManager, conf, replicaIdentifierFunction); + final PipelineStateManager stateManager, + final ConfigurationSource conf) { + super(nodeManager, conf); this.nodeManager = nodeManager; this.conf = conf; this.stateManager = stateManager; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java index 9b70f17f9876..43b2e01c9140 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/RatisPipelineProvider.java @@ -78,8 +78,7 @@ public RatisPipelineProvider(NodeManager nodeManager, this.eventPublisher = eventPublisher; this.scmContext = scmContext; this.placementPolicy = - new PipelinePlacementPolicy<>(nodeManager, stateManager, conf, - ContainerReplica::getReplicaIndex); + new PipelinePlacementPolicy(nodeManager, stateManager, conf); this.pipelineNumberLimit = conf.getInt( ScmConfigKeys.OZONE_SCM_RATIS_PIPELINE_LIMIT, ScmConfigKeys.OZONE_SCM_RATIS_PIPELINE_LIMIT_DEFAULT); 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 672fdae573c3..039cee5ed126 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 @@ -33,11 +33,13 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -74,20 +76,28 @@ public void testReplicasToCopy() { new DummyPlacementPolicy(nodeManager, conf); List list = nodeManager.getNodes(NodeStatus.inServiceHealthy()); - Set replicas = - IntStream.range(1, 6).mapToObj(i -> + Map replicas = + IntStream.range(1, 5).mapToObj(i -> ContainerReplica.newBuilder() .setContainerID(new ContainerID(1)) .setContainerState(CLOSED) .setReplicaIndex(i) - .setDatanodeDetails(list.get(i)).build()) - .collect(Collectors.toSet()); - + .setDatanodeDetails(list.get(i - 1)).build()) + .collect(Collectors.toMap(ContainerReplica::getReplicaIndex, + Function.identity())); + replicas.put(5, ContainerReplica.newBuilder() + .setContainerID(new ContainerID(1)) + .setContainerState(CLOSED) + .setReplicaIndex(5) + .setDatanodeDetails(list.get(5)).build()); Map replicasToCopy = - dummyPlacementPolicy.replicasToCopy(replicas, - 2, 5); - Assertions.assertEquals(replicas, replicasToCopy.keySet()); + dummyPlacementPolicy.replicasToCopy(replicas.values() + .stream().collect(Collectors.toSet()), + 1, 5); + Assertions.assertTrue(replicasToCopy.keySet().stream().findFirst() + .map(replica -> Arrays.asList(replicas.get(1), replicas.get(5)) + .contains(replica)).orElse(false)); Assertions.assertTrue(replicasToCopy.values().stream() .allMatch(i -> i == 1)); } @@ -122,13 +132,13 @@ public void testReplicasToRemove() { } private static class DummyPlacementPolicy extends - SCMCommonPlacementPolicy { + SCMCommonPlacementPolicy { private Map dns; DummyPlacementPolicy( NodeManager nodeManager, ConfigurationSource conf) { - super(nodeManager, conf, ContainerReplica::getReplicaIndex); + super(nodeManager, conf); dns = new HashMap<>(); List datanodeDetails = nodeManager.getNodes(NodeStatus.inServiceHealthy()); @@ -146,5 +156,10 @@ public DatanodeDetails chooseNode(List healthyNodes) { public Node getPlacementGroup(DatanodeDetails dn) { return dns.get(dn); } + + @Override + protected int getRequiredRackCount(int numReplicas) { + return 5; + } } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java index 6352f088199d..fb169c7feac5 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementCapacity.java @@ -29,7 +29,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.HddsTestUtils; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMNodeMetric; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; @@ -121,9 +120,9 @@ public void chooseDatanodes() throws SCMException { .orElse(null); }); - SCMContainerPlacementCapacity scmContainerPlacementRandom = - new SCMContainerPlacementCapacity<>(mockNodeManager, conf, null, true, - null, ContainerReplica::getReplicaIndex); + SCMContainerPlacementCapacity scmContainerPlacementRandom = + new SCMContainerPlacementCapacity(mockNodeManager, conf, null, true, + null); List existingNodes = new ArrayList<>(); existingNodes.add(datanodes.get(0)); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java index 56a1976fea2a..3342c402d7aa 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java @@ -30,7 +30,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -76,9 +75,9 @@ public class TestSCMContainerPlacementRackAware { private final List datanodes = new ArrayList<>(); private final List dnInfos = new ArrayList<>(); // policy with fallback capability - private SCMContainerPlacementRackAware policy; + private SCMContainerPlacementRackAware policy; // policy prohibit fallback - private SCMContainerPlacementRackAware policyNoFallback; + private SCMContainerPlacementRackAware policyNoFallback; // node storage capacity private static final long STORAGE_CAPACITY = 100L; private SCMContainerPlacementMetrics metrics; @@ -181,10 +180,10 @@ private void setup(int datanodeCount) { .thenReturn(cluster); // create placement policy instances - policy = new SCMContainerPlacementRackAware<>(nodeManager, conf, cluster, - true, metrics, ContainerReplica::getReplicaIndex); - policyNoFallback = new SCMContainerPlacementRackAware<>(nodeManager, conf, - cluster, false, metrics, ContainerReplica::getReplicaIndex); + policy = new SCMContainerPlacementRackAware( + nodeManager, conf, cluster, true, metrics); + policyNoFallback = new SCMContainerPlacementRackAware( + nodeManager, conf, cluster, false, metrics); } @BeforeEach @@ -482,9 +481,9 @@ public void testDatanodeWithDefaultNetworkLocation(int datanodeCount) // choose nodes to host 3 replica int nodeNum = 3; - SCMContainerPlacementRackAware newPolicy = - new SCMContainerPlacementRackAware<>(nodeManager, conf, clusterMap, - true, metrics, ContainerReplica::getReplicaIndex); + SCMContainerPlacementRackAware newPolicy = + new SCMContainerPlacementRackAware(nodeManager, conf, clusterMap, true, + metrics); List datanodeDetails = newPolicy.chooseDatanodes(null, null, nodeNum, 0, 15); Assertions.assertEquals(nodeNum, datanodeDetails.size()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java index 604571a89fc8..46bf8031effc 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java @@ -25,7 +25,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.NetworkTopology; @@ -80,7 +79,7 @@ public class TestSCMContainerPlacementRackScatter { private final List datanodes = new ArrayList<>(); private final List dnInfos = new ArrayList<>(); // policy with fallback capability - private SCMContainerPlacementRackScatter policy; + private SCMContainerPlacementRackScatter policy; // node storage capacity private static final long STORAGE_CAPACITY = 100L; private SCMContainerPlacementMetrics metrics; @@ -197,9 +196,8 @@ private void setup(int datanodeCount, int nodesPerRack) { .thenReturn(cluster); // create placement policy instances - policy = new SCMContainerPlacementRackScatter<>( - nodeManager, conf, cluster, true, metrics, - ContainerReplica::getReplicaIndex); + policy = new SCMContainerPlacementRackScatter( + nodeManager, conf, cluster, true, metrics); } @BeforeEach @@ -467,9 +465,9 @@ public void testDatanodeWithDefaultNetworkLocation(int datanodeCount) // choose nodes to host 5 replica int nodeNum = 5; - SCMContainerPlacementRackScatter newPolicy = - new SCMContainerPlacementRackScatter<>(nodeManager, conf, clusterMap, - true, metrics, ContainerReplica::getReplicaIndex); + SCMContainerPlacementRackScatter newPolicy = + new SCMContainerPlacementRackScatter(nodeManager, conf, clusterMap, + true, metrics); List datanodeDetails = newPolicy.chooseDatanodes(null, null, nodeNum, 0, 15); Assertions.assertEquals(nodeNum, datanodeDetails.size()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java index bab6d7c1310e..b7c152b06d35 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRandom.java @@ -28,7 +28,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.HddsTestUtils; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; import org.apache.hadoop.hdds.scm.node.NodeManager; @@ -90,9 +89,9 @@ public void chooseDatanodes() throws SCMException { when(mockNodeManager.getNodes(NodeStatus.inServiceHealthy())) .thenReturn(new ArrayList<>(datanodes)); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, - null, ContainerReplica::getReplicaIndex); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, + null); List existingNodes = new ArrayList<>(); existingNodes.add(datanodes.get(0)); @@ -133,9 +132,9 @@ public void testPlacementPolicySatisified() { } NodeManager mockNodeManager = Mockito.mock(NodeManager.class); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, - null, ContainerReplica::getReplicaIndex); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, + null); ContainerPlacementStatus status = scmContainerPlacementRandom.validateContainerPlacement(datanodes, 3); assertTrue(status.isPolicySatisfied()); @@ -212,9 +211,9 @@ public void testIsValidNode() throws SCMException { when(mockNodeManager.getNodeByUuid(datanodes.get(2).getUuidString())) .thenReturn(datanodes.get(2)); - SCMContainerPlacementRandom scmContainerPlacementRandom = - new SCMContainerPlacementRandom<>(mockNodeManager, conf, null, true, - null, ContainerReplica::getReplicaIndex); + SCMContainerPlacementRandom scmContainerPlacementRandom = + new SCMContainerPlacementRandom(mockNodeManager, conf, null, true, + null); Assertions.assertTrue( scmContainerPlacementRandom.isValidNode(datanodes.get(0), 15L, 15L)); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java index 132bfda33929..f87d2851586a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationTestUtil.java @@ -198,8 +198,7 @@ public static Set createReplicas( public static PlacementPolicy getSimpleTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf) { - return new SCMCommonPlacementPolicy(nodeManager, conf, - ContainerReplica::getReplicaIndex) { + return new SCMCommonPlacementPolicy(nodeManager, conf) { @Override protected List chooseDatanodesInternal( List usedNodes, @@ -223,8 +222,7 @@ public DatanodeDetails chooseNode(List healthyNodes) { public static PlacementPolicy getSameNodeTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf, DatanodeDetails nodeToReturn) { - return new SCMCommonPlacementPolicy(nodeManager, conf, - ContainerReplica::getReplicaIndex) { + return new SCMCommonPlacementPolicy(nodeManager, conf) { @Override protected List chooseDatanodesInternal( List usedNodes, @@ -253,8 +251,7 @@ public DatanodeDetails chooseNode(List healthyNodes) { public static PlacementPolicy getNoNodesTestPlacementPolicy( final NodeManager nodeManager, final OzoneConfiguration conf) { - return new SCMCommonPlacementPolicy(nodeManager, conf, - ContainerReplica::getReplicaIndex) { + return new SCMCommonPlacementPolicy(nodeManager, conf) { @Override protected List chooseDatanodesInternal( List usedNodes, diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java index cc82fafd2d5e..9d5cadeb2d38 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelinePlacementPolicy.java @@ -39,7 +39,6 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; import org.apache.hadoop.hdds.scm.ScmConfigKeys; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.container.TestContainerManagerImpl; import org.apache.hadoop.hdds.scm.exceptions.SCMException; @@ -119,8 +118,8 @@ public void init() throws Exception { .setNodeManager(nodeManager) .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - placementPolicy = new PipelinePlacementPolicy<>( - nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); + placementPolicy = new PipelinePlacementPolicy( + nodeManager, stateManager, conf); } @AfterEach @@ -202,10 +201,8 @@ public void testChooseNodeWithSingleNodeRack() throws IOException { .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - PipelinePlacementPolicy localPlacementPolicy = - new PipelinePlacementPolicy<>(localNodeManager, - tempPipelineStateManager, conf, - ContainerReplica::getReplicaIndex); + PipelinePlacementPolicy localPlacementPolicy = new PipelinePlacementPolicy( + localNodeManager, tempPipelineStateManager, conf); int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber(); List results = localPlacementPolicy.chooseDatanodes( new ArrayList<>(datanodes.size()), @@ -241,10 +238,8 @@ public void testChooseNodeNotEnoughSpace() throws IOException { .setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer()) .build(); - PipelinePlacementPolicy localPlacementPolicy = - new PipelinePlacementPolicy<>(localNodeManager, - tempPipelineStateManager, conf, - ContainerReplica::getReplicaIndex); + PipelinePlacementPolicy localPlacementPolicy = new PipelinePlacementPolicy( + localNodeManager, tempPipelineStateManager, conf); int nodesRequired = HddsProtos.ReplicationFactor.THREE.getNumber(); String expectedMessageSubstring = "Unable to find enough nodes that meet " + @@ -482,8 +477,8 @@ public void testValidatePlacementPolicyOK() { cluster = initTopology(); nodeManager = new MockNodeManager(cluster, getNodesWithRackAwareness(), false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy<>( - nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); + placementPolicy = new PipelinePlacementPolicy( + nodeManager, stateManager, conf); List dns = new ArrayList<>(); dns.add(MockDatanodeDetails @@ -529,8 +524,8 @@ public void testValidatePlacementPolicySingleRackInCluster() { cluster = initTopology(); nodeManager = new MockNodeManager(cluster, new ArrayList<>(), false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy<>( - nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); + placementPolicy = new PipelinePlacementPolicy( + nodeManager, stateManager, conf); List dns = new ArrayList<>(); dns.add(MockDatanodeDetails @@ -621,8 +616,8 @@ private List setupSkewedRacks() { nodeManager = new MockNodeManager(cluster, dns, false, PIPELINE_PLACEMENT_MAX_NODES_COUNT); - placementPolicy = new PipelinePlacementPolicy<>( - nodeManager, stateManager, conf, ContainerReplica::getReplicaIndex); + placementPolicy = new PipelinePlacementPolicy( + nodeManager, stateManager, conf); return dns; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java index 1043751ce32f..6bb641e62b2c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/ozone/container/placement/TestContainerPlacement.java @@ -22,7 +22,6 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementCapacity; import org.apache.hadoop.hdds.scm.container.placement.algorithms.SCMContainerPlacementRandom; @@ -77,15 +76,13 @@ public void testCapacityPlacementYieldsBetterDataDistribution() throws assertEquals(beforeCapacity.getStandardDeviation(), beforeRandom .getStandardDeviation(), 0.001); - SCMContainerPlacementCapacity capacityPlacer = new - SCMContainerPlacementCapacity<>(nodeManagerCapacity, + SCMContainerPlacementCapacity capacityPlacer = new + SCMContainerPlacementCapacity(nodeManagerCapacity, new OzoneConfiguration(), - null, true, null, - ContainerReplica::getReplicaIndex); - SCMContainerPlacementRandom randomPlacer = new - SCMContainerPlacementRandom<>(nodeManagerRandom, - new OzoneConfiguration(), null, true, null, - ContainerReplica::getReplicaIndex); + null, true, null); + SCMContainerPlacementRandom randomPlacer = new + SCMContainerPlacementRandom(nodeManagerRandom, new OzoneConfiguration(), + null, true, null); for (int x = 0; x < opsCount; x++) { long containerSize = random.nextInt(10) * OzoneConsts.GB; From b4b891b030d0759ed2387be242ddf45360c4400f Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 28 Nov 2022 19:33:32 -0800 Subject: [PATCH 03/23] HDDS-7492. Address Review Comments --- .../hdds/client/ECReplicationConfig.java | 10 ++++++++ .../client/ReplicatedReplicationConfig.java | 10 ++++++++ .../hadoop/hdds/client/ReplicationConfig.java | 4 ++++ .../hadoop/hdds/scm/PlacementPolicy.java | 16 ++++--------- .../hdds/scm/SCMCommonPlacementPolicy.java | 23 ++++++++++--------- .../SCMContainerPlacementRandom.java | 3 +-- .../scm/TestSCMCommonPlacementPolicy.java | 19 +++++++++++---- .../TestContainerPlacementFactory.java | 17 ++++++-------- .../recon/fsck/TestContainerHealthTask.java | 16 +++++-------- 9 files changed, 68 insertions(+), 50 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java index 747c6fd52554..27dae0ac270f 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java @@ -209,4 +209,14 @@ public String toString() { public String configFormat() { return HddsProtos.ReplicationType.EC.name() + "/" + data + "-" + parity; } + + @Override + public int getReplicationFactorOfUniqueReplica() { + return 1; + } + + @Override + public int getNumberOfUniqueReplica() { + return data + parity; + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java index c949524c0a6f..7e45af6ded8f 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java @@ -31,4 +31,14 @@ public interface ReplicatedReplicationConfig extends ReplicationConfig { * @return the replication factor */ HddsProtos.ReplicationFactor getReplicationFactor(); + + @Override + default int getReplicationFactorOfUniqueReplica() { + return getReplicationFactor().getNumber(); + } + + @Override + default int getNumberOfUniqueReplica() { + return 1; + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java index 610419527a4b..9042c352ced9 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java @@ -223,4 +223,8 @@ static ReplicationConfig parseWithoutFallback(ReplicationType type, String configFormat(); + int getReplicationFactorOfUniqueReplica(); + + int getNumberOfUniqueReplica(); + } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index c630118a13c4..affb28562ad2 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -17,6 +17,7 @@ package org.apache.hadoop.hdds.scm; +import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import java.io.IOException; @@ -29,7 +30,7 @@ * A PlacementPolicy support choosing datanodes to build * pipelines or containers with specified constraints. */ -public interface PlacementPolicy { +public interface PlacementPolicy { default List chooseDatanodes( List excludedNodes, @@ -68,17 +69,8 @@ List chooseDatanodes(List usedNodes, ContainerPlacementStatus validateContainerPlacement( List dns, int replicas); Map replicasToCopy(Set replicas, - int expectedCountPerUniqueReplica, - int expectedUniqueGroups); + ReplicationConfig replicationConfig); Set replicasToRemove(Set replicas, - int expectedCountPerUniqueReplica, - int expectedUniqueGroups); - - - /** Gets the group of from the datanode based on the placement. - * @param dn - * @return PlacementGroup - */ - PlacementGroup getPlacementGroup(DatanodeDetails dn); + ReplicationConfig replicationConfig); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index d2f80f06c211..67d3fbdbeb0c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -35,6 +35,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; @@ -58,7 +59,7 @@ * functions which are common to placement policies. */ public abstract class SCMCommonPlacementPolicy implements - PlacementPolicy { + PlacementPolicy { @VisibleForTesting static final Logger LOG = LoggerFactory.getLogger(SCMCommonPlacementPolicy.class); @@ -457,7 +458,7 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } @Override public Set replicasToRemove(Set replicas, - int expectedCountPerUniqueReplica, int expectedUniqueGroups) { + ReplicationConfig replicationConfig) { Map> replicaIdMap = new HashMap<>(); Map>> placementGroupReplicaIdMap = new HashMap<>(); @@ -500,7 +501,8 @@ public Set replicasToRemove(Set replicas, .map(Map.Entry::getKey) .collect(Collectors.toList())); - while (replicaIdMap.get(rid).size() > expectedCountPerUniqueReplica) { + while (replicaIdMap.get(rid).size() > + replicationConfig.getReplicationFactorOfUniqueReplica()) { Node rack = pq.poll(); Set replicaSet = placementGroupReplicaIdMap.get(rack).get(rid); @@ -524,8 +526,7 @@ public Set replicasToRemove(Set replicas, @Override public Map replicasToCopy( - Set replicas, int expectedCountPerUniqueReplicas, - int expectedUniqueGroups) { + Set replicas, ReplicationConfig replicationConfig) { Map replicaIdMap = new HashMap<>(); Map> placementGroupReplicaIdMap = new HashMap<>(); @@ -545,7 +546,7 @@ public Map replicasToCopy( }); } int misreplicationCnt = Math.max(getRequiredRackCount( - expectedUniqueGroups * expectedCountPerUniqueReplicas) + replicationConfig.getRequiredNodes()) - placementGroupReplicaIdMap.size(), 0); Map copyRIDMap = new HashMap<>(); @@ -556,9 +557,10 @@ public Map replicasToCopy( .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) .collect(Collectors.groupingBy(replicaIdentifierFunction, Collectors.counting())); - for (Integer rid : cntMap.keySet()) { - int additionalReplica = Math.toIntExact(cntMap.get(rid)); - copyRIDMap.compute(rid, (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) + for (Map.Entry ridCntEntry : cntMap.entrySet()) { + int additionalReplica = Math.toIntExact(ridCntEntry.getValue()); + copyRIDMap.compute(ridCntEntry.getKey(), + (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) + additionalReplica); misreplicationCnt -= additionalReplica; } @@ -569,8 +571,7 @@ public Map replicasToCopy( Map.Entry::getValue)); } - @Override - public Node getPlacementGroup(DatanodeDetails dn) { + protected Node getPlacementGroup(DatanodeDetails dn) { return nodeManager.getClusterNetworkTopologyMap().getAncestor(dn, 1); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java index 45fe9dd90576..2aa11211015c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRandom.java @@ -24,7 +24,6 @@ import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; -import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.slf4j.Logger; @@ -42,7 +41,7 @@ * can be practically used. */ public final class SCMContainerPlacementRandom extends SCMCommonPlacementPolicy - implements PlacementPolicy { + implements PlacementPolicy { @VisibleForTesting public static final Logger LOG = LoggerFactory.getLogger(SCMContainerPlacementRandom.class); 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 039cee5ed126..a574299eaa68 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm; +import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -32,6 +33,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import java.util.Arrays; import java.util.HashMap; @@ -90,11 +92,15 @@ public void testReplicasToCopy() { .setContainerState(CLOSED) .setReplicaIndex(5) .setDatanodeDetails(list.get(5)).build()); - + ReplicationConfig replicationConfig = Mockito.mock(ReplicationConfig.class); + Mockito.when(replicationConfig.getNumberOfUniqueReplica()).thenReturn(5); + Mockito.when(replicationConfig.getRequiredNodes()).thenReturn(5); + Mockito.when(replicationConfig.getReplicationFactorOfUniqueReplica()) + .thenReturn(1); Map replicasToCopy = dummyPlacementPolicy.replicasToCopy(replicas.values() .stream().collect(Collectors.toSet()), - 1, 5); + replicationConfig); Assertions.assertTrue(replicasToCopy.keySet().stream().findFirst() .map(replica -> Arrays.asList(replicas.get(1), replicas.get(5)) .contains(replica)).orElse(false)); @@ -122,10 +128,13 @@ public void testReplicasToRemove() { .setReplicaIndex(1) .setDatanodeDetails(list.get(7)).build(); replicas.add(replica); - + ReplicationConfig replicationConfig = Mockito.mock(ReplicationConfig.class); + Mockito.when(replicationConfig.getNumberOfUniqueReplica()).thenReturn(5); + Mockito.when(replicationConfig.getRequiredNodes()).thenReturn(5); + Mockito.when(replicationConfig.getReplicationFactorOfUniqueReplica()) + .thenReturn(1); Set replicasToRemove = - dummyPlacementPolicy.replicasToRemove(replicas, - 1, 5); + dummyPlacementPolicy.replicasToRemove(replicas, replicationConfig); Assertions.assertEquals(replicasToRemove.size(), 1); Assertions.assertEquals(replicasToRemove.stream().findFirst().get(), replica); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index 9083107e6620..bc4a0dceeab8 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Set; +import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -37,7 +38,6 @@ import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.NetworkTopology; import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl; -import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.net.NodeSchema; import org.apache.hadoop.hdds.scm.net.NodeSchemaManager; import org.apache.hadoop.hdds.scm.node.DatanodeInfo; @@ -181,7 +181,7 @@ public void testECPolicy() throws IOException { * A dummy container placement implementation for test. */ public static class DummyImpl implements - PlacementPolicy { + PlacementPolicy { @Override public List chooseDatanodes( List usedNodes, @@ -199,22 +199,19 @@ public List chooseDatanodes( @Override public Map replicasToCopy( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { + Set replicas, + ReplicationConfig replicationConfig) { return null; } @Override public Set replicasToRemove( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { + Set replicas, + ReplicationConfig replicationConfig) { return null; } - @Override - public Node getPlacementGroup(DatanodeDetails dn) { - return dn.getParent(); - } + } @Test diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index a06433bc09cc..bdafbd0f0bd0 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -35,6 +35,7 @@ import java.util.UUID; 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.MockDatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -47,7 +48,6 @@ import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.container.placement.algorithms.ContainerPlacementStatusDefault; -import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.ozone.recon.persistence.ContainerHealthSchemaManager; import org.apache.hadoop.ozone.recon.scm.ReconStorageContainerManagerFacade; import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider; @@ -346,7 +346,7 @@ private ContainerInfo getMockDeletedContainer(int containerID) { * to validateContainerPlacement, then it will return an invalid placement. */ private static class MockPlacementPolicy implements - PlacementPolicy { + PlacementPolicy { private UUID misRepWhenDnPresent = null; @@ -375,22 +375,18 @@ public ContainerPlacementStatus validateContainerPlacement( @Override public Map replicasToCopy( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { + Set replicas, + ReplicationConfig replicationConfig) { return null; } @Override public Set replicasToRemove( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { + Set replicas, + ReplicationConfig replicationConfig) { return null; } - @Override - public Node getPlacementGroup(DatanodeDetails dn) { - return dn.getParent(); - } private boolean isDnPresent(List dns) { for (DatanodeDetails dn : dns) { From 7d360418220e56b496f29b3af78ccfdad1c4cd50 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 29 Nov 2022 14:05:23 -0800 Subject: [PATCH 04/23] HDDS-7492. Revert replication config addition to function --- .../hdds/client/ECReplicationConfig.java | 10 ------ .../client/ReplicatedReplicationConfig.java | 10 ------ .../hadoop/hdds/client/ReplicationConfig.java | 4 --- .../hadoop/hdds/scm/PlacementPolicy.java | 7 ++-- .../hdds/scm/SCMCommonPlacementPolicy.java | 35 ++++++------------- .../scm/TestSCMCommonPlacementPolicy.java | 19 +++------- .../TestContainerPlacementFactory.java | 10 +++--- .../recon/fsck/TestContainerHealthTask.java | 9 +++-- 8 files changed, 27 insertions(+), 77 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java index 27dae0ac270f..747c6fd52554 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java @@ -209,14 +209,4 @@ public String toString() { public String configFormat() { return HddsProtos.ReplicationType.EC.name() + "/" + data + "-" + parity; } - - @Override - public int getReplicationFactorOfUniqueReplica() { - return 1; - } - - @Override - public int getNumberOfUniqueReplica() { - return data + parity; - } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java index 7e45af6ded8f..c949524c0a6f 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicatedReplicationConfig.java @@ -31,14 +31,4 @@ public interface ReplicatedReplicationConfig extends ReplicationConfig { * @return the replication factor */ HddsProtos.ReplicationFactor getReplicationFactor(); - - @Override - default int getReplicationFactorOfUniqueReplica() { - return getReplicationFactor().getNumber(); - } - - @Override - default int getNumberOfUniqueReplica() { - return 1; - } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java index 9042c352ced9..610419527a4b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java @@ -223,8 +223,4 @@ static ReplicationConfig parseWithoutFallback(ReplicationType type, String configFormat(); - int getReplicationFactorOfUniqueReplica(); - - int getNumberOfUniqueReplica(); - } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index affb28562ad2..6d3a829d7adb 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -17,7 +17,6 @@ package org.apache.hadoop.hdds.scm; -import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import java.io.IOException; @@ -69,8 +68,10 @@ List chooseDatanodes(List usedNodes, ContainerPlacementStatus validateContainerPlacement( List dns, int replicas); Map replicasToCopy(Set replicas, - ReplicationConfig replicationConfig); + int expectedCountPerUniqueReplica, + int expectedUniqueGroups); Set replicasToRemove(Set replicas, - ReplicationConfig replicationConfig); + int expectedCountPerUniqueReplica, + int expectedUniqueGroups); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 67d3fbdbeb0c..975753383ef4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -35,7 +35,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; @@ -90,27 +89,12 @@ public abstract class SCMCommonPlacementPolicy implements * @param conf Configuration class. */ public SCMCommonPlacementPolicy(NodeManager nodeManager, - ConfigurationSource conf, - Function replicaIdentifierFunction) { + ConfigurationSource conf) { this.nodeManager = nodeManager; this.conf = conf; this.shouldRemovePeers = ScmUtils.shouldRemovePeers(conf); - this.replicaIdentifierFunction = replicaIdentifierFunction; } - /** - * Constructor. - * - * @param nodeManager NodeManager - * @param conf Configuration class. - */ - public SCMCommonPlacementPolicy(NodeManager nodeManager, - ConfigurationSource conf) { - this(nodeManager, conf, ContainerReplica::getReplicaIndex); - } - - - /** * Return node manager. * @@ -458,13 +442,13 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } @Override public Set replicasToRemove(Set replicas, - ReplicationConfig replicationConfig) { + int expectedCountPerUniqueReplica, int expectedUniqueGroups) { Map> replicaIdMap = new HashMap<>(); Map>> placementGroupReplicaIdMap = new HashMap<>(); Map placementGroupCntMap = new HashMap<>(); for (ContainerReplica replica:replicas) { - Integer replicaId = replicaIdentifierFunction.apply(replica); + Integer replicaId = replica.getReplicaIndex(); Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); if (!replicaIdMap.containsKey(replicaId)) { replicaIdMap.put(replicaId, Sets.newHashSet()); @@ -501,8 +485,7 @@ public Set replicasToRemove(Set replicas, .map(Map.Entry::getKey) .collect(Collectors.toList())); - while (replicaIdMap.get(rid).size() > - replicationConfig.getReplicationFactorOfUniqueReplica()) { + while (replicaIdMap.get(rid).size() > expectedCountPerUniqueReplica) { Node rack = pq.poll(); Set replicaSet = placementGroupReplicaIdMap.get(rack).get(rid); @@ -526,12 +509,13 @@ public Set replicasToRemove(Set replicas, @Override public Map replicasToCopy( - Set replicas, ReplicationConfig replicationConfig) { + Set replicas, int expectedCountPerUniqueReplicas, + int expectedUniqueGroups) { Map replicaIdMap = new HashMap<>(); Map> placementGroupReplicaIdMap = new HashMap<>(); for (ContainerReplica replica:replicas) { - Integer replicaId = replicaIdentifierFunction.apply(replica); + Integer replicaId = replica.getReplicaIndex(); Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); if (!replicaIdMap.containsKey(replicaId)) { replicaIdMap.put(replicaId, replica); @@ -546,7 +530,7 @@ public Map replicasToCopy( }); } int misreplicationCnt = Math.max(getRequiredRackCount( - replicationConfig.getRequiredNodes()) + expectedUniqueGroups * expectedCountPerUniqueReplicas) - placementGroupReplicaIdMap.size(), 0); Map copyRIDMap = new HashMap<>(); @@ -555,7 +539,8 @@ public Map replicasToCopy( if (misreplicationCnt > 0 && replicaSet.size() > 1) { Map cntMap = replicaSet.stream() .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) - .collect(Collectors.groupingBy(replicaIdentifierFunction, + .collect(Collectors.groupingBy( + ContainerReplica::getReplicaIndex, Collectors.counting())); for (Map.Entry ridCntEntry : cntMap.entrySet()) { int additionalReplica = Math.toIntExact(ridCntEntry.getValue()); 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 a574299eaa68..039cee5ed126 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 @@ -18,7 +18,6 @@ package org.apache.hadoop.hdds.scm; -import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -33,7 +32,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import java.util.Arrays; import java.util.HashMap; @@ -92,15 +90,11 @@ public void testReplicasToCopy() { .setContainerState(CLOSED) .setReplicaIndex(5) .setDatanodeDetails(list.get(5)).build()); - ReplicationConfig replicationConfig = Mockito.mock(ReplicationConfig.class); - Mockito.when(replicationConfig.getNumberOfUniqueReplica()).thenReturn(5); - Mockito.when(replicationConfig.getRequiredNodes()).thenReturn(5); - Mockito.when(replicationConfig.getReplicationFactorOfUniqueReplica()) - .thenReturn(1); + Map replicasToCopy = dummyPlacementPolicy.replicasToCopy(replicas.values() .stream().collect(Collectors.toSet()), - replicationConfig); + 1, 5); Assertions.assertTrue(replicasToCopy.keySet().stream().findFirst() .map(replica -> Arrays.asList(replicas.get(1), replicas.get(5)) .contains(replica)).orElse(false)); @@ -128,13 +122,10 @@ public void testReplicasToRemove() { .setReplicaIndex(1) .setDatanodeDetails(list.get(7)).build(); replicas.add(replica); - ReplicationConfig replicationConfig = Mockito.mock(ReplicationConfig.class); - Mockito.when(replicationConfig.getNumberOfUniqueReplica()).thenReturn(5); - Mockito.when(replicationConfig.getRequiredNodes()).thenReturn(5); - Mockito.when(replicationConfig.getReplicationFactorOfUniqueReplica()) - .thenReturn(1); + Set replicasToRemove = - dummyPlacementPolicy.replicasToRemove(replicas, replicationConfig); + dummyPlacementPolicy.replicasToRemove(replicas, + 1, 5); Assertions.assertEquals(replicasToRemove.size(), 1); Assertions.assertEquals(replicasToRemove.stream().findFirst().get(), replica); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index bc4a0dceeab8..7e5b265042f4 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -23,7 +23,6 @@ import java.util.Map; import java.util.Set; -import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -199,19 +198,18 @@ public List chooseDatanodes( @Override public Map replicasToCopy( - Set replicas, - ReplicationConfig replicationConfig) { + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { return null; } @Override public Set replicasToRemove( - Set replicas, - ReplicationConfig replicationConfig) { + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { return null; } - } @Test diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index bdafbd0f0bd0..7431a41ee283 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -35,7 +35,6 @@ import java.util.UUID; 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.MockDatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -375,15 +374,15 @@ public ContainerPlacementStatus validateContainerPlacement( @Override public Map replicasToCopy( - Set replicas, - ReplicationConfig replicationConfig) { + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { return null; } @Override public Set replicasToRemove( - Set replicas, - ReplicationConfig replicationConfig) { + Set replicas, int expectedCountPerUniqueReplica, + int expectedUniqueGroups) { return null; } From 7efb3ee98ff627f209288e76e107a19582d9ca13 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 29 Nov 2022 14:31:20 -0800 Subject: [PATCH 05/23] HDDS-7492. Remove Function ReplicasToRemove --- .../hadoop/hdds/scm/PlacementPolicy.java | 4 -- .../hdds/scm/SCMCommonPlacementPolicy.java | 66 ------------------- .../scm/TestSCMCommonPlacementPolicy.java | 29 -------- .../TestContainerPlacementFactory.java | 8 --- .../recon/fsck/TestContainerHealthTask.java | 7 -- 5 files changed, 114 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index 6d3a829d7adb..0d5438311022 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -70,8 +70,4 @@ ContainerPlacementStatus validateContainerPlacement( Map replicasToCopy(Set replicas, int expectedCountPerUniqueReplica, int expectedUniqueGroups); - - Set replicasToRemove(Set replicas, - int expectedCountPerUniqueReplica, - int expectedUniqueGroups); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 975753383ef4..26e4773dcc39 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -440,72 +440,6 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } return false; } - @Override - public Set replicasToRemove(Set replicas, - int expectedCountPerUniqueReplica, int expectedUniqueGroups) { - Map> replicaIdMap = new HashMap<>(); - Map>> placementGroupReplicaIdMap - = new HashMap<>(); - Map placementGroupCntMap = new HashMap<>(); - for (ContainerReplica replica:replicas) { - Integer replicaId = replica.getReplicaIndex(); - Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); - if (!replicaIdMap.containsKey(replicaId)) { - replicaIdMap.put(replicaId, Sets.newHashSet()); - } - if (!placementGroupReplicaIdMap.containsKey(placementGroup)) { - placementGroupReplicaIdMap.put(placementGroup, Maps.newHashMap()); - } - placementGroupCntMap.compute(placementGroup, - (group, cnt) -> (cnt == null ? 0 : cnt) + 1); - replicaIdMap.get(replicaId).add(replica); - Map> placementGroupReplicaIDMap = - placementGroupReplicaIdMap.get(placementGroup); - placementGroupReplicaIDMap.compute(replicaId, - (rid, placementGroupReplicas) -> { - if (placementGroupReplicas == null) { - placementGroupReplicas = Sets.newHashSet(); - } - placementGroupReplicas.add(replica); - return placementGroupReplicas; - }); - } - - Set replicasToRemove = new HashSet<>(); - List sortedRIDList = replicaIdMap.keySet().stream() - .sorted((o1, o2) -> Integer.compare(replicaIdMap.get(o2).size(), - replicaIdMap.get(o1).size())).collect(Collectors.toList()); - for (Integer rid : sortedRIDList) { - Queue pq = new PriorityQueue<>((o1, o2) -> - Integer.compare(placementGroupCntMap.get(o2), - placementGroupCntMap.get(o1))); - pq.addAll(placementGroupReplicaIdMap.entrySet() - .stream() - .filter(nodeMapEntry -> nodeMapEntry.getValue().containsKey(rid)) - .map(Map.Entry::getKey) - .collect(Collectors.toList())); - - while (replicaIdMap.get(rid).size() > expectedCountPerUniqueReplica) { - Node rack = pq.poll(); - Set replicaSet = - placementGroupReplicaIdMap.get(rack).get(rid); - if (replicaSet.size() > 0) { - ContainerReplica r = replicaSet.stream().findFirst().get(); - replicasToRemove.add(r); - replicaSet.remove(r); - replicaIdMap.get(rid).remove(r); - placementGroupCntMap.compute(rack, - (group, cnt) -> (cnt == null ? 0 : cnt) - 1); - if (replicaSet.size() == 0) { - placementGroupReplicaIdMap.get(rack).remove(rid); - } else { - pq.add(rack); - } - } - } - } - return replicasToRemove; - } @Override public Map replicasToCopy( 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 039cee5ed126..271863cb77c1 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 @@ -102,35 +102,6 @@ public void testReplicasToCopy() { .allMatch(i -> i == 1)); } - @Test - public void testReplicasToRemove() { - DummyPlacementPolicy dummyPlacementPolicy = - new DummyPlacementPolicy(nodeManager, conf); - List list = - nodeManager.getNodes(NodeStatus.inServiceHealthy()); - Set replicas = - IntStream.range(1, 6).mapToObj(i -> - ContainerReplica.newBuilder() - .setContainerID(new ContainerID(1)) - .setContainerState(CLOSED) - .setReplicaIndex(i) - .setDatanodeDetails(list.get(i)).build()) - .collect(Collectors.toSet()); - ContainerReplica replica = ContainerReplica.newBuilder() - .setContainerID(new ContainerID(1)) - .setContainerState(CLOSED) - .setReplicaIndex(1) - .setDatanodeDetails(list.get(7)).build(); - replicas.add(replica); - - Set replicasToRemove = - dummyPlacementPolicy.replicasToRemove(replicas, - 1, 5); - Assertions.assertEquals(replicasToRemove.size(), 1); - Assertions.assertEquals(replicasToRemove.stream().findFirst().get(), - replica); - } - private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { private Map dns; diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index 7e5b265042f4..022c506227a6 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -202,14 +202,6 @@ public Map replicasToCopy( int expectedUniqueGroups) { return null; } - - @Override - public Set replicasToRemove( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { - return null; - } - } @Test diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index 7431a41ee283..a1b3655297b3 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -379,13 +379,6 @@ public Map replicasToCopy( return null; } - @Override - public Set replicasToRemove( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { - return null; - } - private boolean isDnPresent(List dns) { for (DatanodeDetails dn : dns) { From fd3ed13c681b68ff7a5778e4fd318b1284adf52e Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 29 Nov 2022 15:02:21 -0800 Subject: [PATCH 06/23] HDDS-7492. Simplify toCopyFunction --- .../hadoop/hdds/scm/PlacementPolicy.java | 2 +- .../hdds/scm/SCMCommonPlacementPolicy.java | 54 +++++++------------ .../scm/TestSCMCommonPlacementPolicy.java | 20 ++++--- .../TestContainerPlacementFactory.java | 2 +- .../recon/fsck/TestContainerHealthTask.java | 2 +- 5 files changed, 30 insertions(+), 50 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index 0d5438311022..1dc15e5e6d11 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -67,7 +67,7 @@ List chooseDatanodes(List usedNodes, */ ContainerPlacementStatus validateContainerPlacement( List dns, int replicas); - Map replicasToCopy(Set replicas, + Set replicasToCopy(Set replicas, int expectedCountPerUniqueReplica, int expectedUniqueGroups); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 26e4773dcc39..eab29d6557ea 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -442,52 +442,34 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } @Override - public Map replicasToCopy( + public Set replicasToCopy( Set replicas, int expectedCountPerUniqueReplicas, int expectedUniqueGroups) { - Map replicaIdMap = new HashMap<>(); Map> placementGroupReplicaIdMap - = new HashMap<>(); - for (ContainerReplica replica:replicas) { - Integer replicaId = replica.getReplicaIndex(); - Node placementGroup = getPlacementGroup(replica.getDatanodeDetails()); - if (!replicaIdMap.containsKey(replicaId)) { - replicaIdMap.put(replicaId, replica); - } - placementGroupReplicaIdMap.compute(placementGroup, - (rid, placementGroupReplicas) -> { - if (placementGroupReplicas == null) { - placementGroupReplicas = Sets.newHashSet(); - } - placementGroupReplicas.add(replica); - return placementGroupReplicas; - }); - } - int misreplicationCnt = Math.max(getRequiredRackCount( - expectedUniqueGroups * expectedCountPerUniqueReplicas) + = replicas.stream().collect(Collectors.groupingBy(replica -> + this.getPlacementGroup(replica.getDatanodeDetails()), + Collectors.toSet())); + + int totalNumberOfReplicas = + expectedUniqueGroups * expectedCountPerUniqueReplicas; + int requiredNumberOfPlacementGroups = getRequiredRackCount( + expectedUniqueGroups * expectedCountPerUniqueReplicas); + int replicasPerPlacementGroup = + totalNumberOfReplicas/requiredNumberOfPlacementGroups; + int misreplicationCnt = Math.max(requiredNumberOfPlacementGroups - placementGroupReplicaIdMap.size(), 0); - Map copyRIDMap = new HashMap<>(); + Set copyReplicaSet = Sets.newHashSet(); for (Set replicaSet: placementGroupReplicaIdMap .values()) { - if (misreplicationCnt > 0 && replicaSet.size() > 1) { - Map cntMap = replicaSet.stream() + if (misreplicationCnt > copyReplicaSet.size() && replicaSet.size() > + replicasPerPlacementGroup) { + copyReplicaSet.addAll(replicaSet.stream() .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) - .collect(Collectors.groupingBy( - ContainerReplica::getReplicaIndex, - Collectors.counting())); - for (Map.Entry ridCntEntry : cntMap.entrySet()) { - int additionalReplica = Math.toIntExact(ridCntEntry.getValue()); - copyRIDMap.compute(ridCntEntry.getKey(), - (replicaIdx, cnt) -> (cnt == null ? 0 : cnt) - + additionalReplica); - misreplicationCnt -= additionalReplica; - } + .collect(Collectors.toSet())); } } - return copyRIDMap.entrySet().stream() - .collect(Collectors.toMap(e -> replicaIdMap.get(e.getKey()), - Map.Entry::getValue)); + return copyReplicaSet; } protected Node getPlacementGroup(DatanodeDetails dn) { 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 271863cb77c1..db2843b96bdf 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm; +import com.google.common.collect.Sets; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -76,30 +77,27 @@ public void testReplicasToCopy() { new DummyPlacementPolicy(nodeManager, conf); List list = nodeManager.getNodes(NodeStatus.inServiceHealthy()); - Map replicas = + List replicas = IntStream.range(1, 5).mapToObj(i -> ContainerReplica.newBuilder() .setContainerID(new ContainerID(1)) .setContainerState(CLOSED) .setReplicaIndex(i) .setDatanodeDetails(list.get(i - 1)).build()) - .collect(Collectors.toMap(ContainerReplica::getReplicaIndex, - Function.identity())); - replicas.put(5, ContainerReplica.newBuilder() + .collect(Collectors.toList()); + replicas.add(ContainerReplica.newBuilder() .setContainerID(new ContainerID(1)) .setContainerState(CLOSED) .setReplicaIndex(5) .setDatanodeDetails(list.get(5)).build()); - Map replicasToCopy = - dummyPlacementPolicy.replicasToCopy(replicas.values() - .stream().collect(Collectors.toSet()), + Set replicasToCopy = + dummyPlacementPolicy.replicasToCopy(Sets.newHashSet(replicas), 1, 5); - Assertions.assertTrue(replicasToCopy.keySet().stream().findFirst() - .map(replica -> Arrays.asList(replicas.get(1), replicas.get(5)) + Assertions.assertTrue(replicasToCopy.stream().findFirst() + .map(replica -> Arrays.asList(replicas.get(0), replicas.get(4)) .contains(replica)).orElse(false)); - Assertions.assertTrue(replicasToCopy.values().stream() - .allMatch(i -> i == 1)); + Assertions.assertEquals(1, replicasToCopy.size()); } private static class DummyPlacementPolicy extends diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index 022c506227a6..822bcadee83a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -197,7 +197,7 @@ public List chooseDatanodes( } @Override - public Map replicasToCopy( + public Set replicasToCopy( Set replicas, int expectedCountPerUniqueReplica, int expectedUniqueGroups) { return null; diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index a1b3655297b3..d6c00b47901c 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -373,7 +373,7 @@ public ContainerPlacementStatus validateContainerPlacement( } @Override - public Map replicasToCopy( + public Set replicasToCopy( Set replicas, int expectedCountPerUniqueReplica, int expectedUniqueGroups) { return null; From 209429e715fa5d6b4d1ae78fdcc98650b383b282 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 29 Nov 2022 15:10:19 -0800 Subject: [PATCH 07/23] HDDS-7492. Fix Checkstyle Issues --- .../hadoop/hdds/scm/PlacementPolicy.java | 1 - .../hdds/scm/SCMCommonPlacementPolicy.java | 30 ++++++++----------- .../scm/TestSCMCommonPlacementPolicy.java | 1 - .../TestContainerPlacementFactory.java | 1 - .../recon/fsck/TestContainerHealthTask.java | 1 - 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index 1dc15e5e6d11..f59f534003cd 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Set; /** diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index eab29d6557ea..8664d73db79d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -18,22 +18,8 @@ package org.apache.hadoop.hdds.scm; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.PriorityQueue; -import java.util.Queue; -import java.util.Random; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; - +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -47,11 +33,19 @@ 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 com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Random; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + /** * This policy implements a set of invariants which are common * for all basic placement policies, acts as the repository of helper @@ -455,7 +449,7 @@ public Set replicasToCopy( int requiredNumberOfPlacementGroups = getRequiredRackCount( expectedUniqueGroups * expectedCountPerUniqueReplicas); int replicasPerPlacementGroup = - totalNumberOfReplicas/requiredNumberOfPlacementGroups; + totalNumberOfReplicas / requiredNumberOfPlacementGroups; int misreplicationCnt = Math.max(requiredNumberOfPlacementGroups - placementGroupReplicaIdMap.size(), 0); Set copyReplicaSet = Sets.newHashSet(); 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 db2843b96bdf..8105bd7cac1a 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 @@ -40,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index 822bcadee83a..5cc82bd87691 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.Set; import org.apache.hadoop.hdds.conf.OzoneConfiguration; diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index d6c00b47901c..eed65282e32b 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -30,7 +30,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.UUID; From ce6654d268a948b23a3ac1c681044ca68064fdd9 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 2 Dec 2022 09:27:59 -0800 Subject: [PATCH 08/23] HDDS-7492. Address review comments --- .../hadoop/hdds/scm/PlacementPolicy.java | 10 ++-- .../hdds/scm/SCMCommonPlacementPolicy.java | 25 ++++------ .../apache/hadoop/hdds/scm/HddsTestUtils.java | 18 ++++++-- .../scm/TestSCMCommonPlacementPolicy.java | 46 +++++++++---------- .../TestContainerPlacementFactory.java | 8 ++-- .../recon/fsck/TestContainerHealthTask.java | 7 ++- 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index f59f534003cd..daa59c4a8789 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -66,7 +66,11 @@ List chooseDatanodes(List usedNodes, */ ContainerPlacementStatus validateContainerPlacement( List dns, int replicas); - Set replicasToCopy(Set replicas, - int expectedCountPerUniqueReplica, - int expectedUniqueGroups); + + /** + * Given a set of replicas of a container, return a set of replicas to copy + * to another node to fix misreplication. + * @param replicas + */ + Set replicasToCopyToFixMisreplication(Set replicas); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 8664d73db79d..dddd876bff6c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -436,30 +436,27 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } @Override - public Set replicasToCopy( - Set replicas, int expectedCountPerUniqueReplicas, - int expectedUniqueGroups) { - Map> placementGroupReplicaIdMap + public Set replicasToCopyToFixMisreplication( + Set replicas) { + Map> placementGroupReplicaIdMap = replicas.stream().collect(Collectors.groupingBy(replica -> - this.getPlacementGroup(replica.getDatanodeDetails()), - Collectors.toSet())); + this.getPlacementGroup(replica.getDatanodeDetails()))); - int totalNumberOfReplicas = - expectedUniqueGroups * expectedCountPerUniqueReplicas; + int totalNumberOfReplicas = replicas.size(); int requiredNumberOfPlacementGroups = getRequiredRackCount( - expectedUniqueGroups * expectedCountPerUniqueReplicas); + totalNumberOfReplicas); int replicasPerPlacementGroup = totalNumberOfReplicas / requiredNumberOfPlacementGroups; int misreplicationCnt = Math.max(requiredNumberOfPlacementGroups - placementGroupReplicaIdMap.size(), 0); Set copyReplicaSet = Sets.newHashSet(); - for (Set replicaSet: placementGroupReplicaIdMap + for (List replicaList: placementGroupReplicaIdMap .values()) { - if (misreplicationCnt > copyReplicaSet.size() && replicaSet.size() > + if (misreplicationCnt > copyReplicaSet.size() && replicaList.size() > replicasPerPlacementGroup) { - copyReplicaSet.addAll(replicaSet.stream() - .limit(Math.min(replicaSet.size() - 1, misreplicationCnt)) + copyReplicaSet.addAll(replicaList.stream() + .limit(Math.min(replicaList.size() - 1, misreplicationCnt)) .collect(Collectors.toSet())); } } @@ -469,6 +466,4 @@ public Set replicasToCopy( protected Node getPlacementGroup(DatanodeDetails dn) { return nodeManager.getClusterNetworkTopologyMap().getAncestor(dn, 1); } - - } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java index 99503679726f..165dac0e4442 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm; import com.google.common.base.Preconditions; +import com.google.common.collect.Sets; import org.apache.commons.lang3.RandomUtils; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.RatisReplicationConfig; @@ -744,14 +745,14 @@ public static ContainerReplica getReplicas( return builder.build(); } - public static Set getReplicasWithReplicaIndex( + public static List getReplicasWithReplicaIndex( final ContainerID containerId, final ContainerReplicaProto.State state, final long usedBytes, final long keyCount, final long sequenceId, - final DatanodeDetails... datanodeDetails) { - Set replicas = new HashSet<>(); + final Iterable datanodeDetails) { + List replicas = new ArrayList<>(); int replicaIndex = 1; for (DatanodeDetails datanode : datanodeDetails) { replicas.add(getReplicaBuilder(containerId, state, @@ -762,6 +763,17 @@ public static Set getReplicasWithReplicaIndex( return replicas; } + public static Set getReplicasWithReplicaIndex( + final ContainerID containerId, + final ContainerReplicaProto.State state, + final long usedBytes, + final long keyCount, + final long sequenceId, + final DatanodeDetails... datanodeDetails) { + return Sets.newHashSet(getReplicasWithReplicaIndex(containerId, state, + usedBytes, keyCount, sequenceId, Arrays.asList(datanodeDetails))); + } + public static Pipeline getRandomPipeline() { 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 8105bd7cac1a..cd27c60b2f62 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 @@ -42,6 +42,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.util.stream.Stream; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; @@ -62,7 +63,7 @@ public void setup() { @Test public void testGetResultSet() throws SCMException { DummyPlacementPolicy dummyPlacementPolicy = - new DummyPlacementPolicy(nodeManager, conf); + new DummyPlacementPolicy(nodeManager, conf, 5); List list = nodeManager.getNodes(NodeStatus.inServiceHealthy()); List result = dummyPlacementPolicy.getResultSet(3, list); @@ -71,47 +72,44 @@ public void testGetResultSet() throws SCMException { } @Test - public void testReplicasToCopy() { + public void testReplicasToFixMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = - new DummyPlacementPolicy(nodeManager, conf); + new DummyPlacementPolicy(nodeManager, conf, 5); List list = nodeManager.getNodes(NodeStatus.inServiceHealthy()); + List replicaDns = + Stream.of(0, 1, 2, 3, 4, 5) + .map(list::get).collect(Collectors.toList()); + + List replicas = - IntStream.range(1, 5).mapToObj(i -> - ContainerReplica.newBuilder() - .setContainerID(new ContainerID(1)) - .setContainerState(CLOSED) - .setReplicaIndex(i) - .setDatanodeDetails(list.get(i - 1)).build()) - .collect(Collectors.toList()); - replicas.add(ContainerReplica.newBuilder() - .setContainerID(new ContainerID(1)) - .setContainerState(CLOSED) - .setReplicaIndex(5) - .setDatanodeDetails(list.get(5)).build()); - - Set replicasToCopy = - dummyPlacementPolicy.replicasToCopy(Sets.newHashSet(replicas), - 1, 5); + HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), + CLOSED, 0, 0, 0, replicaDns); + + Set replicasToCopy = dummyPlacementPolicy + .replicasToCopyToFixMisreplication(Sets.newHashSet(replicas)); Assertions.assertTrue(replicasToCopy.stream().findFirst() .map(replica -> Arrays.asList(replicas.get(0), replicas.get(4)) .contains(replica)).orElse(false)); Assertions.assertEquals(1, replicasToCopy.size()); } - private static class DummyPlacementPolicy extends - SCMCommonPlacementPolicy { + private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { private Map dns; + private int rackCnt; DummyPlacementPolicy( NodeManager nodeManager, - ConfigurationSource conf) { + ConfigurationSource conf, + int rackCnt) { super(nodeManager, conf); dns = new HashMap<>(); List datanodeDetails = nodeManager.getNodes(NodeStatus.inServiceHealthy()); + this.rackCnt = Math.min(rackCnt, datanodeDetails.size()); for (int idx = 0; idx < datanodeDetails.size(); idx++) { - dns.put(datanodeDetails.get(idx), datanodeDetails.get(idx % 5)); + dns.put(datanodeDetails.get(idx), datanodeDetails.get(idx % + this.rackCnt)); } } @@ -127,7 +125,7 @@ public Node getPlacementGroup(DatanodeDetails dn) { @Override protected int getRequiredRackCount(int numReplicas) { - return 5; + return Math.min(numReplicas, rackCnt); } } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java index 5cc82bd87691..254a20e48071 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementFactory.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -196,10 +197,9 @@ public List chooseDatanodes( } @Override - public Set replicasToCopy( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { - return null; + public Set replicasToCopyToFixMisreplication( + Set replicas) { + return Collections.emptySet(); } } diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java index eed65282e32b..27de4c743a67 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTask.java @@ -372,10 +372,9 @@ public ContainerPlacementStatus validateContainerPlacement( } @Override - public Set replicasToCopy( - Set replicas, int expectedCountPerUniqueReplica, - int expectedUniqueGroups) { - return null; + public Set replicasToCopyToFixMisreplication( + Set replicas) { + return Collections.emptySet(); } From 045fbb8936868d040ba769682cb339f84eb99835 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 2 Dec 2022 09:38:10 -0800 Subject: [PATCH 09/23] HDDS-7492. Address review comments --- .../apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 7 +++++-- .../hadoop/hdds/scm/TestSCMCommonPlacementPolicy.java | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index dddd876bff6c..666d270ac1c0 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -63,8 +63,6 @@ public abstract class SCMCommonPlacementPolicy implements private final ConfigurationSource conf; private final boolean shouldRemovePeers; - private Function replicaIdentifierFunction; - /** * Return for replication factor 1 containers where the placement policy * is always met, or not met (zero replicas available) rather than creating a @@ -435,6 +433,11 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, return false; } + /** + * Given a set of replicas of a container, return a set of replicas to copy + * to another node to fix misreplication. + * @param replicas + */ @Override public Set replicasToCopyToFixMisreplication( Set replicas) { 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 cd27c60b2f62..bf204f218ef7 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 @@ -41,7 +41,6 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; From 0e107f788c82ac915b75b568820f62e3b439d090 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 2 Dec 2022 10:42:34 -0800 Subject: [PATCH 10/23] HDDS-7492. Adding test cases --- .../apache/hadoop/hdds/scm/HddsTestUtils.java | 13 ++- .../scm/TestSCMCommonPlacementPolicy.java | 95 +++++++++++++++---- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java index 165dac0e4442..65427bcb454c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java @@ -692,11 +692,20 @@ public static Set getReplicas( } public static Set getReplicas( + final ContainerID containerId, + final ContainerReplicaProto.State state, + final long sequenceId, + final DatanodeDetails... datanodeDetails) { + return Sets.newHashSet(getReplicas(containerId, state, sequenceId, + Arrays.asList(datanodeDetails))); + } + + public static List getReplicas( final ContainerID containerId, final ContainerReplicaProto.State state, final long sequenceId, - final DatanodeDetails... datanodeDetails) { - Set replicas = new HashSet<>(); + final Iterable datanodeDetails) { + List replicas = new ArrayList<>(); for (DatanodeDetails datanode : datanodeDetails) { replicas.add(getReplicas(containerId, state, sequenceId, datanode.getUuid(), datanode)); 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 bf204f218ef7..d54455dd30f7 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 @@ -30,16 +30,13 @@ import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.scm.node.NodeStatus; import org.apache.hadoop.ozone.container.common.SCMTestUtils; +import org.apache.ratis.thirdparty.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; -import java.util.Arrays; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -70,14 +67,77 @@ public void testGetResultSet() throws SCMException { Assertions.assertNotEquals(1, resultSet.size()); } + private void testReplicasToFixMisreplication( + List replicas, + DummyPlacementPolicy placementPolicy, + int expectedNumberOfReplicasToCopy, + Map expectedNumberOfCopyOperationFromRack) { + Set replicasToCopy = placementPolicy + .replicasToCopyToFixMisreplication(Sets.newHashSet(replicas)); + Assertions.assertEquals(expectedNumberOfReplicasToCopy, + replicasToCopy.size()); + Map rackCopyMap = + replicasToCopy.stream().collect(Collectors.groupingBy( + replica -> placementPolicy + .getPlacementGroup(replica.getDatanodeDetails()), + Collectors.counting())); + Set racks = replicas.stream() + .map(ContainerReplica::getDatanodeDetails) + .map(placementPolicy::getPlacementGroup) + .collect(Collectors.toSet()); + for(Node rack: racks) { + Assertions.assertEquals( + expectedNumberOfCopyOperationFromRack.getOrDefault(rack, 0), + rackCopyMap.getOrDefault(rack, 0l).intValue()); + } + } + @Test public void testReplicasToFixMisreplication() { + DummyPlacementPolicy dummyPlacementPolicy = + new DummyPlacementPolicy(nodeManager, conf, 5); + List racks = dummyPlacementPolicy.racks; + List list = + nodeManager.getNodes(NodeStatus.inServiceHealthy()); + List replicaDns = + Stream.of(0, 1, 2, 3, 5) + .map(list::get).collect(Collectors.toList()); + List replicas = + HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), + CLOSED, 0, 0, 0, replicaDns); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 1, + ImmutableMap.of(racks.get(0), 1)); + //Changing Rack of Dn 1 to move to rack 0 + dummyPlacementPolicy.rackMap.put(list.get(1), + dummyPlacementPolicy.getPlacementGroup(list.get(0))); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, + ImmutableMap.of(racks.get(0), 2)); + //Changing Rack of Dn 2 to move to rack 0 + dummyPlacementPolicy.rackMap.put(list.get(2), + dummyPlacementPolicy.getPlacementGroup(list.get(0))); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, + ImmutableMap.of(racks.get(0), 3)); + //Changing Rack of Dn 4 to move to rack 3 + dummyPlacementPolicy.rackMap.put(list.get(4), + dummyPlacementPolicy.getPlacementGroup(list.get(3))); + replicaDns = + Stream.of(0, 1, 2, 3, 4) + .map(list::get).collect(Collectors.toList()); + //Creating Replicas without replica Index + replicas = HddsTestUtils.getReplicas(new ContainerID(1), + CLOSED, 0, replicaDns); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, + ImmutableMap.of(racks.get(0), 2, racks.get(3), 1)); + } + + @Test + public void testReplicasWithoutMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); List list = nodeManager.getNodes(NodeStatus.inServiceHealthy()); List replicaDns = - Stream.of(0, 1, 2, 3, 4, 5) + Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); @@ -87,14 +147,14 @@ public void testReplicasToFixMisreplication() { Set replicasToCopy = dummyPlacementPolicy .replicasToCopyToFixMisreplication(Sets.newHashSet(replicas)); - Assertions.assertTrue(replicasToCopy.stream().findFirst() - .map(replica -> Arrays.asList(replicas.get(0), replicas.get(4)) - .contains(replica)).orElse(false)); - Assertions.assertEquals(1, replicasToCopy.size()); + Assertions.assertEquals(0, replicasToCopy.size()); } + + private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { - private Map dns; + private Map rackMap; + private List racks; private int rackCnt; DummyPlacementPolicy( @@ -102,13 +162,16 @@ private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { ConfigurationSource conf, int rackCnt) { super(nodeManager, conf); - dns = new HashMap<>(); + rackMap = new HashMap<>(); List datanodeDetails = nodeManager.getNodes(NodeStatus.inServiceHealthy()); this.rackCnt = Math.min(rackCnt, datanodeDetails.size()); + this.racks = new ArrayList<>(this.rackCnt); + for (int r = 0; r < this.rackCnt; r++) { + racks.add(Mockito.mock(Node.class)); + } for (int idx = 0; idx < datanodeDetails.size(); idx++) { - dns.put(datanodeDetails.get(idx), datanodeDetails.get(idx % - this.rackCnt)); + rackMap.put(datanodeDetails.get(idx), racks.get(idx % this.rackCnt)); } } @@ -119,7 +182,7 @@ public DatanodeDetails chooseNode(List healthyNodes) { @Override public Node getPlacementGroup(DatanodeDetails dn) { - return dns.get(dn); + return rackMap.get(dn); } @Override From b261b17c938113c941276a07668d63e6471c4b9a Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 2 Dec 2022 10:46:39 -0800 Subject: [PATCH 11/23] HDDS-7492. Fix Checkstyle Issues --- .../hdds/scm/SCMCommonPlacementPolicy.java | 1 - .../apache/hadoop/hdds/scm/HddsTestUtils.java | 1 - .../hdds/scm/TestSCMCommonPlacementPolicy.java | 17 +++++++++++------ 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 666d270ac1c0..d365136a442c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -43,7 +43,6 @@ import java.util.Objects; import java.util.Random; import java.util.Set; -import java.util.function.Function; import java.util.stream.Collectors; /** diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java index 65427bcb454c..eb0741662c60 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java @@ -85,7 +85,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.UUID; 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 d54455dd30f7..baac3203ad24 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 @@ -36,7 +36,12 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -78,17 +83,17 @@ private void testReplicasToFixMisreplication( replicasToCopy.size()); Map rackCopyMap = replicasToCopy.stream().collect(Collectors.groupingBy( - replica -> placementPolicy - .getPlacementGroup(replica.getDatanodeDetails()), + replica -> placementPolicy + .getPlacementGroup(replica.getDatanodeDetails()), Collectors.counting())); Set racks = replicas.stream() .map(ContainerReplica::getDatanodeDetails) .map(placementPolicy::getPlacementGroup) .collect(Collectors.toSet()); - for(Node rack: racks) { + for (Node rack: racks) { Assertions.assertEquals( expectedNumberOfCopyOperationFromRack.getOrDefault(rack, 0), - rackCopyMap.getOrDefault(rack, 0l).intValue()); + rackCopyMap.getOrDefault(rack, 0L).intValue()); } } @@ -123,7 +128,7 @@ public void testReplicasToFixMisreplication() { replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); - //Creating Replicas without replica Index + //Creating Replicas without replica Index for ratis case replicas = HddsTestUtils.getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, From 96ece79cf0d1693674c3c1802252c4862b09459b Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 2 Dec 2022 10:53:10 -0800 Subject: [PATCH 12/23] HDDS-7492. Add testcases --- .../scm/TestSCMCommonPlacementPolicy.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) 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 baac3203ad24..fb78b104bf5f 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 @@ -128,7 +128,25 @@ public void testReplicasToFixMisreplication() { replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); - //Creating Replicas without replica Index for ratis case + //Creating Replicas without replica Index + replicas = HddsTestUtils.getReplicas(new ContainerID(1), + CLOSED, 0, replicaDns); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, + ImmutableMap.of(racks.get(0), 2, racks.get(3), 1)); + //Creating Replicas without replica Index for replicas < number of racks + replicaDns = + Stream.of(0, 1, 3, 4) + .map(list::get).collect(Collectors.toList()); + replicas = HddsTestUtils.getReplicas(new ContainerID(1), + CLOSED, 0, replicaDns); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, + ImmutableMap.of(racks.get(0), 1, racks.get(3), 1)); + + //Creating Replicas without replica Index for replicas > number of racks + replicaDns = + Stream.of(0, 1, 2, 3, 4, 6) + .map(list::get).collect(Collectors.toList()); + replicas = HddsTestUtils.getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, From 923d94043073ad4805c4fd1aee2d63e9f5143b09 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 5 Dec 2022 07:53:11 -0700 Subject: [PATCH 13/23] HDDS-7492. Update Javadoc --- .../java/org/apache/hadoop/hdds/scm/PlacementPolicy.java | 5 +++-- .../org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java index daa59c4a8789..af41a157400b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/PlacementPolicy.java @@ -68,8 +68,9 @@ ContainerPlacementStatus validateContainerPlacement( List dns, int replicas); /** - * Given a set of replicas of a container, return a set of replicas to copy - * to another node to fix misreplication. + * Given a set of replicas of a container which are + * neither over underreplicated nor overreplicated, + * return a set of replicas to copy to another node to fix misreplication. * @param replicas */ Set replicasToCopyToFixMisreplication(Set replicas); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index d365136a442c..32d99a532ead 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -433,8 +433,9 @@ public boolean isValidNode(DatanodeDetails datanodeDetails, } /** - * Given a set of replicas of a container, return a set of replicas to copy - * to another node to fix misreplication. + * Given a set of replicas of a container which are + * neither over underreplicated nor overreplicated, + * return a set of replicas to copy to another node to fix misreplication. * @param replicas */ @Override From d3f052e887df9c977e9ae237604c6c3a0e2fa0a2 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Mon, 5 Dec 2022 12:26:48 -0700 Subject: [PATCH 14/23] HDDS-7492. Add Max Replicas per Rack in SCM CommonPlacement Policy for misreplication --- .../hdds/scm/SCMCommonPlacementPolicy.java | 75 +++++++++++++++---- .../ContainerPlacementStatusDefault.java | 36 +++++++-- .../scm/pipeline/PipelinePlacementPolicy.java | 5 ++ .../scm/TestSCMCommonPlacementPolicy.java | 28 +++++-- 4 files changed, 118 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 32d99a532ead..1f8ccefad096 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -41,6 +41,9 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.PriorityQueue; +import java.util.Queue; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; @@ -351,6 +354,21 @@ protected int getRequiredRackCount(int numReplicas) { return 1; } + /** + * Default implementation to return the max number of replicas per rack. + * For simple policies that are not rack aware + * we return numReplicas, from this default implementation. + * + * @param numReplicas - The desired replica counts + * @return The max number of replicas per rack + */ + protected int getMaxReplicasPerRack(int numReplicas) { + return numReplicas / getRequiredRackCount(numReplicas) + + Math.min(numReplicas % getRequiredRackCount(numReplicas), 1); + } + + + /** * This default implementation handles rack aware policies and non rack * aware policies. If a future placement policy needs to check more than racks @@ -369,6 +387,7 @@ public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); int requiredRacks = getRequiredRackCount(replicas); + int maxReplicasPerRack = getMaxReplicasPerRack(replicas); if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. @@ -383,16 +402,17 @@ public ContainerPlacementStatus validateContainerPlacement( // The leaf nodes are all at max level, so the number of nodes at // leafLevel - 1 is the rack count numRacks = topology.getNumOfNodes(maxLevel - 1); - final long currentRackCount = dns.stream() - .map(this::getPlacementGroup) - .distinct() - .count(); + Map currentRackCount = dns.stream() + .collect(Collectors.groupingBy(this::getPlacementGroup, + Collectors.counting())); if (replicas < requiredRacks) { requiredRacks = replicas; } return new ContainerPlacementStatusDefault( - (int)currentRackCount, requiredRacks, numRacks); + currentRackCount.size(), requiredRacks, numRacks, maxReplicasPerRack, + currentRackCount.values().stream().map(Long::intValue) + .collect(Collectors.toList())); } /** @@ -446,21 +466,46 @@ public Set replicasToCopyToFixMisreplication( this.getPlacementGroup(replica.getDatanodeDetails()))); int totalNumberOfReplicas = replicas.size(); - int requiredNumberOfPlacementGroups = getRequiredRackCount( - totalNumberOfReplicas); + int requiredNumberOfPlacementGroups = + getRequiredRackCount(totalNumberOfReplicas); + int additionalNumberOfRacksRequired = Math.max( + requiredNumberOfPlacementGroups - placementGroupReplicaIdMap.size(), + 0); int replicasPerPlacementGroup = - totalNumberOfReplicas / requiredNumberOfPlacementGroups; - int misreplicationCnt = Math.max(requiredNumberOfPlacementGroups - - placementGroupReplicaIdMap.size(), 0); + getMaxReplicasPerRack(totalNumberOfReplicas); Set copyReplicaSet = Sets.newHashSet(); for (List replicaList: placementGroupReplicaIdMap .values()) { - if (misreplicationCnt > copyReplicaSet.size() && replicaList.size() > - replicasPerPlacementGroup) { - copyReplicaSet.addAll(replicaList.stream() - .limit(Math.min(replicaList.size() - 1, misreplicationCnt)) - .collect(Collectors.toSet())); + if (replicaList.size() > replicasPerPlacementGroup) { + List replicasToBeCopied = replicaList.stream() + .limit(replicaList.size() - replicasPerPlacementGroup) + .collect(Collectors.toList()); + copyReplicaSet.addAll(replicasToBeCopied); + replicaList.removeAll(replicasToBeCopied); + } + } + if (additionalNumberOfRacksRequired > copyReplicaSet.size()) { + additionalNumberOfRacksRequired -= copyReplicaSet.size(); + Queue> placementGroupReplicas = + new PriorityQueue<>((o1, o2) -> + Integer.compare(o2.size(), o1.size())); + placementGroupReplicas.addAll(placementGroupReplicaIdMap.values()); + while (placementGroupReplicas.size() > 0 + && additionalNumberOfRacksRequired > 0) { + List replicaList = placementGroupReplicas.poll(); + int numberOfReplicasToBeCopied = Math.max(1, + Math.min(replicaList.size() + - Optional.ofNullable(placementGroupReplicas.peek()) + .map(List::size).orElse(0), + additionalNumberOfRacksRequired)); + List replicasToBeCopied = replicaList.stream() + .limit(numberOfReplicasToBeCopied) + .collect(Collectors.toList()); + copyReplicaSet.addAll(replicasToBeCopied); + replicaList.removeAll(replicasToBeCopied); + placementGroupReplicas.add(replicaList); + additionalNumberOfRacksRequired -= replicasToBeCopied.size(); } } return copyReplicaSet; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java index 3fdcd2f40169..159c9f5af834 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java @@ -18,6 +18,9 @@ import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; +import java.util.Collections; +import java.util.List; + /** * Simple Status object to check if a container is replicated across enough * racks. @@ -29,16 +32,32 @@ public class ContainerPlacementStatusDefault private final int currentRacks; private final int totalRacks; + private final int maxReplicasPerRack; + private final List rackReplicaCnts; + + public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks, - int totalRacks) { + int totalRacks, int maxReplicasPerRack, List rackReplicaCnts) { this.requiredRacks = requiredRacks; this.currentRacks = currentRacks; this.totalRacks = totalRacks; + this.maxReplicasPerRack = maxReplicasPerRack; + this.rackReplicaCnts = rackReplicaCnts; + } + + public ContainerPlacementStatusDefault(int requiredRacks, int currentRacks, + int totalRacks) { + this(requiredRacks, currentRacks, totalRacks, 1, + currentRacks == 0 ? Collections.emptyList() + : Collections.nCopies(currentRacks, 1)); } @Override public boolean isPolicySatisfied() { - return currentRacks >= totalRacks || currentRacks >= requiredRacks; + if (currentRacks < Math.min(totalRacks, requiredRacks)) { + return false; + } + return rackReplicaCnts.stream().allMatch(cnt -> cnt <= maxReplicasPerRack); } @Override @@ -46,8 +65,13 @@ public String misReplicatedReason() { if (isPolicySatisfied()) { return null; } - return "The container is mis-replicated as it is on " + currentRacks + - " racks but should be on " + requiredRacks + " racks."; + if (currentRacks < Math.min(requiredRacks, maxReplicasPerRack)) { + return "The container is mis-replicated as it is on " + currentRacks + + " racks but should be on " + requiredRacks + " racks."; + } + return "The container is mis-replicated as max number of replicas per rack " + + "is " + maxReplicasPerRack + " but number of replicas per rack" + + " are " + rackReplicaCnts.toString(); } @Override @@ -55,7 +79,9 @@ public int misReplicationCount() { if (isPolicySatisfied()) { return 0; } - return requiredRacks - currentRacks; + return Math.max(requiredRacks - currentRacks, + rackReplicaCnts.stream().mapToInt( + cnt -> Math.max(maxReplicasPerRack - cnt, 0)).sum()); } @Override diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java index befc0543a357..3b36510517f0 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java @@ -105,6 +105,11 @@ private boolean isNonClosedRatisThreePipeline(Pipeline p) { && !p.isClosed(); } + @Override + protected int getMaxReplicasPerRack(int numReplicas) { + return numReplicas - 1; + } + /** * Filter out viable nodes based on * 1. nodes that are healthy 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 fb78b104bf5f..69e9dbde4b57 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; @@ -102,8 +103,7 @@ public void testReplicasToFixMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); List racks = dummyPlacementPolicy.racks; - List list = - nodeManager.getNodes(NodeStatus.inServiceHealthy()); + List list = nodeManager.getAllNodes(); List replicaDns = Stream.of(0, 1, 2, 3, 5) .map(list::get).collect(Collectors.toList()); @@ -149,8 +149,25 @@ public void testReplicasToFixMisreplication() { replicas = HddsTestUtils.getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); - testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, - ImmutableMap.of(racks.get(0), 2, racks.get(3), 1)); + + int idx = 0; + Map expectedRackCnt = Maps.newHashMap(); + expectedRackCnt.put(racks.get(0), 1); + expectedRackCnt.put(racks.get(3), 0); + expectedRackCnt.compute(expectedRackCnt.keySet().stream().findFirst().get(), + (node, integer) -> integer + 1); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, + expectedRackCnt); + dummyPlacementPolicy = + new DummyPlacementPolicy(nodeManager, conf, 2); + racks = dummyPlacementPolicy.racks; + replicaDns = Stream.of(0, 2, 4, 6, 8) + .map(list::get).collect(Collectors.toList()); + replicas = HddsTestUtils.getReplicas(new ContainerID(1), + CLOSED, 0, replicaDns); + testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, + ImmutableMap.of(racks.get(0), 2)); + } @Test @@ -186,8 +203,7 @@ private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { int rackCnt) { super(nodeManager, conf); rackMap = new HashMap<>(); - List datanodeDetails = - nodeManager.getNodes(NodeStatus.inServiceHealthy()); + List datanodeDetails = nodeManager.getAllNodes(); this.rackCnt = Math.min(rackCnt, datanodeDetails.size()); this.racks = new ArrayList<>(this.rackCnt); for (int r = 0; r < this.rackCnt; r++) { From 8f023aaa2e0569a4c63432d6c00deb46be1f46a1 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 6 Dec 2022 12:04:46 -0700 Subject: [PATCH 15/23] HDDS-7492. Change algorithm to support max replicas per rack simplify to reduce lines of code --- .../hdds/scm/SCMCommonPlacementPolicy.java | 56 +++++++------------ .../scm/pipeline/PipelinePlacementPolicy.java | 4 +- .../scm/TestSCMCommonPlacementPolicy.java | 8 +-- 3 files changed, 22 insertions(+), 46 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 1f8ccefad096..dc762770f341 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -360,11 +361,12 @@ protected int getRequiredRackCount(int numReplicas) { * we return numReplicas, from this default implementation. * * @param numReplicas - The desired replica counts + * @param numberOfRacks - The desired number of racks * @return The max number of replicas per rack */ - protected int getMaxReplicasPerRack(int numReplicas) { - return numReplicas / getRequiredRackCount(numReplicas) - + Math.min(numReplicas % getRequiredRackCount(numReplicas), 1); + protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + return numReplicas / numberOfRacks + + Math.min(numReplicas % numberOfRacks, 1); } @@ -387,7 +389,7 @@ public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); int requiredRacks = getRequiredRackCount(replicas); - int maxReplicasPerRack = getMaxReplicasPerRack(replicas); + int maxReplicasPerRack = getMaxReplicasPerRack(replicas, requiredRacks); if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. @@ -468,44 +470,24 @@ public Set replicasToCopyToFixMisreplication( int totalNumberOfReplicas = replicas.size(); int requiredNumberOfPlacementGroups = getRequiredRackCount(totalNumberOfReplicas); - int additionalNumberOfRacksRequired = Math.max( - requiredNumberOfPlacementGroups - placementGroupReplicaIdMap.size(), - 0); - int replicasPerPlacementGroup = - getMaxReplicasPerRack(totalNumberOfReplicas); Set copyReplicaSet = Sets.newHashSet(); - - for (List replicaList: placementGroupReplicaIdMap - .values()) { - if (replicaList.size() > replicasPerPlacementGroup) { - List replicasToBeCopied = replicaList.stream() - .limit(replicaList.size() - replicasPerPlacementGroup) - .collect(Collectors.toList()); - copyReplicaSet.addAll(replicasToBeCopied); - replicaList.removeAll(replicasToBeCopied); - } - } - if (additionalNumberOfRacksRequired > copyReplicaSet.size()) { - additionalNumberOfRacksRequired -= copyReplicaSet.size(); - Queue> placementGroupReplicas = - new PriorityQueue<>((o1, o2) -> - Integer.compare(o2.size(), o1.size())); - placementGroupReplicas.addAll(placementGroupReplicaIdMap.values()); - while (placementGroupReplicas.size() > 0 - && additionalNumberOfRacksRequired > 0) { - List replicaList = placementGroupReplicas.poll(); - int numberOfReplicasToBeCopied = Math.max(1, - Math.min(replicaList.size() - - Optional.ofNullable(placementGroupReplicas.peek()) - .map(List::size).orElse(0), - additionalNumberOfRacksRequired)); + List> replicaSet = placementGroupReplicaIdMap + .values().stream() + .sorted((o1, o2) -> Integer.compare(o2.size(), o1.size())) + .collect(Collectors.toList()); + for (List replicaList: replicaSet) { + int maxReplicasPerPlacementGroup = getMaxReplicasPerRack( + totalNumberOfReplicas, requiredNumberOfPlacementGroups); + int numberOfReplicasToBeCopied = Math.max(0, + replicaList.size() - maxReplicasPerPlacementGroup); + totalNumberOfReplicas -= Math.min(replicaList.size(), + maxReplicasPerPlacementGroup); + requiredNumberOfPlacementGroups -= 1; + if (numberOfReplicasToBeCopied > 0) { List replicasToBeCopied = replicaList.stream() .limit(numberOfReplicasToBeCopied) .collect(Collectors.toList()); copyReplicaSet.addAll(replicasToBeCopied); - replicaList.removeAll(replicasToBeCopied); - placementGroupReplicas.add(replicaList); - additionalNumberOfRacksRequired -= replicasToBeCopied.size(); } } return copyReplicaSet; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java index 3b36510517f0..32262b6f3bf0 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java @@ -106,8 +106,8 @@ private boolean isNonClosedRatisThreePipeline(Pipeline p) { } @Override - protected int getMaxReplicasPerRack(int numReplicas) { - return numReplicas - 1; + protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + return Math.max(numReplicas - 1, 1); } /** 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 69e9dbde4b57..0d8c4b4ee77e 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 @@ -150,14 +150,8 @@ public void testReplicasToFixMisreplication() { replicas = HddsTestUtils.getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); - int idx = 0; - Map expectedRackCnt = Maps.newHashMap(); - expectedRackCnt.put(racks.get(0), 1); - expectedRackCnt.put(racks.get(3), 0); - expectedRackCnt.compute(expectedRackCnt.keySet().stream().findFirst().get(), - (node, integer) -> integer + 1); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, - expectedRackCnt); + ImmutableMap.of(racks.get(0), 1, racks.get(3), 1)); dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 2); racks = dummyPlacementPolicy.racks; From 0d231eb6dc2adecd863f8d363f1d2d3a0097da04 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 6 Dec 2022 23:37:22 -0700 Subject: [PATCH 16/23] HDDS-7492. Fix bug to get non zero denominator for number of required placement groups --- .../org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index dc762770f341..60a112175fee 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -474,6 +474,7 @@ public Set replicasToCopyToFixMisreplication( List> replicaSet = placementGroupReplicaIdMap .values().stream() .sorted((o1, o2) -> Integer.compare(o2.size(), o1.size())) + .limit(requiredNumberOfPlacementGroups) .collect(Collectors.toList()); for (List replicaList: replicaSet) { int maxReplicasPerPlacementGroup = getMaxReplicasPerRack( From 15472d064320a711d555205c68774ffb63d3ad56 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 6 Dec 2022 23:37:38 -0700 Subject: [PATCH 17/23] HDDS-7492. Fix testcases --- .../scm/TestSCMCommonPlacementPolicy.java | 189 +++++++++++++----- .../apache/ozone/test/GenericTestUtils.java | 10 + 2 files changed, 144 insertions(+), 55 deletions(-) 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 0d8c4b4ee77e..ae11364a3ba3 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 @@ -18,8 +18,10 @@ package org.apache.hadoop.hdds.scm; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import javafx.util.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -31,6 +33,7 @@ import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.scm.node.NodeStatus; import org.apache.hadoop.ozone.container.common.SCMTestUtils; +import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.thirdparty.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -38,12 +41,15 @@ import org.mockito.Mockito; import java.util.ArrayList; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; @@ -66,8 +72,7 @@ public void setup() { public void testGetResultSet() throws SCMException { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = - nodeManager.getNodes(NodeStatus.inServiceHealthy()); + List list = nodeManager.getAllNodes(); List result = dummyPlacementPolicy.getResultSet(3, list); Set resultSet = new HashSet<>(result); Assertions.assertNotEquals(1, resultSet.size()); @@ -99,82 +104,150 @@ private void testReplicasToFixMisreplication( } @Test - public void testReplicasToFixMisreplication() { + public void testReplicasToFixMisreplicationWithOneMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); List racks = dummyPlacementPolicy.racks; List list = nodeManager.getAllNodes(); - List replicaDns = - Stream.of(0, 1, 2, 3, 5) + List replicaDns = Stream.of(0, 1, 2, 3, 5) .map(list::get).collect(Collectors.toList()); List replicas = HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), CLOSED, 0, 0, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 1, ImmutableMap.of(racks.get(0), 1)); - //Changing Rack of Dn 1 to move to rack 0 - dummyPlacementPolicy.rackMap.put(list.get(1), - dummyPlacementPolicy.getPlacementGroup(list.get(0))); + } + + @Test + public void testReplicasToFixMisreplicationWithTwoMisreplication() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( + nodeManager, conf, + GenericTestUtils.getReverseMap( + ImmutableMap.of(0, ImmutableList.of(0, 1, 5), + 1, ImmutableList.of(6), + 2, ImmutableList.of(2, 7), + 3, ImmutableList.of(3, 8), + 4, ImmutableList.of(4, 9))), 5); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 2, 3, 5) + .map(list::get).collect(Collectors.toList()); + List replicas = + HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), + CLOSED, 0, 0, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, ImmutableMap.of(racks.get(0), 2)); - //Changing Rack of Dn 2 to move to rack 0 - dummyPlacementPolicy.rackMap.put(list.get(2), - dummyPlacementPolicy.getPlacementGroup(list.get(0))); + } + + @Test + public void testReplicasToFixMisreplicationWithThreeMisreplication() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( + nodeManager, conf, + GenericTestUtils.getReverseMap( + ImmutableMap.of(0, ImmutableList.of(0, 1, 2, 5), + 1, ImmutableList.of(6), + 2, ImmutableList.of(7), + 3, ImmutableList.of(3, 8), + 4, ImmutableList.of(4, 9))), 5); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 2, 3, 5) + .map(list::get).collect(Collectors.toList()); + List replicas = + HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), + CLOSED, 0, 0, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, ImmutableMap.of(racks.get(0), 3)); - //Changing Rack of Dn 4 to move to rack 3 - dummyPlacementPolicy.rackMap.put(list.get(4), - dummyPlacementPolicy.getPlacementGroup(list.get(3))); - replicaDns = - Stream.of(0, 1, 2, 3, 4) + } + + @Test + public void + testReplicasToFixMisreplicationWithThreeMisreplicationOnDifferentRack() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( + nodeManager, conf, + GenericTestUtils.getReverseMap( + ImmutableMap.of(0, ImmutableList.of(0, 1, 2, 5), + 1, ImmutableList.of(6), + 2, ImmutableList.of(7), + 3, ImmutableList.of(3, 4, 8), + 4, ImmutableList.of(9))), 5); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); //Creating Replicas without replica Index - replicas = HddsTestUtils.getReplicas(new ContainerID(1), - CLOSED, 0, replicaDns); + List replicas = HddsTestUtils + .getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 3, ImmutableMap.of(racks.get(0), 2, racks.get(3), 1)); - //Creating Replicas without replica Index for replicas < number of racks - replicaDns = - Stream.of(0, 1, 3, 4) + } + + @Test + public void + testReplicasToFixMisreplicationWithReplicationFactorLessThanNumberOfRack() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( + nodeManager, conf, + GenericTestUtils.getReverseMap( + ImmutableMap.of(0, ImmutableList.of(0, 1, 5), + 1, ImmutableList.of(6), + 2, ImmutableList.of(2, 7), + 3, ImmutableList.of(3, 4, 8), + 4, ImmutableList.of(9))), 5); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 3, 4) .map(list::get).collect(Collectors.toList()); - replicas = HddsTestUtils.getReplicas(new ContainerID(1), - CLOSED, 0, replicaDns); + //Creating Replicas without replica Index for replicas < number of racks + List replicas = HddsTestUtils + .getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, ImmutableMap.of(racks.get(0), 1, racks.get(3), 1)); + } - //Creating Replicas without replica Index for replicas > number of racks - replicaDns = - Stream.of(0, 1, 2, 3, 4, 6) + @Test + public void + testReplicasToFixMisreplicationWithReplicationFactorMoreThanNumberOfRack() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( + nodeManager, conf, + GenericTestUtils.getReverseMap( + ImmutableMap.of(0, ImmutableList.of(0, 1, 2, 5), + 1, ImmutableList.of(6), + 2, ImmutableList.of(7), + 3, ImmutableList.of(3, 4, 8), + 4, ImmutableList.of(9))), 5); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 2, 3, 4, 6) .map(list::get).collect(Collectors.toList()); - - replicas = HddsTestUtils.getReplicas(new ContainerID(1), - CLOSED, 0, replicaDns); - + //Creating Replicas without replica Index for replicas >number of racks + List replicas = HddsTestUtils + .getReplicas(new ContainerID(1), CLOSED, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, ImmutableMap.of(racks.get(0), 1, racks.get(3), 1)); - dummyPlacementPolicy = + } + + @Test + public void testReplicasToFixMisreplicationMaxReplicaPerRack() { + DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 2); - racks = dummyPlacementPolicy.racks; - replicaDns = Stream.of(0, 2, 4, 6, 8) - .map(list::get).collect(Collectors.toList()); - replicas = HddsTestUtils.getReplicas(new ContainerID(1), - CLOSED, 0, replicaDns); + List racks = dummyPlacementPolicy.racks; + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 2, 4, 6, 8) + .map(list::get).collect(Collectors.toList()); + List replicas = + HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), + CLOSED, 0, 0, 0, replicaDns); testReplicasToFixMisreplication(replicas, dummyPlacementPolicy, 2, ImmutableMap.of(racks.get(0), 2)); - } @Test public void testReplicasWithoutMisreplication() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy(nodeManager, conf, 5); - List list = - nodeManager.getNodes(NodeStatus.inServiceHealthy()); - List replicaDns = - Stream.of(0, 1, 2, 3, 4) + List list = nodeManager.getAllNodes(); + List replicaDns = Stream.of(0, 1, 2, 3, 4) .map(list::get).collect(Collectors.toList()); - - List replicas = HddsTestUtils.getReplicasWithReplicaIndex(new ContainerID(1), CLOSED, 0, 0, 0, replicaDns); @@ -191,23 +264,29 @@ private static class DummyPlacementPolicy extends SCMCommonPlacementPolicy { private List racks; private int rackCnt; - DummyPlacementPolicy( - NodeManager nodeManager, - ConfigurationSource conf, + DummyPlacementPolicy(NodeManager nodeManager, ConfigurationSource conf, int rackCnt) { + this(nodeManager, conf, + IntStream.range(0, nodeManager.getAllNodes().size()).boxed() + .collect(Collectors.toMap(Function.identity(), + idx -> idx % rackCnt)), rackCnt); + } + + DummyPlacementPolicy(NodeManager nodeManager, ConfigurationSource conf, + Map datanodeRackMap, int rackCnt) { super(nodeManager, conf); - rackMap = new HashMap<>(); + this.rackCnt = rackCnt; + this.racks = IntStream.range(0, rackCnt) + .mapToObj(i -> Mockito.mock(Node.class)).collect(Collectors.toList()); List datanodeDetails = nodeManager.getAllNodes(); - this.rackCnt = Math.min(rackCnt, datanodeDetails.size()); - this.racks = new ArrayList<>(this.rackCnt); - for (int r = 0; r < this.rackCnt; r++) { - racks.add(Mockito.mock(Node.class)); - } - for (int idx = 0; idx < datanodeDetails.size(); idx++) { - rackMap.put(datanodeDetails.get(idx), racks.get(idx % this.rackCnt)); - } + rackMap = datanodeRackMap.entrySet().stream() + .collect(Collectors.toMap( + entry -> datanodeDetails.get(entry.getKey()), + entry -> racks.get(entry.getValue()))); } + + @Override public DatanodeDetails chooseNode(List healthyNodes) { return healthyNodes.get(0); diff --git a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java index e03f0a7ffe2e..daa8624708ce 100644 --- a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java @@ -25,10 +25,13 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.io.UnsupportedEncodingException; +import java.util.List; +import java.util.Map; import java.util.concurrent.TimeoutException; import com.google.common.base.Preconditions; import com.google.common.base.Supplier; +import javafx.util.Pair; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; @@ -43,6 +46,7 @@ import org.mockito.Mockito; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertTrue; @@ -283,6 +287,12 @@ public static T getFieldReflection(Object object, String fieldName) return value; } + public static Map getReverseMap(Map> map) { + return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() + .map(v ->new Pair<>(v, entry.getKey()))) + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + } + /** * Class to capture logs for doing assertions. */ From 26244fa05f4663d294d3fec1f68420c59ba6a607 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 00:02:16 -0700 Subject: [PATCH 18/23] HDDS-7492. Fix Algorithm to remove max number of replicas --- .../hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 7 +------ .../hdds/scm/TestSCMCommonPlacementPolicy.java | 14 +++++--------- .../org/apache/ozone/test/GenericTestUtils.java | 2 +- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 60a112175fee..37f3a4f897f3 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -38,13 +38,9 @@ import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; -import java.util.PriorityQueue; -import java.util.Queue; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; @@ -481,8 +477,7 @@ public Set replicasToCopyToFixMisreplication( totalNumberOfReplicas, requiredNumberOfPlacementGroups); int numberOfReplicasToBeCopied = Math.max(0, replicaList.size() - maxReplicasPerPlacementGroup); - totalNumberOfReplicas -= Math.min(replicaList.size(), - maxReplicasPerPlacementGroup); + totalNumberOfReplicas -= maxReplicasPerPlacementGroup; requiredNumberOfPlacementGroups -= 1; if (numberOfReplicasToBeCopied > 0) { List replicasToBeCopied = replicaList.stream() 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 ae11364a3ba3..885e47474973 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 @@ -19,9 +19,7 @@ package org.apache.hadoop.hdds.scm; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import javafx.util.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -31,7 +29,6 @@ import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.node.NodeManager; -import org.apache.hadoop.hdds.scm.node.NodeStatus; import org.apache.hadoop.ozone.container.common.SCMTestUtils; import org.apache.ozone.test.GenericTestUtils; import org.apache.ratis.thirdparty.com.google.common.collect.ImmutableMap; @@ -40,9 +37,6 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -162,7 +156,7 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { @Test public void - testReplicasToFixMisreplicationWithThreeMisreplicationOnDifferentRack() { + testReplicasToFixMisreplicationWithThreeMisreplicationOnDifferentRack() { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( nodeManager, conf, GenericTestUtils.getReverseMap( @@ -184,7 +178,8 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { @Test public void - testReplicasToFixMisreplicationWithReplicationFactorLessThanNumberOfRack() { + testReplicasToFixMisreplicationWithReplicationFactorLessThanNumberOfRack( + ) { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( nodeManager, conf, GenericTestUtils.getReverseMap( @@ -206,7 +201,8 @@ public void testReplicasToFixMisreplicationWithThreeMisreplication() { @Test public void - testReplicasToFixMisreplicationWithReplicationFactorMoreThanNumberOfRack() { + testReplicasToFixMisreplicationWithReplicationFactorMoreThanNumberOfRack( + ) { DummyPlacementPolicy dummyPlacementPolicy = new DummyPlacementPolicy( nodeManager, conf, GenericTestUtils.getReverseMap( diff --git a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java index daa8624708ce..b226cf685c1b 100644 --- a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java @@ -289,7 +289,7 @@ public static T getFieldReflection(Object object, String fieldName) public static Map getReverseMap(Map> map) { return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() - .map(v ->new Pair<>(v, entry.getKey()))) + .map(v -> new Pair<>(v, entry.getKey()))) .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); } From 20620e7ee6692d67103acd9332f59d4a31f38fde Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 08:22:35 -0700 Subject: [PATCH 19/23] HDDS-7492. Fix Pair Import Issue --- .../src/main/java/org/apache/ozone/test/GenericTestUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java index b226cf685c1b..771f5137a4ff 100644 --- a/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java +++ b/hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java @@ -31,10 +31,10 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; -import javafx.util.Pair; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.log4j.Appender; import org.apache.log4j.Layout; import org.apache.log4j.Level; @@ -289,7 +289,7 @@ public static T getFieldReflection(Object object, String fieldName) public static Map getReverseMap(Map> map) { return map.entrySet().stream().flatMap(entry -> entry.getValue().stream() - .map(v -> new Pair<>(v, entry.getKey()))) + .map(v -> Pair.of(v, entry.getKey()))) .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); } From f62044d6555cccd7fcb2d83fecf9d6dfa07815c0 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 10:45:23 -0700 Subject: [PATCH 20/23] HDDS-7492. Fix number of racks required while validating container placement --- .../hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index 37f3a4f897f3..e28c161c06d8 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -384,7 +384,12 @@ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); - int requiredRacks = getRequiredRackCount(replicas); + // We have a network topology so calculate if it is satisfied or not. + final int maxLevel = topology.getMaxLevel(); + // The leaf nodes are all at max level, so the number of nodes at + // leafLevel - 1 is the rack count + int numRacks = topology.getNumOfNodes(maxLevel - 1); + int requiredRacks = Math.min(getRequiredRackCount(replicas), numRacks); int maxReplicasPerRack = getMaxReplicasPerRack(replicas, requiredRacks); if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { @@ -394,12 +399,6 @@ public ContainerPlacementStatus validateContainerPlacement( return invalidPlacement; } } - // We have a network topology so calculate if it is satisfied or not. - int numRacks = 1; - final int maxLevel = topology.getMaxLevel(); - // The leaf nodes are all at max level, so the number of nodes at - // leafLevel - 1 is the rack count - numRacks = topology.getNumOfNodes(maxLevel - 1); Map currentRackCount = dns.stream() .collect(Collectors.groupingBy(this::getPlacementGroup, Collectors.counting())); From 75963bdbf569cfbd0f66d2ea00d7181dfb53d6fc Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 18:30:30 -0700 Subject: [PATCH 21/23] HDDS-7492. Fix Rack Scatter Policy to support max replicas per rack --- .../hdds/scm/SCMCommonPlacementPolicy.java | 14 ++-- .../ContainerPlacementStatusDefault.java | 2 +- .../SCMContainerPlacementRackAware.java | 5 ++ .../SCMContainerPlacementRackScatter.java | 77 +++++++++++-------- .../TestSCMContainerPlacementRackScatter.java | 56 ++++++++++---- 5 files changed, 100 insertions(+), 54 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java index e28c161c06d8..535b03508d34 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java @@ -385,12 +385,7 @@ public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. - final int maxLevel = topology.getMaxLevel(); - // The leaf nodes are all at max level, so the number of nodes at - // leafLevel - 1 is the rack count - int numRacks = topology.getNumOfNodes(maxLevel - 1); - int requiredRacks = Math.min(getRequiredRackCount(replicas), numRacks); - int maxReplicasPerRack = getMaxReplicasPerRack(replicas, requiredRacks); + int requiredRacks = getRequiredRackCount(replicas); if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. @@ -402,10 +397,15 @@ public ContainerPlacementStatus validateContainerPlacement( Map currentRackCount = dns.stream() .collect(Collectors.groupingBy(this::getPlacementGroup, Collectors.counting())); - + final int maxLevel = topology.getMaxLevel(); + // The leaf nodes are all at max level, so the number of nodes at + // leafLevel - 1 is the rack count + int numRacks = topology.getNumOfNodes(maxLevel - 1); if (replicas < requiredRacks) { requiredRacks = replicas; } + int maxReplicasPerRack = getMaxReplicasPerRack(replicas, + Math.min(requiredRacks, numRacks)); return new ContainerPlacementStatusDefault( currentRackCount.size(), requiredRacks, numRacks, maxReplicasPerRack, currentRackCount.values().stream().map(Long::intValue) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java index 159c9f5af834..a0fb5db92dab 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java @@ -65,7 +65,7 @@ public String misReplicatedReason() { if (isPolicySatisfied()) { return null; } - if (currentRacks < Math.min(requiredRacks, maxReplicasPerRack)) { + if (currentRacks < Math.min(requiredRacks, totalRacks)) { return "The container is mis-replicated as it is on " + currentRacks + " racks but should be on " + requiredRacks + " racks."; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java index 4ee7408fbb5f..2a00c4f207e8 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java @@ -376,6 +376,11 @@ private List chooseNodes(List excludedNodes, } } + @Override + protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + return Math.max(numReplicas - 1, 1); + } + @Override protected int getRequiredRackCount(int numReplicas) { return REQUIRED_RACKS; diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java index eff2dc86c426..34b2f72df311 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm.container.placement.algorithms; import com.google.common.annotations.VisibleForTesting; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.ContainerPlacementStatus; @@ -35,7 +36,10 @@ import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -78,13 +82,19 @@ public SCMContainerPlacementRackScatter(final NodeManager nodeManager, } public Set chooseNodesFromRacks(List racks, - List unavailableNodes, - List mutableFavoredNodes, - int nodesRequired, final long metadataSizeRequired, - final long dataSizeRequired, int maxOuterLoopIterations) { + List unavailableNodes, + List mutableFavoredNodes, + int nodesRequired, final Pair metadatasizeDatasizePair, + int maxOuterLoopIterations, final Pair, Integer> + rackCntMapMaxReplicaPerRackPair) { if (nodesRequired <= 0) { return Collections.emptySet(); } + final long metadataSizeRequired = metadatasizeDatasizePair.getKey(); + final long dataSizeRequired = metadatasizeDatasizePair.getValue(); + final Map rackCntMap = + rackCntMapMaxReplicaPerRackPair.getKey(); + final int maxReplicasPerRack = rackCntMapMaxReplicaPerRackPair.getValue(); List toChooseRacks = new LinkedList<>(); Set chosenNodes = new LinkedHashSet<>(); Set skippedRacks = new HashSet<>(); @@ -97,8 +107,10 @@ public Set chooseNodesFromRacks(List racks, int chosenListSize = chosenNodes.size(); // Refill toChooseRacks, we put skippedRacks in front of toChooseRacks - // for a even distribution - toChooseRacks.addAll(racks); + // for an even distribution + toChooseRacks.addAll(racks.stream() + .filter(rack -> rackCntMap.getOrDefault(rack, 0) + < maxReplicasPerRack).collect(Collectors.toList())); if (!skippedRacks.isEmpty()) { toChooseRacks.removeAll(skippedRacks); toChooseRacks.addAll(0, skippedRacks); @@ -111,6 +123,7 @@ public Set chooseNodesFromRacks(List racks, Node curRack = getRackOfDatanodeDetails(favoredNode); if (toChooseRacks.contains(curRack)) { chosenNodes.add(favoredNode); + rackCntMap.merge(curRack, 1, Math::addExact); toChooseRacks.remove(curRack); chosenFavoredNodesInForLoop.add(favoredNode); unavailableNodes.add(favoredNode); @@ -137,6 +150,7 @@ public Set chooseNodesFromRacks(List racks, metadataSizeRequired, dataSizeRequired); if (node != null) { chosenNodes.add((DatanodeDetails) node); + rackCntMap.merge(rack, 1, Math::addExact); mutableFavoredNodes.remove(node); unavailableNodes.add(node); nodesRequired--; @@ -208,7 +222,6 @@ protected List chooseDatanodesInternal( " ExcludedNode = " + excludedNodesCount, SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE); } - List mutableFavoredNodes = new ArrayList<>(); if (favoredNodes != null) { // Generate mutableFavoredNodes, only stores valid favoredNodes @@ -228,14 +241,15 @@ protected List chooseDatanodesInternal( usedNodes = Collections.emptyList(); } List racks = getAllRacks(); - Set usedRacks = usedNodes.stream() + Map usedRacksCntMap = usedNodes.stream() .map(node -> networkTopology.getAncestor(node, RACK_LEVEL)) .filter(node -> node != null) - .collect(Collectors.toSet()); + .collect(Collectors.toMap(Function.identity(), e -> 1, + Math::addExact)); int requiredReplicationFactor = usedNodes.size() + nodesRequired; - int numberOfRacksRequired = - getRequiredRackCount(requiredReplicationFactor); - int additionalRacksRequired = numberOfRacksRequired - usedRacks.size(); + int numberOfRacksRequired = getRequiredRackCount(requiredReplicationFactor); + int additionalRacksRequired = + numberOfRacksRequired - usedRacksCntMap.size(); if (nodesRequired < additionalRacksRequired) { String reason = "Required nodes size: " + nodesRequired + " is less than required number of racks to choose: " @@ -243,12 +257,15 @@ protected List chooseDatanodesInternal( LOG.warn("Placement policy cannot choose the enough racks. {}" + "Total number of Required Racks: {} Used Racks Count:" + " {}, Required Nodes count: {}", - reason, numberOfRacksRequired, usedRacks.size(), nodesRequired); + reason, numberOfRacksRequired, usedRacksCntMap.size(), + nodesRequired); throw new SCMException(reason, SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE); } + int maxReplicasPerRack = getMaxReplicasPerRack(requiredReplicationFactor, + numberOfRacksRequired); // For excluded nodes, we sort their racks at rear - racks = sortRackWithExcludedNodes(racks, excludedNodes, usedRacks); + racks = sortRackWithExcludedNodes(racks, excludedNodes, usedRacksCntMap); List unavailableNodes = new ArrayList<>(); if (excludedNodes != null) { @@ -265,14 +282,16 @@ protected List chooseDatanodesInternal( LOG.warn("Placement policy cannot choose the enough racks. {}" + "Total number of Required Racks: {} Used Racks Count:" + " {}, Required Nodes count: {}", - reason, numberOfRacksRequired, usedRacks.size(), nodesRequired); + reason, numberOfRacksRequired, usedRacksCntMap.size(), + nodesRequired); throw new SCMException(reason, SCMException.ResultCodes.FAILED_TO_FIND_SUITABLE_NODE); } chosenNodes.addAll(chooseNodesFromRacks(racks, unavailableNodes, mutableFavoredNodes, additionalRacksRequired, - metadataSizeRequired, dataSizeRequired, 1)); + Pair.of(metadataSizeRequired, dataSizeRequired), 1, + usedRacksCntMap, maxReplicasPerRack)); if (chosenNodes.size() < additionalRacksRequired) { String reason = "Chosen nodes size from Unique Racks: " + chosenNodes @@ -288,16 +307,13 @@ protected List chooseDatanodesInternal( } if (chosenNodes.size() < nodesRequired) { - racks.addAll(usedRacks); - usedRacks.addAll(chosenNodes.stream() - .map(node -> networkTopology.getAncestor(node, RACK_LEVEL)) - .filter(node -> node != null) - .collect(Collectors.toSet())); - sortRackWithExcludedNodes(racks, excludedNodes, usedRacks); - racks.addAll(usedRacks); + racks.addAll(usedRacksCntMap.keySet()); + racks = sortRackWithExcludedNodes(racks, excludedNodes, usedRacksCntMap); + racks.addAll(usedRacksCntMap.keySet()); chosenNodes.addAll(chooseNodesFromRacks(racks, unavailableNodes, mutableFavoredNodes, nodesRequired - chosenNodes.size(), - metadataSizeRequired, dataSizeRequired, Integer.MAX_VALUE)); + Pair.of(metadataSizeRequired, dataSizeRequired), + Integer.MAX_VALUE, usedRacksCntMap, maxReplicasPerRack)); } List result = new ArrayList<>(chosenNodes); @@ -319,9 +335,8 @@ protected List chooseDatanodesInternal( .flatMap(List::stream).collect(Collectors.toList()), requiredReplicationFactor); if (!placementStatus.isPolicySatisfied()) { - String errorMsg = "ContainerPlacementPolicy not met, currentRacks is " + - placementStatus.actualPlacementCount() + " desired racks is " + - placementStatus.expectedPlacementCount(); + String errorMsg = "ContainerPlacementPolicy not met. Misreplication" + + " Reason: " + placementStatus.misReplicatedReason(); throw new SCMException(errorMsg, null); } return result; @@ -416,7 +431,7 @@ private Node getRackOfDatanodeDetails(DatanodeDetails datanodeDetails) { * @return */ private List sortRackWithExcludedNodes(List racks, - List excludedNodes, Set usedRacks) { + List excludedNodes, Map usedRacks) { if ((excludedNodes == null || excludedNodes.isEmpty()) && usedRacks.isEmpty()) { return racks; @@ -425,12 +440,12 @@ private List sortRackWithExcludedNodes(List racks, .map(node -> networkTopology.getAncestor(node, RACK_LEVEL)) // Dead Nodes have been removed from the topology and so have a // null rack. We need to exclude those from the rack list. - .filter(node -> node != null) - .filter(node -> !usedRacks.contains(node)) + .filter(Objects::nonNull) + .filter(node -> !usedRacks.containsKey(node)) .collect(Collectors.toSet()); List result = new ArrayList<>(); for (Node rack : racks) { - if (!usedRacks.contains(rack) && !lessPreferredRacks.contains(rack)) { + if (!usedRacks.containsKey(rack) && !lessPreferredRacks.contains(rack)) { result.add(rack); } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java index 46bf8031effc..b49037e58376 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java @@ -58,6 +58,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONED; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.HEALTHY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_DATANODE_RATIS_VOLUME_FREE_SPACE_MIN; +import static org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES; import static org.apache.hadoop.hdds.scm.net.NetConstants.LEAF_SCHEMA; import static org.apache.hadoop.hdds.scm.net.NetConstants.RACK_SCHEMA; import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT_SCHEMA; @@ -245,20 +246,34 @@ public void chooseNodeWithNoExcludedNodes(int datanodeCount) nodeNum = 5; if (datanodeCount > nodeNum) { assumeTrue(datanodeCount >= NODE_PER_RACK); - datanodeDetails = policy.chooseDatanodes(null, null, nodeNum, 0, 15); - Assertions.assertEquals(nodeNum, datanodeDetails.size()); - Assertions.assertEquals(getRackSize(datanodeDetails), - Math.min(nodeNum, rackNum)); + if (datanodeCount == 6) { + int finalNodeNum = nodeNum; + SCMException e = assertThrows(SCMException.class, + () -> policy.chooseDatanodes(null, null, finalNodeNum, 0, 15)); + assertEquals(FAILED_TO_FIND_HEALTHY_NODES, e.getResult()); + } else { + datanodeDetails = policy.chooseDatanodes(null, null, nodeNum, 0, 15); + Assertions.assertEquals(nodeNum, datanodeDetails.size()); + Assertions.assertEquals(getRackSize(datanodeDetails), + Math.min(nodeNum, rackNum)); + } } // 10 replicas nodeNum = 10; if (datanodeCount > nodeNum) { assumeTrue(datanodeCount > 2 * NODE_PER_RACK); - datanodeDetails = policy.chooseDatanodes(null, null, nodeNum, 0, 15); - Assertions.assertEquals(nodeNum, datanodeDetails.size()); - Assertions.assertEquals(getRackSize(datanodeDetails), - Math.min(nodeNum, rackNum)); + if (datanodeCount == 11) { + int finalNodeNum = nodeNum; + SCMException e = assertThrows(SCMException.class, + () -> policy.chooseDatanodes(null, null, finalNodeNum, 0, 15)); + assertEquals(FAILED_TO_FIND_HEALTHY_NODES, e.getResult()); + } else { + datanodeDetails = policy.chooseDatanodes(null, null, nodeNum, 0, 15); + Assertions.assertEquals(nodeNum, datanodeDetails.size()); + Assertions.assertEquals(getRackSize(datanodeDetails), + Math.min(nodeNum, rackNum)); + } } } @@ -314,11 +329,20 @@ public void chooseNodeWithExcludedNodes(int datanodeCount) totalNum = 5; excludedNodes.clear(); excludedNodes.add(datanodes.get(0)); - datanodeDetails = policy.chooseDatanodes( - excludedNodes, null, nodeNum, 0, 15); - Assertions.assertEquals(nodeNum, datanodeDetails.size()); - Assertions.assertEquals(getRackSize(datanodeDetails, excludedNodes), - Math.min(totalNum, rackNum)); + if (datanodeCount == 6) { + int finalNodeNum = nodeNum; + SCMException e = assertThrows(SCMException.class, + () -> policy.chooseDatanodes(excludedNodes, null, + finalNodeNum, 0, 15)); + assertEquals(FAILED_TO_FIND_HEALTHY_NODES, e.getResult()); + } else { + datanodeDetails = policy.chooseDatanodes( + excludedNodes, null, nodeNum, 0, 15); + Assertions.assertEquals(nodeNum, datanodeDetails.size()); + Assertions.assertEquals(getRackSize(datanodeDetails, excludedNodes), + Math.min(totalNum, rackNum)); + } + // 5 replicas, two existing datanodes on different rack nodeNum = 3; @@ -344,7 +368,9 @@ public void chooseNodeWithExcludedNodes(int datanodeCount) SCMException e = assertThrows(SCMException.class, () -> policy.chooseDatanodes(excludedNodes, null, 3, 0, 15)); String message = e.getMessage(); - assumeTrue(message.contains("ContainerPlacementPolicy not met")); + assertTrue(message.contains("Chosen nodes size from Unique Racks: 1," + + " but required nodes to choose from Unique Racks: " + + "2 do not match.")); } else { datanodeDetails = policy.chooseDatanodes( excludedNodes, null, nodeNum, 0, 15); @@ -567,7 +593,7 @@ public void testInValidChooseNodesWithUsedNodesWithInsufficientRacks() { assertEquals("Chosen nodes size from Unique Racks: 1, but required " + "nodes to choose from Unique Racks: 2 do not match.", exception.getMessage()); - assertEquals(SCMException.ResultCodes.FAILED_TO_FIND_HEALTHY_NODES, + assertEquals(FAILED_TO_FIND_HEALTHY_NODES, exception.getResult()); } From 50648fada3aae85e81089133c5e0d4a195db53be Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 18:35:45 -0700 Subject: [PATCH 22/23] HDDS-7492. Fix issue in rack scatter --- .../algorithms/SCMContainerPlacementRackScatter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java index 34b2f72df311..2f86df870822 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackScatter.java @@ -291,7 +291,7 @@ protected List chooseDatanodesInternal( chosenNodes.addAll(chooseNodesFromRacks(racks, unavailableNodes, mutableFavoredNodes, additionalRacksRequired, Pair.of(metadataSizeRequired, dataSizeRequired), 1, - usedRacksCntMap, maxReplicasPerRack)); + Pair.of(usedRacksCntMap, maxReplicasPerRack))); if (chosenNodes.size() < additionalRacksRequired) { String reason = "Chosen nodes size from Unique Racks: " + chosenNodes @@ -313,7 +313,7 @@ protected List chooseDatanodesInternal( chosenNodes.addAll(chooseNodesFromRacks(racks, unavailableNodes, mutableFavoredNodes, nodesRequired - chosenNodes.size(), Pair.of(metadataSizeRequired, dataSizeRequired), - Integer.MAX_VALUE, usedRacksCntMap, maxReplicasPerRack)); + Integer.MAX_VALUE, Pair.of(usedRacksCntMap, maxReplicasPerRack))); } List result = new ArrayList<>(chosenNodes); From 7eb77cee5248792fc59285b21f0287043c2185d9 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 7 Dec 2022 20:00:23 -0700 Subject: [PATCH 23/23] HDDS-7492. Fix max replica per rack for pipeline placement policy & rack aware --- .../placement/algorithms/SCMContainerPlacementRackAware.java | 3 +++ .../hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java index 2a00c4f207e8..4f07024f16db 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/SCMContainerPlacementRackAware.java @@ -378,6 +378,9 @@ private List chooseNodes(List excludedNodes, @Override protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + if (numberOfRacks == 1) { + return numReplicas; + } return Math.max(numReplicas - 1, 1); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java index 32262b6f3bf0..2287475d8299 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java @@ -107,6 +107,9 @@ private boolean isNonClosedRatisThreePipeline(Pipeline p) { @Override protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { + if (numberOfRacks == 1) { + return numReplicas; + } return Math.max(numReplicas - 1, 1); }