J-HowHuang commented on code in PR #15618:
URL: https://github.com/apache/pinot/pull/15618#discussion_r2076094593
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -1352,26 +1394,58 @@ static boolean isExternalViewConverged(String
tableNameWithType,
Map<String, Map<String, String>> externalViewSegmentStates,
Map<String, Map<String, String>> idealStateSegmentStates, boolean
lowDiskMode, boolean bestEfforts,
@Nullable Set<String> segmentsToMonitor) {
- return isExternalViewConverged(tableNameWithType,
externalViewSegmentStates, idealStateSegmentStates, lowDiskMode,
- bestEfforts, segmentsToMonitor, LOGGER);
+ return getNumRemainingSegmentsToProcess(tableNameWithType,
externalViewSegmentStates, idealStateSegmentStates,
+ lowDiskMode, bestEfforts, segmentsToMonitor, LOGGER, true) == 0;
}
/**
- * NOTE:
- * Only check the segments in the IdealState and being monitored. Extra
segments in ExternalView are ignored because
- * they are not managed by the rebalancer.
- * For each segment checked:
- * - In regular mode, it is okay to have extra instances in ExternalView as
long as the instance states in IdealState
- * are reached.
- * - In low disk mode, instance states in ExternalView must match IdealState
to ensure the segments are deleted from
- * server before moving to the next assignment.
- * For ERROR state in ExternalView, if using best-efforts, log a warning and
treat it as good state; if not, throw an
- * exception to abort the rebalance because we are not able to get out of
the ERROR state.
+ * Check if the external view has converged to the ideal state. See
`getNumRemainingSegmentsToProcess` for details on
+ * how the convergence is determined.
*/
private static boolean isExternalViewConverged(String tableNameWithType,
Map<String, Map<String, String>> externalViewSegmentStates,
Map<String, Map<String, String>> idealStateSegmentStates, boolean
lowDiskMode, boolean bestEfforts,
@Nullable Set<String> segmentsToMonitor, Logger tableRebalanceLogger) {
+ return getNumRemainingSegmentsToProcess(tableNameWithType,
externalViewSegmentStates, idealStateSegmentStates,
+ lowDiskMode, bestEfforts, segmentsToMonitor, tableRebalanceLogger,
true) == 0;
+ }
+
+ @VisibleForTesting
+ static int getNumRemainingSegmentsToProcess(String tableNameWithType,
+ Map<String, Map<String, String>> externalViewSegmentStates,
+ Map<String, Map<String, String>> idealStateSegmentStates, boolean
lowDiskMode, boolean bestEfforts,
+ @Nullable Set<String> segmentsToMonitor) {
+ return getNumRemainingSegmentsToProcess(tableNameWithType,
externalViewSegmentStates, idealStateSegmentStates,
+ lowDiskMode, bestEfforts, segmentsToMonitor, LOGGER, false);
+ }
+
+ /**
+ * Count the number of segments that are not in the expected state. If
`earlyReturn=true` it returns as soon as the
+ * count becomes non-zero (effectively return 1). This is used to check
whether the ExternalView has converged to
Review Comment:
This function only returns 1 or 0 when `earlyReturn=true`, returns 1 if
`remainingSegmentsToProcess > 0`, returns 0 otherwise. number of segments
processed "so far" can be confusing imo, am I misunderstanding you?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
