Skip to content

HDDS-9683. Containers present on decommission nodes are reported as mis-replicated #5726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adoroszlai merged 5 commits into from
Jan 2, 2024
Merged

HDDS-9683. Containers present on decommission nodes are reported as mis-replicated #5726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b563fd3
Containers belonging to decommissioning or decommissioned nodes, are …
xBis7 Dec 4, 2023
6b441a8
Tests MethodSource cleanup
xBis7 Dec 12, 2023
a224ee7
Merge remote-tracking branch 'origin/master' into HDDS-9683
xBis7 Dec 19, 2023
5dfd1ac
adjust max number of replicas per rack
xBis7 Dec 19, 2023
8c0f8ba
checkstyle
xBis7 Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions ...op-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/SCMCommonPlacementPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,16 @@ public ContainerPlacementStatus validateContainerPlacement(
}
int maxReplicasPerRack = getMaxReplicasPerRack(replicas,
Math.min(requiredRacks, numRacks));

// There are scenarios where there could be excessive replicas on a rack due to nodes
// in decommission or maintenance or over-replication in general.
// In these cases, ReplicationManager shouldn't report mis-replication.
// The original intention of mis-replication was to indicate that there isn't enough rack tolerance.
// In case of over-replication, this isn't an issue.
// Mis-replication should be an issue once over-replication is fixed, not before.
// Adjust the maximum number of replicas per rack to allow containers
// with excessive replicas to not be reported as mis-replicated.
maxReplicasPerRack += Math.max(0, dns.size() - replicas);
return new ContainerPlacementStatusDefault(
currentRackCount.size(), requiredRacks, numRacks, maxReplicasPerRack,
currentRackCount);
Expand Down
47 changes: 47 additions & 0 deletions ...he/hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackAware.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.hadoop.hdds.conf.StorageUnit;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
import org.apache.hadoop.hdds.scm.ContainerPlacementStatus;
Expand All @@ -52,6 +53,7 @@
import org.apache.commons.lang3.StringUtils;

import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONED;
import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE;
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.net.NetConstants.LEAF_SCHEMA;
Expand All @@ -60,6 +62,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
Expand Down Expand Up @@ -563,6 +566,50 @@ public void testvalidateContainerPlacementSingleRackCluster() {
assertEquals(0, stat.misReplicationCount());
}

@ParameterizedTest
@MethodSource("org.apache.hadoop.hdds.scm.node.NodeStatus#outOfServiceStates")
public void testOverReplicationAndOutOfServiceNodes(HddsProtos.NodeOperationalState state) {
setup(7);
// 7 datanodes, all nodes are used.
// /rack0/node0 -> IN_SERVICE
// /rack0/node1 -> IN_SERVICE
// /rack0/node2 -> OFFLINE
// /rack0/node3 -> OFFLINE
// /rack0/node4 -> OFFLINE
// /rack1/node5 -> IN_SERVICE
// /rack1/node6 -> OFFLINE
datanodes.get(2).setPersistedOpState(state);
datanodes.get(3).setPersistedOpState(state);
datanodes.get(4).setPersistedOpState(state);
datanodes.get(6).setPersistedOpState(state);
List<DatanodeDetails> dns = new ArrayList<>(datanodes);

ContainerPlacementStatus status = policy.validateContainerPlacement(dns, 3);
assertTrue(status.isPolicySatisfied());
assertEquals(2, status.actualPlacementCount());
assertEquals(2, status.expectedPlacementCount());
assertEquals(0, status.misReplicationCount());
assertNull(status.misReplicatedReason());

// /rack0/node0 -> IN_SERVICE
// /rack0/node1 -> IN_SERVICE
// /rack0/node2 -> OFFLINE > IN_SERVICE
// /rack0/node3 -> OFFLINE
// /rack0/node4 -> OFFLINE
// /rack1/node5 -> IN_SERVICE
// /rack1/node6 -> OFFLINE > IN_SERVICE
datanodes.get(2).setPersistedOpState(IN_SERVICE);
datanodes.get(6).setPersistedOpState(IN_SERVICE);
dns = new ArrayList<>(datanodes);

status = policy.validateContainerPlacement(dns, 3);
assertTrue(status.isPolicySatisfied());
assertEquals(2, status.actualPlacementCount());
assertEquals(2, status.expectedPlacementCount());
assertEquals(0, status.misReplicationCount());
assertNull(status.misReplicatedReason());
}

