NealSun96 commented on code in PR #2189:
URL: https://github.com/apache/helix/pull/2189#discussion_r1029694727
##########
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:
That's because `currentAllocations` is losing one element each time a
corresponding replica is there, if you look a few lines down. So
`currentAllocations.iterator().next()` is just the first element (whatever that
means in a set) of this set.
--
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]
