(pinot) branch master updated: Fix rebalancer converge check to ensure EV is converged before reporting success (#12182)

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new fade75bf4e Fix rebalancer converge check to ensure EV is converged before reporting success (#12182)
fade75bf4e is described below

commit fade75bf4e114a824ed6c1559ee103e51e1de9f7
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Tue Dec 19 16:32:42 2023 -0800

    Fix rebalancer converge check to ensure EV is converged before reporting success (#12182)
---
 .../core/assignment/segment/SegmentAssignmentUtils.java      |  8 ++++----
 .../controller/helix/core/rebalance/TableRebalancer.java     | 12 ++++++++----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
index 228a4fb264..52f736a555 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
@@ -292,16 +292,16 @@ public class SegmentAssignmentUtils {
     return numSegmentsToBeMovedPerInstance;
   }
 
-  public static Set<String> getSegmentsToMove(Map<String, Map<String, String>> oldAssignment,
+  public static List<String> getSegmentsToMove(Map<String, Map<String, String>> oldAssignment,
       Map<String, Map<String, String>> newAssignment) {
-    Set<String> result = new HashSet<>();
+    List<String> segmentsToMove = new ArrayList<>();
     for (Map.Entry<String, Map<String, String>> entry : newAssignment.entrySet()) {
       String segmentName = entry.getKey();
       if (!entry.getValue().equals(oldAssignment.get(segmentName))) {
-        result.add(segmentName);
+        segmentsToMove.add(segmentName);
       }
     }
-    return result;
+    return segmentsToMove;
   }
 
   /**
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java
index 318d251bfc..5c3514fa7e 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java
@@ -23,6 +23,7 @@ import com.google.common.base.Preconditions;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -336,7 +337,7 @@ public class TableRebalancer {
     //    current instances as this is the best we can do, and can help the table get out of this state.
     // 2. Only check the segments to be moved because we don't need to maintain available replicas for segments not
     //    being moved, including segments with all replicas OFFLINE (error segments during consumption).
-    Set<String> segmentsToMove = SegmentAssignmentUtils.getSegmentsToMove(currentAssignment, targetAssignment);
+    List<String> segmentsToMove = SegmentAssignmentUtils.getSegmentsToMove(currentAssignment, targetAssignment);
 
     int numReplicas = Integer.MAX_VALUE;
     for (String segment : segmentsToMove) {
@@ -379,12 +380,15 @@ public class TableRebalancer {
     // 5. Update the IdealState to the next assignment. If the IdealState changes before the update, go back to step 1.
     while (true) {
       // Wait for ExternalView to converge before updating the next IdealState
+      // NOTE: Monitor the segments to be moved from both the previous round and this round to ensure the moved segments
+      //       in the previous round are also converged.
+      Set<String> segmentsToMonitor = new HashSet<>(segmentsToMove);
       segmentsToMove = SegmentAssignmentUtils.getSegmentsToMove(currentAssignment, targetAssignment);
+      segmentsToMonitor.addAll(segmentsToMove);
       IdealState idealState;
       try {
-        idealState =
-            waitForExternalViewToConverge(tableNameWithType, bestEfforts, segmentsToMove, externalViewCheckIntervalInMs,
-                externalViewStabilizationTimeoutInMs);
+        idealState = waitForExternalViewToConverge(tableNameWithType, bestEfforts, segmentsToMonitor,
+            externalViewCheckIntervalInMs, externalViewStabilizationTimeoutInMs);
       } catch (Exception e) {
         String errorMsg = String.format(
             "For rebalanceId: %s, caught exception while waiting for ExternalView to converge for table: %s, "


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org