@ParameterizedTest
@MethodSource("numDatanodes")
public void testOutOfServiceNodesNotSelected(int datanodeCount) {
Expand Down
68 changes: 68 additions & 0 deletions .../hadoop/hdds/scm/container/placement/algorithms/TestSCMContainerPlacementRackScatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.hadoop.hdds.conf.StorageUnit;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.StorageReportProto;
import org.apache.hadoop.hdds.scm.ContainerPlacementStatus;
Expand Down Expand Up @@ -68,6 +69,7 @@
import static org.hamcrest.Matchers.matchesPattern;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
Expand Down Expand Up @@ -569,6 +571,72 @@ public void testValidateContainerPlacement(int datanodeCount) {
assertEquals(0, stat.misReplicationCount());
}

@ParameterizedTest
@MethodSource("org.apache.hadoop.hdds.scm.node.NodeStatus#outOfServiceStates")
public void testOverReplicationAndOutOfServiceNodes(HddsProtos.NodeOperationalState state) {
setup(9, 3);
// 9 datanodes, 3 per rack.
// /rack0/node0 -> IN_SERVICE - used
// /rack0/node1
// /rack0/node2
// /rack1/node3 -> IN_SERVICE - used
// /rack1/node4
// /rack1/node5
// /rack2/node6 -> IN_SERVICE - used
// /rack2/node7
// /rack2/node8
List<DatanodeDetails> dns = new ArrayList<>();
dns.add(datanodes.get(0));
dns.add(datanodes.get(3));
dns.add(datanodes.get(6));

ContainerPlacementStatus status = policy.validateContainerPlacement(dns, 3);
assertTrue(status.isPolicySatisfied());
assertEquals(3, status.actualPlacementCount());
assertEquals(3, status.expectedPlacementCount());
assertEquals(0, status.misReplicationCount());
assertNull(status.misReplicatedReason());

// /rack0/node0 -> IN_SERVICE - used
// /rack0/node1 -> OFFLINE - used
// /rack0/node2
// /rack1/node3 -> IN_SERVICE - used
// /rack1/node4 -> OFFLINE - used
// /rack1/node5
// /rack2/node6 -> IN_SERVICE - used
// /rack2/node7
// /rack2/node8
datanodes.get(1).setPersistedOpState(state);
datanodes.get(4).setPersistedOpState(state);
dns.add(datanodes.get(1));
dns.add(datanodes.get(4));

status = policy.validateContainerPlacement(dns, 3);
assertTrue(status.isPolicySatisfied());
assertEquals(3, status.actualPlacementCount());
assertEquals(3, status.expectedPlacementCount());
assertEquals(0, status.misReplicationCount());
assertNull(status.misReplicatedReason());

// /rack0/node0 -> IN_SERVICE - used
// /rack0/node1 -> OFFLINE - used
// /rack0/node2 -> IN_SERVICE - used
// /rack1/node3 -> IN_SERVICE - used
// /rack1/node4 -> OFFLINE - used
// /rack1/node5
// /rack2/node6 -> IN_SERVICE - used
// /rack2/node7
// /rack2/node8
dns.add(datanodes.get(2));

status = policy.validateContainerPlacement(dns, 3);
assertTrue(status.isPolicySatisfied());
assertEquals(3, status.actualPlacementCount());
assertEquals(3, status.expectedPlacementCount());
assertEquals(0, status.misReplicationCount());
assertNull(status.misReplicatedReason());
}

public List<DatanodeDetails> getDatanodes(List<Integer> dnIndexes) {
return dnIndexes.stream().map(datanodes::get).collect(Collectors.toList());
}
Expand Down