siddhantsangwan commented on code in PR #3956:
URL: https://github.com/apache/ozone/pull/3956#discussion_r1024353971
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java:
##########
@@ -447,4 +457,162 @@ public void testOverAndUnderReplicated() {
ReplicationManagerReport.HealthState.OVER_REPLICATED));
}
+ @Test
+ public void testMisReplicatedContainer() {
+ ContainerInfo container = createContainerInfo(repConfig);
+
+ // Placement policy is always violated
+ Mockito.when(placementPolicy.validateContainerPlacement(
+ Mockito.any(),
+ Mockito.anyInt()
+ )).thenAnswer(invocation ->
+ new ContainerPlacementStatusDefault(4, 5, 9));
+
+ Set<ContainerReplica> replicas = createReplicas(container.containerID(),
+ Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2),
+ Pair.of(IN_SERVICE, 3), Pair.of(IN_SERVICE, 4),
+ Pair.of(IN_SERVICE, 5));
+ ContainerCheckRequest request = requestBuilder
+ .setContainerReplicas(replicas)
+ .setContainerInfo(container)
+ .build();
+
+ ContainerHealthResult result = healthCheck.checkHealth(request);
+ Assert.assertEquals(HealthState.MIS_REPLICATED, result.getHealthState());
+
+ // Under-replicated takes precedence and the over-replication is ignored
+ // for now.
Review Comment:
NIT: Comment is incorrect
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestECReplicationCheckHandler.java:
##########
@@ -447,4 +457,162 @@ public void testOverAndUnderReplicated() {
ReplicationManagerReport.HealthState.OVER_REPLICATED));
}
+ @Test
+ public void testMisReplicatedContainer() {
+ ContainerInfo container = createContainerInfo(repConfig);
+
+ // Placement policy is always violated
+ Mockito.when(placementPolicy.validateContainerPlacement(
+ Mockito.any(),
+ Mockito.anyInt()
+ )).thenAnswer(invocation ->
+ new ContainerPlacementStatusDefault(4, 5, 9));
+
+ Set<ContainerReplica> replicas = createReplicas(container.containerID(),
+ Pair.of(IN_SERVICE, 1), Pair.of(IN_SERVICE, 2),
+ Pair.of(IN_SERVICE, 3), Pair.of(IN_SERVICE, 4),
+ Pair.of(IN_SERVICE, 5));
+ ContainerCheckRequest request = requestBuilder
+ .setContainerReplicas(replicas)
+ .setContainerInfo(container)
+ .build();
+
+ ContainerHealthResult result = healthCheck.checkHealth(request);
+ Assert.assertEquals(HealthState.MIS_REPLICATED, result.getHealthState());
+
+ // Under-replicated takes precedence and the over-replication is ignored
+ // for now.
+ Assert.assertTrue(healthCheck.handle(request));
+ Assert.assertEquals(0, repQueue.underReplicatedQueueSize());
+ Assert.assertEquals(0, repQueue.overReplicatedQueueSize());
+ Assert.assertEquals(1, repQueue.misReplicatedQueueSize());
+ Assert.assertEquals(0, report.getStat(
+ ReplicationManagerReport.HealthState.UNDER_REPLICATED));
+ Assert.assertEquals(0, report.getStat(
+ ReplicationManagerReport.HealthState.OVER_REPLICATED));
+ Assert.assertEquals(1, report.getStat(
+ ReplicationManagerReport.HealthState.MIS_REPLICATED));
+ }
+
+ @Test
+ public void testMisReplicatedContainerFixedByPending() {
+ ContainerInfo container = createContainerInfo(repConfig);
+
+ Mockito.when(placementPolicy.validateContainerPlacement(
+ Mockito.any(),
+ Mockito.anyInt()
+ )).thenAnswer(invocation -> {
+ List<DatanodeDetails> dns = invocation.getArgument(0);
+ // If the number of DNs is 3 or less make it be mis-replicated
Review Comment:
NIT: comment should be 5 instead of 3
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/ECReplicationCheckHandler.java:
##########
@@ -71,7 +80,7 @@ public boolean handle(ContainerCheckRequest request) {
}
// TODO - if it is unrecoverable, should we return false to other
// handlers can be tried?
Review Comment:
Unrelated to this PR but I'm addressing the TODO. If it is unrecoverable I
don't think we should return false now because the unhealthy handler will try
to handle them (we did add a check in that handler to preserve them if it finds
the container is under replicated so it _should_ be fine anyway).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
