[GitHub] [helix] narendly commented on a change in pull request #519: Refine the rebalance scope calculating logic in the WAGED rebalancer.

[GitBox <gi...@apache.org>]

narendly commented on a change in pull request #519: Refine the rebalance scope calculating logic in the WAGED rebalancer.
URL: https://github.com/apache/helix/pull/519#discussion_r337800360
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/ClusterModelProvider.java
 ##########
 @@ -97,27 +97,32 @@ public static ClusterModel generateClusterModel(ResourceControllerDataProvider d
    * Find the minimum set of replicas that need to be reassigned.
    * A replica needs to be reassigned if one of the following condition is true:
    * 1. Cluster topology (the cluster config / any instance config) has been updated.
-   * 2. The baseline assignment has been updated.
-   * 3. The resource config has been updated.
-   * 4. The resource idealstate has been updated. TODO remove this condition when all resource configurations are migrated to resource config.
-   * 5. If the current best possible assignment does not contain the partition's valid assignment.
+   * 2. The resource config has been updated.
+   * 3. If the current best possible assignment does not contain the partition's valid assignment.
    *
    * @param replicaMap             A map contains all the replicas grouped by resource name.
    * @param clusterChanges         A map contains all the important metadata updates that happened after the previous rebalance.
-   * @param activeInstances        All the instances that are alive and enabled.
+   * @param activeInstances        All the instances that are alive and enabled according to the delay rebalance configuration.
+   * @param liveInstances          All the instances that are alive.
    * @param bestPossibleAssignment The current best possible assignment.
    * @param allocatedReplicas      Return the allocated replicas grouped by the target instance name.
    * @return The replicas that need to be reassigned.
    */
   private static Set<AssignableReplica> findToBeAssignedReplicas(
       Map<String, Set<AssignableReplica>> replicaMap,
       Map<HelixConstants.ChangeType, Set<String>> clusterChanges, Set<String> activeInstances,
-      Map<String, ResourceAssignment> bestPossibleAssignment,
+      Set<String> liveInstances, Map<String, ResourceAssignment> bestPossibleAssignment,
       Map<String, Set<AssignableReplica>> allocatedReplicas) {
     Set<AssignableReplica> toBeAssignedReplicas = new HashSet<>();
+
+    // newly connected nodes = changed liveInstance nodes & currently active instances.
+    Set<String> newlyConnectedNodes = clusterChanges
+        .getOrDefault(HelixConstants.ChangeType.LIVE_INSTANCE, Collections.emptySet());
+    newlyConnectedNodes.retainAll(liveInstances);
     if (clusterChanges.containsKey(HelixConstants.ChangeType.CLUSTER_CONFIG) || clusterChanges
-        .containsKey(HelixConstants.ChangeType.INSTANCE_CONFIG)) {
-      // If the cluster topology has been modified, need to reassign all replicas
+        .containsKey(HelixConstants.ChangeType.INSTANCE_CONFIG) || !newlyConnectedNodes.isEmpty()) {
+      // 1. If the cluster topology has been modified, need to reassign all replicas.
+      // 2. If any node was re-connect, need to reassign all replicas for balance.
 
 Review comment:
   reconnected.
   "for balance"? What do you mean exactly? Sorry this is confusing. For global baseline calculation or balance as in evenness?
   
   Follow-up question: this is for recovery from a temporary assignment state (due to delayed rebalance) to the original assignment, correct?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org