swamirishi commented on code in PR #4099:
URL: https://github.com/apache/ozone/pull/4099#discussion_r1051093340
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerHealthResult.java:
##########
@@ -207,19 +215,28 @@ public boolean isUnrecoverable() {
* containers are not spread across enough racks.
*/
public static class MisReplicatedHealthResult
- extends ContainerHealthResult {
+ extends UnderReplicatedHealthResult {
- private final boolean replicatedOkAfterPending;
+ /**
+ * In UnderReplicatedHealthState, DECOMMISSION_REDUNDANCY is defined as
+ * 5 so that containers which are really under replicated get fixed as a
+ * priority over decommissioning hosts. We have defined that a container
+ * can only be mis replicated if it is not over or under replicated. Fixing
+ * mis replication is arguably less important than competing a
decommission.
+ * So as a lot of mis replicated container do not block decommission, we
+ * set the redundancy of mis replicated containers to 6 so they sort after
+ * under / over replicated and decommissioning replicas in the under
+ * replication queue.
+ */
+ private static final int MIS_REP_REDUNDANCY = 6;
Review Comment:
Instead of constant value, can we have some priority based on the
misreplication count
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -525,8 +528,18 @@ public Map<DatanodeDetails, SCMCommand<?>>
processUnderReplicatedContainer(
List<ContainerReplicaOp> pendingOps =
containerReplicaPendingOps.getPendingOps(containerID);
if (result.getContainerInfo().getReplicationType() == EC) {
- return ecUnderReplicationHandler.processAndCreateCommands(replicas,
- pendingOps, result, maintenanceRedundancy);
+ if (result.getHealthState()
Review Comment:
nit: probably making it into a switch case would be a good.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java:
##########
@@ -525,8 +528,18 @@ public Map<DatanodeDetails, SCMCommand<?>>
processUnderReplicatedContainer(
List<ContainerReplicaOp> pendingOps =
containerReplicaPendingOps.getPendingOps(containerID);
if (result.getContainerInfo().getReplicationType() == EC) {
- return ecUnderReplicationHandler.processAndCreateCommands(replicas,
- pendingOps, result, maintenanceRedundancy);
+ if (result.getHealthState()
Review Comment:
Can we create a separate method to processMisreplicatedContainer & have this
if case in UnderReplicatedProcessor class?
--
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]
