siddhantsangwan commented on code in PR #4539:
URL: https://github.com/apache/ozone/pull/4539#discussion_r1162340328
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/TestContainerPlacementStatusDefault.java:
##########
@@ -35,53 +35,44 @@ 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(2, 2);
assertTrue(stat.isPolicySatisfied());
assertEquals(0, stat.misReplicationCount());
- stat = new ContainerPlacementStatusDefault(2, 2, 3);
- assertTrue(stat.isPolicySatisfied());
- assertEquals(0, stat.misReplicationCount());
-
- stat = new ContainerPlacementStatusDefault(3, 2, 3);
- 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());
}
@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());
+ assertEquals(2, 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());
Review Comment:
The original code here doesn't make much sense to me because it's saying
`currentRacks` is 1 but the last argument says 1 replica is on 1 rack and 2
replicas on another rack.
--
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]
