From 8e6b9564bffa07b35ded7283223c4028a4c8e6d5 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 5 Apr 2023 12:01:36 +0200 Subject: [PATCH 01/11] HDDS-8383. Misreplication cannot be resolved with single rack --- .../org/apache/hadoop/hdds/scm/net/NetworkTopology.java | 6 ++++++ .../apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 7 ++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java index c863dc3da557..3c072cbe9c4d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java @@ -242,4 +242,10 @@ Node getNode(int leafIndex, String scope, List excludedScopes, */ List sortByDistanceCost(Node reader, List nodes, int activeLen); + + default int getRackCount() { + // The leaf nodes are all at max level, so the number of nodes at + // leafLevel - 1 is the rack count + return getNumOfNodes(getMaxLevel() - 1); + } } 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 8d02d7fc3b80..ccfd032d1d05 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 @@ -391,7 +391,8 @@ public ContainerPlacementStatus validateContainerPlacement( NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. int requiredRacks = getRequiredRackCount(replicas); - if (topology == null || replicas == 1 || requiredRacks == 1) { + int numRacks = topology != null ? topology.getRackCount() : 1; + if (numRacks == 1 || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. return validPlacement; @@ -402,10 +403,6 @@ 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; } From 251eeb750003071c67adc62c86c95f33ef619fa6 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Wed, 5 Apr 2023 12:07:48 +0200 Subject: [PATCH 02/11] Use new getRackCount method to reduce duplication --- .../algorithms/SCMContainerPlacementRackScatter.java | 3 +-- .../hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java | 5 +---- 2 files changed, 2 insertions(+), 6 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 495f97e0c36c..67f15181de27 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 @@ -438,8 +438,7 @@ protected int getRequiredRackCount(int numReplicas) { if (networkTopology == null) { return 1; } - int maxLevel = networkTopology.getMaxLevel(); - int numRacks = networkTopology.getNumOfNodes(maxLevel - 1); + int numRacks = networkTopology.getRackCount(); // Return the num of Rack if numRack less than numReplicas return Math.min(numRacks, numReplicas); } 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 c4bfd3d1cd0e..5a9fa82c195d 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 @@ -475,10 +475,7 @@ protected DatanodeDetails chooseNodeBasedOnSameRack( * @return true when all nodes are equal */ private boolean checkAllNodesAreEqual(NetworkTopology topology) { - if (topology == null) { - return true; - } - return (topology.getNumOfNodes(topology.getMaxLevel() - 1) == 1); + return topology == null || topology.getRackCount() == 1; } @Override From 1ef8ae9f556bae7fe50f19465f3d6b72d04ef6fc Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 10 Apr 2023 09:47:54 +0200 Subject: [PATCH 03/11] Fix at root SCMContainerPlacementRackAware#getRequiredRackCount instead of SCMCommonPlacementPolicy#validateContainerPlacement --- .../hdds/scm/SCMCommonPlacementPolicy.java | 12 +++++----- .../ContainerPlacementStatusDefault.java | 13 +++++------ .../SCMContainerPlacementRackAware.java | 3 ++- .../TestContainerPlacementFactory.java | 2 +- .../TestContainerPlacementStatusDefault.java | 22 +++++++++---------- .../TestECUnderReplicationHandler.java | 2 +- .../TestLegacyReplicationManager.java | 12 +++++----- .../TestRatisOverReplicationHandler.java | 6 ++--- .../replication/TestReplicationManager.java | 6 ++--- .../health/TestECReplicationCheckHandler.java | 12 +++++----- .../TestRatisReplicationCheckHandler.java | 14 ++++++------ .../recon/fsck/TestContainerHealthStatus.java | 4 ++-- .../recon/fsck/TestContainerHealthTask.java | 4 ++-- ...estContainerHealthTaskRecordGenerator.java | 12 +++++----- 14 files changed, 60 insertions(+), 64 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 ccfd032d1d05..e4eba4de5259 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 @@ -74,9 +74,9 @@ public abstract class SCMCommonPlacementPolicy implements * or the replication factor is 1 or the required racks is 1. */ private ContainerPlacementStatus validPlacement - = new ContainerPlacementStatusDefault(1, 1, 1); + = new ContainerPlacementStatusDefault(1, 1); private ContainerPlacementStatus invalidPlacement - = new ContainerPlacementStatusDefault(0, 1, 1); + = new ContainerPlacementStatusDefault(0, 1); /** * Constructor. @@ -391,8 +391,7 @@ public ContainerPlacementStatus validateContainerPlacement( NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. int requiredRacks = getRequiredRackCount(replicas); - int numRacks = topology != null ? topology.getRackCount() : 1; - if (numRacks == 1 || replicas == 1 || requiredRacks == 1) { + if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. return validPlacement; @@ -406,10 +405,9 @@ public ContainerPlacementStatus validateContainerPlacement( if (replicas < requiredRacks) { requiredRacks = replicas; } - int maxReplicasPerRack = getMaxReplicasPerRack(replicas, - Math.min(requiredRacks, numRacks)); + int maxReplicasPerRack = getMaxReplicasPerRack(replicas, requiredRacks); return new ContainerPlacementStatusDefault( - currentRackCount.size(), requiredRacks, numRacks, maxReplicasPerRack, + currentRackCount.size(), requiredRacks, maxReplicasPerRack, currentRackCount.values().stream().map(Long::intValue) .collect(Collectors.toList())); } 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 220775540d00..56534cfdc023 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 @@ -30,31 +30,28 @@ public class ContainerPlacementStatusDefault private final int requiredRacks; 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 maxReplicasPerRack, List rackReplicaCnts) { + int maxReplicasPerRack, List rackReplicaCnts) { this.requiredRacks = requiredRacks; this.currentRacks = currentRacks; - this.totalRacks = totalRacks; this.maxReplicasPerRack = maxReplicasPerRack; this.rackReplicaCnts = rackReplicaCnts; } - public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks, - int totalRacks) { - this(currentRacks, requiredRacks, totalRacks, 1, + public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks) { + this(currentRacks, requiredRacks, 1, currentRacks == 0 ? Collections.emptyList() : Collections.nCopies(currentRacks, 1)); } @Override public boolean isPolicySatisfied() { - if (currentRacks < Math.min(totalRacks, requiredRacks)) { + if (currentRacks < requiredRacks) { return false; } return rackReplicaCnts.stream().allMatch(cnt -> cnt <= maxReplicasPerRack); @@ -86,7 +83,7 @@ public int misReplicationCount() { @Override public int expectedPlacementCount() { - return Math.min(requiredRacks, totalRacks); + return requiredRacks; } @Override 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 4f07024f16db..b1861abe4ce7 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 @@ -386,6 +386,7 @@ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { @Override protected int getRequiredRackCount(int numReplicas) { - return REQUIRED_RACKS; + int racks = networkTopology != null ? networkTopology.getRackCount() : 1; + return Math.min(REQUIRED_RACKS, racks); } } 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 5d17f1389a3e..74cbdbf6e94c 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 @@ -194,7 +194,7 @@ public List chooseDatanodes( @Override public ContainerPlacementStatus validateContainerPlacement(List dns, int replicas) { - return new ContainerPlacementStatusDefault(1, 1, 1); + return new ContainerPlacementStatusDefault(1, 1); } @Override diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java index 74f083e8da47..725d189b91d0 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java @@ -35,24 +35,24 @@ public class TestContainerPlacementStatusDefault { @Test public void testPlacementSatisfiedCorrectly() { ContainerPlacementStatusDefault stat = - new ContainerPlacementStatusDefault(1, 1, 1); + new ContainerPlacementStatusDefault(1, 1); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); // Requires 2 racks, but cluster only has 1 - stat = new ContainerPlacementStatusDefault(1, 2, 1); + stat = new ContainerPlacementStatusDefault(1, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 2, 3); + stat = new ContainerPlacementStatusDefault(2, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(3, 2, 3); + stat = new ContainerPlacementStatusDefault(3, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(3, 2, 3); + stat = new ContainerPlacementStatusDefault(3, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); } @@ -60,28 +60,28 @@ public void testPlacementSatisfiedCorrectly() { @Test public void testPlacementNotSatisfied() { ContainerPlacementStatusDefault stat = - new ContainerPlacementStatusDefault(1, 2, 2); + new ContainerPlacementStatusDefault(1, 2); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); // Zero rack, but need 2 - shouldn't really happen in practice - stat = new ContainerPlacementStatusDefault(0, 2, 1); + stat = new ContainerPlacementStatusDefault(0, 2); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 3, 3); + stat = new ContainerPlacementStatusDefault(2, 3); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 4, 3, 1, Arrays.asList(1, 3)); + stat = new ContainerPlacementStatusDefault(2, 4, 1, Arrays.asList(1, 3)); assertFalse(stat.isPolicySatisfied()); assertEquals(2, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(1, 4, 3, 1, Arrays.asList(1, 2)); + stat = new ContainerPlacementStatusDefault(1, 4, 1, Arrays.asList(1, 2)); assertFalse(stat.isPolicySatisfied()); assertEquals(2, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 2, 3, 2, Arrays.asList(3, 1)); + stat = new ContainerPlacementStatusDefault(2, 2, 2, Arrays.asList(3, 1)); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java index 6ea963ab1481..cd89c8ff0a1d 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java @@ -135,7 +135,7 @@ public NodeStatus getNodeStatus(DatanodeDetails dd) { ecPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2)); } @Test diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java index fa1e474cfe36..49a9ac08111c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java @@ -220,7 +220,7 @@ public void setup() Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(2, 2, 3)); + new ContainerPlacementStatusDefault(2, 2)); clock = new TestClock(Instant.now(), ZoneId.of("UTC")); containerReplicaPendingOps = new ContainerReplicaPendingOps(conf, clock); createReplicationManager(new ReplicationManagerConfiguration()); @@ -2217,7 +2217,7 @@ public void additionalReplicaScheduledWhenMisReplicated() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer(invocation -> { - return new ContainerPlacementStatusDefault(1, 2, 3); + return new ContainerPlacementStatusDefault(1, 2); }); int currentReplicateCommandCount = datanodeCommandHandler @@ -2253,7 +2253,7 @@ public void additionalReplicaScheduledWhenMisReplicated() Mockito.anyList(), Mockito.anyInt() )).thenAnswer(invocation -> { - return new ContainerPlacementStatusDefault(1, 2, 3); + return new ContainerPlacementStatusDefault(1, 2); }); currentReplicateCommandCount = datanodeCommandHandler.getInvocationCount( @@ -2306,7 +2306,7 @@ public void overReplicatedButRemovingMakesMisReplicated() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(1, 2, 3)); + invocation -> new ContainerPlacementStatusDefault(1, 2)); int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); @@ -2362,7 +2362,7 @@ public void testOverReplicatedAndPolicySatisfied() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(2, 2, 3)); + invocation -> new ContainerPlacementStatusDefault(2, 2)); final int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); @@ -2410,7 +2410,7 @@ public void testOverReplicatedAndPolicyUnSatisfiedAndDeleted() Mockito.argThat(list -> list != null && list.size() <= 4), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(1, 2, 3)); + invocation -> new ContainerPlacementStatusDefault(1, 2)); int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java index c83c2a30d93c..35e857a3295a 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java @@ -82,7 +82,7 @@ public void setup() throws NodeNotFoundException, NotLeaderException, policy = Mockito.mock(PlacementPolicy.class); Mockito.when(policy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2)); replicationManager = Mockito.mock(ReplicationManager.class); Mockito.when(replicationManager.getNodeStatus(any(DatanodeDetails.class))) @@ -195,7 +195,7 @@ public void testOverReplicatedContainerBecomesMisReplicatedOnRemoving() // checked. Mockito.when(policy.validateContainerPlacement( Mockito.argThat(list -> list.size() <= 4), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); testProcessing(replicas, Collections.emptyList(), getOverReplicatedHealthResult(), 0); @@ -225,7 +225,7 @@ public void testOverReplicatedClosedContainerWithQuasiClosedReplica() // checked. Mockito.when(policy.validateContainerPlacement( Mockito.argThat(list -> list.size() <= 4), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); Set>> commands = testProcessing( replicas, Collections.emptyList(), getOverReplicatedHealthResult(), 2); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java index d9020216d3ed..b99a8ed81949 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java @@ -114,11 +114,11 @@ public void setup() throws IOException { containerManager = Mockito.mock(ContainerManager.class); ratisPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ratisPlacementPolicy.validateContainerPlacement(anyList(), - anyInt())).thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); + anyInt())).thenReturn(new ContainerPlacementStatusDefault(2, 2)); ecPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2)); scmContext = Mockito.mock(SCMContext.class); @@ -626,7 +626,7 @@ public void testUnderReplicationQueuePopulated() { // replicated take precedence. Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); ContainerInfo decomContainer = createContainerInfo(repConfig, 1, HddsProtos.LifeCycleState.CLOSED); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java index 850e975b22ea..209f97337e00 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java @@ -79,7 +79,7 @@ public void setup() { placementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(placementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2)); healthCheck = new ECReplicationCheckHandler(placementPolicy); repConfig = new ECReplicationConfig(3, 2); repQueue = new ReplicationQueue(); @@ -551,7 +551,7 @@ public void testMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5, 9)); + new ContainerPlacementStatusDefault(4, 5)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), @@ -587,9 +587,9 @@ public void testMisReplicatedContainerFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 5 or less make it be mis-replicated if (dns.size() <= 5) { - return new ContainerPlacementStatusDefault(4, 5, 9); + return new ContainerPlacementStatusDefault(4, 5); } else { - return new ContainerPlacementStatusDefault(5, 5, 9); + return new ContainerPlacementStatusDefault(5, 5); } }); @@ -632,7 +632,7 @@ public void testUnderAndMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5, 9)); + new ContainerPlacementStatusDefault(4, 5)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), @@ -667,7 +667,7 @@ public void testOverAndMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5, 9)); + new ContainerPlacementStatusDefault(4, 5)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java index ca227cc16473..9c72784b36be 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java @@ -78,7 +78,7 @@ public void setup() throws IOException { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(2, 2, 3)); + new ContainerPlacementStatusDefault(2, 2)); healthCheck = new RatisReplicationCheckHandler(containerPlacementPolicy); repConfig = RatisReplicationConfig.getInstance(THREE); repQueue = new ReplicationQueue(); @@ -559,7 +559,7 @@ public void testUnderReplicatedWithMisReplication() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(1, 2, 3)); + new ContainerPlacementStatusDefault(1, 2)); ContainerInfo container = createContainerInfo(repConfig); Set replicas @@ -591,9 +591,9 @@ public void testUnderReplicatedWithMisReplicationFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 3 or less make it be mis-replicated if (dns.size() <= 3) { - return new ContainerPlacementStatusDefault(1, 2, 3); + return new ContainerPlacementStatusDefault(1, 2); } else { - return new ContainerPlacementStatusDefault(2, 2, 3); + return new ContainerPlacementStatusDefault(2, 2); } }); @@ -632,7 +632,7 @@ public void testMisReplicated() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(1, 2, 3)); + new ContainerPlacementStatusDefault(1, 2)); ContainerInfo container = createContainerInfo(repConfig); Set replicas @@ -662,9 +662,9 @@ public void testMisReplicatedFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 3 or less make it be mis-replicated if (dns.size() <= 3) { - return new ContainerPlacementStatusDefault(1, 2, 3); + return new ContainerPlacementStatusDefault(1, 2); } else { - return new ContainerPlacementStatusDefault(2, 2, 3); + return new ContainerPlacementStatusDefault(2, 2); } }); diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java index 572e4d7f51ea..c0c4be24119f 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java @@ -58,7 +58,7 @@ public void setup() { when(container.getContainerID()).thenReturn((long)123456); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 1, 1)); + .thenReturn(new ContainerPlacementStatusDefault(1, 1)); } @Test @@ -156,7 +156,7 @@ public void testMisReplicated() { ContainerReplicaProto.State.CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); ContainerHealthStatus status = new ContainerHealthStatus(container, replicas, placementPolicy); assertFalse(status.isHealthy()); 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 33a2a33811b7..181c56578d89 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 @@ -366,9 +366,9 @@ public List chooseDatanodes( public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { if (misRepWhenDnPresent != null && isDnPresent(dns)) { - return new ContainerPlacementStatusDefault(1, 2, 3); + return new ContainerPlacementStatusDefault(1, 2); } else { - return new ContainerPlacementStatusDefault(1, 1, 1); + return new ContainerPlacementStatusDefault(1, 1); } } diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java index 4e86ca905672..819761fd8f3d 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java @@ -66,7 +66,7 @@ public void setup() { when(container.getContainerID()).thenReturn((long)123456); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 1, 1)); + .thenReturn(new ContainerPlacementStatusDefault(1, 1)); } @Test @@ -170,7 +170,7 @@ public void testMisReplicatedRecordRetainedAndUpdated() { generateReplicas(container, CLOSED, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 3, 5)); + .thenReturn(new ContainerPlacementStatusDefault(2, 3)); ContainerHealthStatus status = new ContainerHealthStatus(container, replicas, placementPolicy); @@ -196,7 +196,7 @@ public void testMisReplicatedRecordRetainedAndUpdated() { // Container is now placed OK - should be removed. when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(3, 3, 5)); + .thenReturn(new ContainerPlacementStatusDefault(3, 3)); status = new ContainerHealthStatus(container, replicas, placementPolicy); assertFalse(ContainerHealthTask.ContainerHealthRecords .retainOrUpdateRecord(status, rec)); @@ -235,7 +235,7 @@ public void testCorrectRecordsGenerated() { generateReplicas(container, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords @@ -263,7 +263,7 @@ public void testCorrectRecordsGenerated() { replicas.clear(); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords @@ -305,7 +305,7 @@ public void testRecordNotGeneratedIfAlreadyExists() { replicas = generateReplicas(container, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords .generateUnhealthyRecords(status, existingRec, (long)1234567); From 3e3adb211991a7564fa7a3ad91b78c64ba20dc7c Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 10 Apr 2023 12:17:20 +0200 Subject: [PATCH 04/11] Fix TestContainerPlacementStatusDefault --- .../TestContainerPlacementStatusDefault.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java index 725d189b91d0..fd247b3e4779 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java @@ -39,11 +39,6 @@ public void testPlacementSatisfiedCorrectly() { assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - // Requires 2 racks, but cluster only has 1 - stat = new ContainerPlacementStatusDefault(1, 2); - assertTrue(stat.isPolicySatisfied()); - assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); @@ -51,10 +46,6 @@ public void testPlacementSatisfiedCorrectly() { stat = new ContainerPlacementStatusDefault(3, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - - stat = new ContainerPlacementStatusDefault(3, 2); - assertTrue(stat.isPolicySatisfied()); - assertEquals(0, stat.misReplicationCount()); } @Test @@ -67,7 +58,7 @@ public void testPlacementNotSatisfied() { // Zero rack, but need 2 - shouldn't really happen in practice stat = new ContainerPlacementStatusDefault(0, 2); assertFalse(stat.isPolicySatisfied()); - assertEquals(1, stat.misReplicationCount()); + assertEquals(2, stat.misReplicationCount()); stat = new ContainerPlacementStatusDefault(2, 3); assertFalse(stat.isPolicySatisfied()); @@ -79,7 +70,7 @@ public void testPlacementNotSatisfied() { stat = new ContainerPlacementStatusDefault(1, 4, 1, Arrays.asList(1, 2)); assertFalse(stat.isPolicySatisfied()); - assertEquals(2, stat.misReplicationCount()); + assertEquals(3, stat.misReplicationCount()); stat = new ContainerPlacementStatusDefault(2, 2, 2, Arrays.asList(3, 1)); assertFalse(stat.isPolicySatisfied()); From 7a0b56a9a0020cde623c53af3a62bd7ed39dab98 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Mon, 10 Apr 2023 12:17:30 +0200 Subject: [PATCH 05/11] Apply same fix in PipelinePlacementPolicy --- .../hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 5a9fa82c195d..c1a5634dfa9c 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 @@ -480,7 +480,9 @@ private boolean checkAllNodesAreEqual(NetworkTopology topology) { @Override protected int getRequiredRackCount(int numReplicas) { - return REQUIRED_RACKS; + NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); + int racks = topology != null ? topology.getRackCount() : 1; + return Math.min(REQUIRED_RACKS, racks); } /** From 2d59517be328880364c5457ab3b35326a5c04895 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:51:57 +0200 Subject: [PATCH 06/11] Revert "Apply same fix in PipelinePlacementPolicy" This reverts commit 7a0b56a9a0020cde623c53af3a62bd7ed39dab98. --- .../hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 c1a5634dfa9c..5a9fa82c195d 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 @@ -480,9 +480,7 @@ private boolean checkAllNodesAreEqual(NetworkTopology topology) { @Override protected int getRequiredRackCount(int numReplicas) { - NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); - int racks = topology != null ? topology.getRackCount() : 1; - return Math.min(REQUIRED_RACKS, racks); + return REQUIRED_RACKS; } /** From 7e871b3292230249e7982642889c3de0f4a01758 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:51:57 +0200 Subject: [PATCH 07/11] Revert "Fix TestContainerPlacementStatusDefault" This reverts commit 3e3adb211991a7564fa7a3ad91b78c64ba20dc7c. --- .../TestContainerPlacementStatusDefault.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java index fd247b3e4779..725d189b91d0 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java @@ -39,6 +39,11 @@ public void testPlacementSatisfiedCorrectly() { assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); + // Requires 2 racks, but cluster only has 1 + stat = new ContainerPlacementStatusDefault(1, 2); + assertTrue(stat.isPolicySatisfied()); + assertEquals(0, stat.misReplicationCount()); + stat = new ContainerPlacementStatusDefault(2, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); @@ -46,6 +51,10 @@ public void testPlacementSatisfiedCorrectly() { stat = new ContainerPlacementStatusDefault(3, 2); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); + + stat = new ContainerPlacementStatusDefault(3, 2); + assertTrue(stat.isPolicySatisfied()); + assertEquals(0, stat.misReplicationCount()); } @Test @@ -58,7 +67,7 @@ public void testPlacementNotSatisfied() { // Zero rack, but need 2 - shouldn't really happen in practice stat = new ContainerPlacementStatusDefault(0, 2); assertFalse(stat.isPolicySatisfied()); - assertEquals(2, stat.misReplicationCount()); + assertEquals(1, stat.misReplicationCount()); stat = new ContainerPlacementStatusDefault(2, 3); assertFalse(stat.isPolicySatisfied()); @@ -70,7 +79,7 @@ public void testPlacementNotSatisfied() { stat = new ContainerPlacementStatusDefault(1, 4, 1, Arrays.asList(1, 2)); assertFalse(stat.isPolicySatisfied()); - assertEquals(3, stat.misReplicationCount()); + assertEquals(2, stat.misReplicationCount()); stat = new ContainerPlacementStatusDefault(2, 2, 2, Arrays.asList(3, 1)); assertFalse(stat.isPolicySatisfied()); From bfd37911c6c65970b019db3dbf68ea5d1dc5ecd9 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:51:57 +0200 Subject: [PATCH 08/11] Revert "Fix at root SCMContainerPlacementRackAware#getRequiredRackCount instead of SCMCommonPlacementPolicy#validateContainerPlacement" This reverts commit 1ef8ae9f556bae7fe50f19465f3d6b72d04ef6fc. --- .../hdds/scm/SCMCommonPlacementPolicy.java | 12 +++++----- .../ContainerPlacementStatusDefault.java | 13 ++++++----- .../SCMContainerPlacementRackAware.java | 3 +-- .../TestContainerPlacementFactory.java | 2 +- .../TestContainerPlacementStatusDefault.java | 22 +++++++++---------- .../TestECUnderReplicationHandler.java | 2 +- .../TestLegacyReplicationManager.java | 12 +++++----- .../TestRatisOverReplicationHandler.java | 6 ++--- .../replication/TestReplicationManager.java | 6 ++--- .../health/TestECReplicationCheckHandler.java | 12 +++++----- .../TestRatisReplicationCheckHandler.java | 14 ++++++------ .../recon/fsck/TestContainerHealthStatus.java | 4 ++-- .../recon/fsck/TestContainerHealthTask.java | 4 ++-- ...estContainerHealthTaskRecordGenerator.java | 12 +++++----- 14 files changed, 64 insertions(+), 60 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 e4eba4de5259..ccfd032d1d05 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 @@ -74,9 +74,9 @@ public abstract class SCMCommonPlacementPolicy implements * or the replication factor is 1 or the required racks is 1. */ private ContainerPlacementStatus validPlacement - = new ContainerPlacementStatusDefault(1, 1); + = new ContainerPlacementStatusDefault(1, 1, 1); private ContainerPlacementStatus invalidPlacement - = new ContainerPlacementStatusDefault(0, 1); + = new ContainerPlacementStatusDefault(0, 1, 1); /** * Constructor. @@ -391,7 +391,8 @@ public ContainerPlacementStatus validateContainerPlacement( NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. int requiredRacks = getRequiredRackCount(replicas); - if (topology == null || replicas == 1 || requiredRacks == 1) { + int numRacks = topology != null ? topology.getRackCount() : 1; + if (numRacks == 1 || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. return validPlacement; @@ -405,9 +406,10 @@ public ContainerPlacementStatus validateContainerPlacement( if (replicas < requiredRacks) { requiredRacks = replicas; } - int maxReplicasPerRack = getMaxReplicasPerRack(replicas, requiredRacks); + int maxReplicasPerRack = getMaxReplicasPerRack(replicas, + Math.min(requiredRacks, numRacks)); return new ContainerPlacementStatusDefault( - currentRackCount.size(), requiredRacks, maxReplicasPerRack, + currentRackCount.size(), requiredRacks, numRacks, maxReplicasPerRack, currentRackCount.values().stream().map(Long::intValue) .collect(Collectors.toList())); } 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 56534cfdc023..220775540d00 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 @@ -30,28 +30,31 @@ public class ContainerPlacementStatusDefault private final int requiredRacks; private final int currentRacks; + private final int totalRacks; private final int maxReplicasPerRack; private final List rackReplicaCnts; public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks, - int maxReplicasPerRack, List rackReplicaCnts) { + int totalRacks, int maxReplicasPerRack, List rackReplicaCnts) { this.requiredRacks = requiredRacks; this.currentRacks = currentRacks; + this.totalRacks = totalRacks; this.maxReplicasPerRack = maxReplicasPerRack; this.rackReplicaCnts = rackReplicaCnts; } - public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks) { - this(currentRacks, requiredRacks, 1, + public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks, + int totalRacks) { + this(currentRacks, requiredRacks, totalRacks, 1, currentRacks == 0 ? Collections.emptyList() : Collections.nCopies(currentRacks, 1)); } @Override public boolean isPolicySatisfied() { - if (currentRacks < requiredRacks) { + if (currentRacks < Math.min(totalRacks, requiredRacks)) { return false; } return rackReplicaCnts.stream().allMatch(cnt -> cnt <= maxReplicasPerRack); @@ -83,7 +86,7 @@ public int misReplicationCount() { @Override public int expectedPlacementCount() { - return requiredRacks; + return Math.min(requiredRacks, totalRacks); } @Override 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 b1861abe4ce7..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 @@ -386,7 +386,6 @@ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { @Override protected int getRequiredRackCount(int numReplicas) { - int racks = networkTopology != null ? networkTopology.getRackCount() : 1; - return Math.min(REQUIRED_RACKS, racks); + return REQUIRED_RACKS; } } 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 74cbdbf6e94c..5d17f1389a3e 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 @@ -194,7 +194,7 @@ public List chooseDatanodes( @Override public ContainerPlacementStatus validateContainerPlacement(List dns, int replicas) { - return new ContainerPlacementStatusDefault(1, 1); + return new ContainerPlacementStatusDefault(1, 1, 1); } @Override diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java index 725d189b91d0..74f083e8da47 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java @@ -35,24 +35,24 @@ public class TestContainerPlacementStatusDefault { @Test public void testPlacementSatisfiedCorrectly() { ContainerPlacementStatusDefault stat = - new ContainerPlacementStatusDefault(1, 1); + new ContainerPlacementStatusDefault(1, 1, 1); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); // Requires 2 racks, but cluster only has 1 - stat = new ContainerPlacementStatusDefault(1, 2); + stat = new ContainerPlacementStatusDefault(1, 2, 1); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 2); + stat = new ContainerPlacementStatusDefault(2, 2, 3); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(3, 2); + stat = new ContainerPlacementStatusDefault(3, 2, 3); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(3, 2); + stat = new ContainerPlacementStatusDefault(3, 2, 3); assertTrue(stat.isPolicySatisfied()); assertEquals(0, stat.misReplicationCount()); } @@ -60,28 +60,28 @@ public void testPlacementSatisfiedCorrectly() { @Test public void testPlacementNotSatisfied() { ContainerPlacementStatusDefault stat = - new ContainerPlacementStatusDefault(1, 2); + new ContainerPlacementStatusDefault(1, 2, 2); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); // Zero rack, but need 2 - shouldn't really happen in practice - stat = new ContainerPlacementStatusDefault(0, 2); + stat = new ContainerPlacementStatusDefault(0, 2, 1); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 3); + stat = new ContainerPlacementStatusDefault(2, 3, 3); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 4, 1, Arrays.asList(1, 3)); + stat = new ContainerPlacementStatusDefault(2, 4, 3, 1, Arrays.asList(1, 3)); assertFalse(stat.isPolicySatisfied()); assertEquals(2, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(1, 4, 1, Arrays.asList(1, 2)); + stat = new ContainerPlacementStatusDefault(1, 4, 3, 1, Arrays.asList(1, 2)); assertFalse(stat.isPolicySatisfied()); assertEquals(2, stat.misReplicationCount()); - stat = new ContainerPlacementStatusDefault(2, 2, 2, Arrays.asList(3, 1)); + stat = new ContainerPlacementStatusDefault(2, 2, 3, 2, Arrays.asList(3, 1)); assertFalse(stat.isPolicySatisfied()); assertEquals(1, stat.misReplicationCount()); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java index cd89c8ff0a1d..6ea963ab1481 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestECUnderReplicationHandler.java @@ -135,7 +135,7 @@ public NodeStatus getNodeStatus(DatanodeDetails dd) { ecPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); } @Test diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java index 49a9ac08111c..fa1e474cfe36 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java @@ -220,7 +220,7 @@ public void setup() Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(2, 2)); + new ContainerPlacementStatusDefault(2, 2, 3)); clock = new TestClock(Instant.now(), ZoneId.of("UTC")); containerReplicaPendingOps = new ContainerReplicaPendingOps(conf, clock); createReplicationManager(new ReplicationManagerConfiguration()); @@ -2217,7 +2217,7 @@ public void additionalReplicaScheduledWhenMisReplicated() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer(invocation -> { - return new ContainerPlacementStatusDefault(1, 2); + return new ContainerPlacementStatusDefault(1, 2, 3); }); int currentReplicateCommandCount = datanodeCommandHandler @@ -2253,7 +2253,7 @@ public void additionalReplicaScheduledWhenMisReplicated() Mockito.anyList(), Mockito.anyInt() )).thenAnswer(invocation -> { - return new ContainerPlacementStatusDefault(1, 2); + return new ContainerPlacementStatusDefault(1, 2, 3); }); currentReplicateCommandCount = datanodeCommandHandler.getInvocationCount( @@ -2306,7 +2306,7 @@ public void overReplicatedButRemovingMakesMisReplicated() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(1, 2)); + invocation -> new ContainerPlacementStatusDefault(1, 2, 3)); int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); @@ -2362,7 +2362,7 @@ public void testOverReplicatedAndPolicySatisfied() Mockito.argThat(list -> list.size() == 3), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(2, 2)); + invocation -> new ContainerPlacementStatusDefault(2, 2, 3)); final int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); @@ -2410,7 +2410,7 @@ public void testOverReplicatedAndPolicyUnSatisfiedAndDeleted() Mockito.argThat(list -> list != null && list.size() <= 4), Mockito.anyInt() )).thenAnswer( - invocation -> new ContainerPlacementStatusDefault(1, 2)); + invocation -> new ContainerPlacementStatusDefault(1, 2, 3)); int currentDeleteCommandCount = datanodeCommandHandler .getInvocationCount(SCMCommandProto.Type.deleteContainerCommand); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java index 35e857a3295a..c83c2a30d93c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestRatisOverReplicationHandler.java @@ -82,7 +82,7 @@ public void setup() throws NodeNotFoundException, NotLeaderException, policy = Mockito.mock(PlacementPolicy.class); Mockito.when(policy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); replicationManager = Mockito.mock(ReplicationManager.class); Mockito.when(replicationManager.getNodeStatus(any(DatanodeDetails.class))) @@ -195,7 +195,7 @@ public void testOverReplicatedContainerBecomesMisReplicatedOnRemoving() // checked. Mockito.when(policy.validateContainerPlacement( Mockito.argThat(list -> list.size() <= 4), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); testProcessing(replicas, Collections.emptyList(), getOverReplicatedHealthResult(), 0); @@ -225,7 +225,7 @@ public void testOverReplicatedClosedContainerWithQuasiClosedReplica() // checked. Mockito.when(policy.validateContainerPlacement( Mockito.argThat(list -> list.size() <= 4), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); Set>> commands = testProcessing( replicas, Collections.emptyList(), getOverReplicatedHealthResult(), 2); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java index b99a8ed81949..d9020216d3ed 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java @@ -114,11 +114,11 @@ public void setup() throws IOException { containerManager = Mockito.mock(ContainerManager.class); ratisPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ratisPlacementPolicy.validateContainerPlacement(anyList(), - anyInt())).thenReturn(new ContainerPlacementStatusDefault(2, 2)); + anyInt())).thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); ecPlacementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); scmContext = Mockito.mock(SCMContext.class); @@ -626,7 +626,7 @@ public void testUnderReplicationQueuePopulated() { // replicated take precedence. Mockito.when(ecPlacementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 3)); ContainerInfo decomContainer = createContainerInfo(repConfig, 1, HddsProtos.LifeCycleState.CLOSED); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java index 209f97337e00..850e975b22ea 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java @@ -79,7 +79,7 @@ public void setup() { placementPolicy = Mockito.mock(PlacementPolicy.class); Mockito.when(placementPolicy.validateContainerPlacement( anyList(), anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 2)); + .thenReturn(new ContainerPlacementStatusDefault(2, 2, 3)); healthCheck = new ECReplicationCheckHandler(placementPolicy); repConfig = new ECReplicationConfig(3, 2); repQueue = new ReplicationQueue(); @@ -551,7 +551,7 @@ public void testMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5)); + new ContainerPlacementStatusDefault(4, 5, 9)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), @@ -587,9 +587,9 @@ public void testMisReplicatedContainerFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 5 or less make it be mis-replicated if (dns.size() <= 5) { - return new ContainerPlacementStatusDefault(4, 5); + return new ContainerPlacementStatusDefault(4, 5, 9); } else { - return new ContainerPlacementStatusDefault(5, 5); + return new ContainerPlacementStatusDefault(5, 5, 9); } }); @@ -632,7 +632,7 @@ public void testUnderAndMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5)); + new ContainerPlacementStatusDefault(4, 5, 9)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), @@ -667,7 +667,7 @@ public void testOverAndMisReplicatedContainer() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(4, 5)); + new ContainerPlacementStatusDefault(4, 5, 9)); Set replicas = createReplicas(container.containerID(), Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2), diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java index 9c72784b36be..ca227cc16473 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestRatisReplicationCheckHandler.java @@ -78,7 +78,7 @@ public void setup() throws IOException { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(2, 2)); + new ContainerPlacementStatusDefault(2, 2, 3)); healthCheck = new RatisReplicationCheckHandler(containerPlacementPolicy); repConfig = RatisReplicationConfig.getInstance(THREE); repQueue = new ReplicationQueue(); @@ -559,7 +559,7 @@ public void testUnderReplicatedWithMisReplication() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(1, 2)); + new ContainerPlacementStatusDefault(1, 2, 3)); ContainerInfo container = createContainerInfo(repConfig); Set replicas @@ -591,9 +591,9 @@ public void testUnderReplicatedWithMisReplicationFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 3 or less make it be mis-replicated if (dns.size() <= 3) { - return new ContainerPlacementStatusDefault(1, 2); + return new ContainerPlacementStatusDefault(1, 2, 3); } else { - return new ContainerPlacementStatusDefault(2, 2); + return new ContainerPlacementStatusDefault(2, 2, 3); } }); @@ -632,7 +632,7 @@ public void testMisReplicated() { Mockito.any(), Mockito.anyInt() )).thenAnswer(invocation -> - new ContainerPlacementStatusDefault(1, 2)); + new ContainerPlacementStatusDefault(1, 2, 3)); ContainerInfo container = createContainerInfo(repConfig); Set replicas @@ -662,9 +662,9 @@ public void testMisReplicatedFixedByPending() { List dns = invocation.getArgument(0); // If the number of DNs is 3 or less make it be mis-replicated if (dns.size() <= 3) { - return new ContainerPlacementStatusDefault(1, 2); + return new ContainerPlacementStatusDefault(1, 2, 3); } else { - return new ContainerPlacementStatusDefault(2, 2); + return new ContainerPlacementStatusDefault(2, 2, 3); } }); diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java index c0c4be24119f..572e4d7f51ea 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthStatus.java @@ -58,7 +58,7 @@ public void setup() { when(container.getContainerID()).thenReturn((long)123456); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 1)); + .thenReturn(new ContainerPlacementStatusDefault(1, 1, 1)); } @Test @@ -156,7 +156,7 @@ public void testMisReplicated() { ContainerReplicaProto.State.CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); ContainerHealthStatus status = new ContainerHealthStatus(container, replicas, placementPolicy); assertFalse(status.isHealthy()); 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 181c56578d89..33a2a33811b7 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 @@ -366,9 +366,9 @@ public List chooseDatanodes( public ContainerPlacementStatus validateContainerPlacement( List dns, int replicas) { if (misRepWhenDnPresent != null && isDnPresent(dns)) { - return new ContainerPlacementStatusDefault(1, 2); + return new ContainerPlacementStatusDefault(1, 2, 3); } else { - return new ContainerPlacementStatusDefault(1, 1); + return new ContainerPlacementStatusDefault(1, 1, 1); } } diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java index 819761fd8f3d..4e86ca905672 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/fsck/TestContainerHealthTaskRecordGenerator.java @@ -66,7 +66,7 @@ public void setup() { when(container.getContainerID()).thenReturn((long)123456); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 1)); + .thenReturn(new ContainerPlacementStatusDefault(1, 1, 1)); } @Test @@ -170,7 +170,7 @@ public void testMisReplicatedRecordRetainedAndUpdated() { generateReplicas(container, CLOSED, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(2, 3)); + .thenReturn(new ContainerPlacementStatusDefault(2, 3, 5)); ContainerHealthStatus status = new ContainerHealthStatus(container, replicas, placementPolicy); @@ -196,7 +196,7 @@ public void testMisReplicatedRecordRetainedAndUpdated() { // Container is now placed OK - should be removed. when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(3, 3)); + .thenReturn(new ContainerPlacementStatusDefault(3, 3, 5)); status = new ContainerHealthStatus(container, replicas, placementPolicy); assertFalse(ContainerHealthTask.ContainerHealthRecords .retainOrUpdateRecord(status, rec)); @@ -235,7 +235,7 @@ public void testCorrectRecordsGenerated() { generateReplicas(container, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords @@ -263,7 +263,7 @@ public void testCorrectRecordsGenerated() { replicas.clear(); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords @@ -305,7 +305,7 @@ public void testRecordNotGeneratedIfAlreadyExists() { replicas = generateReplicas(container, CLOSED, CLOSED); when(placementPolicy.validateContainerPlacement( Mockito.anyList(), Mockito.anyInt())) - .thenReturn(new ContainerPlacementStatusDefault(1, 2)); + .thenReturn(new ContainerPlacementStatusDefault(1, 2, 5)); status = new ContainerHealthStatus(container, replicas, placementPolicy); records = ContainerHealthTask.ContainerHealthRecords .generateUnhealthyRecords(status, existingRec, (long)1234567); From a261e76ad13eee74d1ec4156fc8b9509e30e2493 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:51:57 +0200 Subject: [PATCH 09/11] Revert "Use new getRackCount method to reduce duplication" This reverts commit 251eeb750003071c67adc62c86c95f33ef619fa6. --- .../algorithms/SCMContainerPlacementRackScatter.java | 3 ++- .../hadoop/hdds/scm/pipeline/PipelinePlacementPolicy.java | 5 ++++- 2 files changed, 6 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 67f15181de27..495f97e0c36c 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 @@ -438,7 +438,8 @@ protected int getRequiredRackCount(int numReplicas) { if (networkTopology == null) { return 1; } - int numRacks = networkTopology.getRackCount(); + int maxLevel = networkTopology.getMaxLevel(); + int numRacks = networkTopology.getNumOfNodes(maxLevel - 1); // Return the num of Rack if numRack less than numReplicas return Math.min(numRacks, numReplicas); } 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 5a9fa82c195d..c4bfd3d1cd0e 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 @@ -475,7 +475,10 @@ protected DatanodeDetails chooseNodeBasedOnSameRack( * @return true when all nodes are equal */ private boolean checkAllNodesAreEqual(NetworkTopology topology) { - return topology == null || topology.getRackCount() == 1; + if (topology == null) { + return true; + } + return (topology.getNumOfNodes(topology.getMaxLevel() - 1) == 1); } @Override From 8731b614355ea8065d079ae26adb7491639a46d2 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:51:57 +0200 Subject: [PATCH 10/11] Revert "HDDS-8383. Misreplication cannot be resolved with single rack" This reverts commit 8e6b9564bffa07b35ded7283223c4028a4c8e6d5. --- .../org/apache/hadoop/hdds/scm/net/NetworkTopology.java | 6 ------ .../apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java | 7 +++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java index 3c072cbe9c4d..c863dc3da557 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java @@ -242,10 +242,4 @@ Node getNode(int leafIndex, String scope, List excludedScopes, */ List sortByDistanceCost(Node reader, List nodes, int activeLen); - - default int getRackCount() { - // The leaf nodes are all at max level, so the number of nodes at - // leafLevel - 1 is the rack count - return getNumOfNodes(getMaxLevel() - 1); - } } 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 ccfd032d1d05..8d02d7fc3b80 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 @@ -391,8 +391,7 @@ public ContainerPlacementStatus validateContainerPlacement( NetworkTopology topology = nodeManager.getClusterNetworkTopologyMap(); // We have a network topology so calculate if it is satisfied or not. int requiredRacks = getRequiredRackCount(replicas); - int numRacks = topology != null ? topology.getRackCount() : 1; - if (numRacks == 1 || replicas == 1 || requiredRacks == 1) { + if (topology == null || replicas == 1 || requiredRacks == 1) { if (dns.size() > 0) { // placement is always satisfied if there is at least one DN. return validPlacement; @@ -403,6 +402,10 @@ 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; } From 8ca9e4324346891aa7328b249ef2d4e93244f77c Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 11 Apr 2023 08:54:47 +0200 Subject: [PATCH 11/11] HDDS-8383. Misreplication cannot be resolved with single rack --- .../placement/algorithms/SCMContainerPlacementRackAware.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 4f07024f16db..4d880345ec4f 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 @@ -386,6 +386,9 @@ protected int getMaxReplicasPerRack(int numReplicas, int numberOfRacks) { @Override protected int getRequiredRackCount(int numReplicas) { - return REQUIRED_RACKS; + int racks = networkTopology != null + ? networkTopology.getNumOfNodes(networkTopology.getMaxLevel() - 1) + : 1; + return Math.min(REQUIRED_RACKS, racks); } }