qqu0127 commented on code in PR #2189:
URL: https://github.com/apache/helix/pull/2189#discussion_r1028533565


##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/ClusterModelProvider.java:
##########
@@ -391,6 +418,46 @@ private static Set<AssignableReplica> 
findToBeAssignedReplicasByClusterChanges(
     return toBeAssignedReplicas;
   }
 
+  /**
+   * Find replicas that were assigned to non-active nodes in the current 
assignment.
+   *
+   * @param replicaMap             A map contains all the replicas grouped by 
resource name.
+   * @param activeInstances        All the instances that are live and enabled 
according to the delay rebalance configuration.
+   * @param currentAssignment      The current assignment that was generated 
in the previous rebalance.
+   * @param allocatedReplicas      A map of <Instance -> replicas> to return 
the allocated replicas grouped by the target instance name.
+   * @return The replicas that need to be reassigned.
+   */
+  private static Set<AssignableReplica> 
findToBeAssignedReplicasOnDownInstances(
+      Map<String, Set<AssignableReplica>> replicaMap, Set<String> 
activeInstances,
+      Map<String, ResourceAssignment> currentAssignment,
+      Map<String, Set<AssignableReplica>> allocatedReplicas) {
+    // For any replica that are assigned to non-active instances (down 
instances), add them.
+    Set<AssignableReplica> toBeAssignedReplicas = new HashSet<>();
+    for (String resourceName : replicaMap.keySet()) {
+      Map<String, Map<String, Set<String>>> stateInstanceMap = 
getStateInstanceMap(currentAssignment.get(resourceName));
+
+      for (AssignableReplica replica : replicaMap.get(resourceName)) {
+        String partitionName = replica.getPartitionName();
+        String replicaState = replica.getReplicaState();
+        Set<String> currentAllocations =
+            stateInstanceMap.getOrDefault(partitionName, 
Collections.emptyMap())
+                .getOrDefault(replicaState, Collections.emptySet());
+        if (!currentAllocations.isEmpty()) {
+          String allocatedInstance = currentAllocations.iterator().next();
+          if (activeInstances.contains(allocatedInstance)) {
+            allocatedReplicas.computeIfAbsent(allocatedInstance, key -> new 
HashSet<>()).add(replica);

Review Comment:
   then I'm confused, the code is using `currentAllocations.iterator().next();` 
each time, from your description it should be reusing the same iterator.



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