sodonnel commented on code in PR #3836:
URL: https://github.com/apache/ozone/pull/3836#discussion_r1007837096


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ECUnderReplicationHandler.java:
##########
@@ -353,8 +386,77 @@ private void processMaintenanceOnlyIndexes(
         DatanodeDetails target = iterator.next();
         commands.put(target, replicateCommand);
         additionalMaintenanceCopiesNeeded -= 1;
+        maintIndexes.remove(replica.getReplicaIndex());
+        createdIndexes.add(replica.getReplicaIndex());
+      }
+    }
+  }
+
+  /**
+   * Function processes Replicas to fix placement policy issues.
+   * @param replicas
+   * @param deletionInFlight
+   * @param container
+   * @param excludedNodes
+   * @param sources
+   * @param createdIndexes
+   * @param placementStatus
+   * @throws IOException
+   */
+  private Map<DatanodeDetails, SCMCommand<?>> processMisreplication(
+          Set<ContainerReplica> replicas,
+          List<DatanodeDetails> deletionInFlight, ContainerInfo container,
+          List<DatanodeDetails> excludedNodes,
+          Map<Integer, Pair<ContainerReplica, NodeStatus>> sources,
+          Set<Integer> createdIndexes, ContainerPlacementStatus 
placementStatus)
+          throws IOException {
+    if (placementStatus.isPolicySatisfied() ||
+            placementStatus.misReplicationCount() <= createdIndexes.size()) {
+      return Collections.emptyMap();
+    }
+    Map<DatanodeDetails, SCMCommand<?>> commands = new HashMap<>();
+    List<Integer> sortedReplicaIdx =

Review Comment:
   The placementPolicy gives us the new target, but I don't understand what 
this code is doing to select which replica index it is going to copy onto that 
new target.
   
   Say we have EC 3-2, and the current layout looks like:
   
   ```
   index: 1 Rack: 1
   Index:2 Rack: 2 **
   Index:3 Rack: 2 **
   Index:4 Rack: 4
   Index:5 Rack: 5
   ```
   When we goto the placement policy, it says "here is a new target on rack 3".
   
   That means we need to make a new copy of index 2 or 3 onto rack 3, and then 
later remove the original index 2 or 3, but that is handled later by the over 
rep handler. Copying index 1, 4 or 5 to Rack 3 does not improve the placement, 
as we are still short by a rack.
   
   How does the logic here address that, as you seem to be sorting the DNs in 
order of duplicate index count first, and then using that below to pick which 
node to make a new copy of.
   
   



-- 
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]

Reply via email to