siddhantsangwan commented on code in PR #4006:
URL: https://github.com/apache/ozone/pull/4006#discussion_r1045811966
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/placement/algorithms/ContainerPlacementStatusDefault.java:
##########
@@ -29,33 +32,56 @@
private final int currentRacks;
private final int totalRacks;
+ private final int maxReplicasPerRack;
+ private final List<Integer> rackReplicaCnts;
+
+
public ContainerPlacementStatusDefault(int currentRacks, int requiredRacks,
- int totalRacks) {
+ int totalRacks, int maxReplicasPerRack, List<Integer> 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
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, totalRacks)) {
+ 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
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());
Review Comment:
Trying to figure out what this part is doing. Should it be `cnt -
maxReplicasPerRack` instead? @swamirishi @sodonnel
--
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]
