junkaixue commented on code in PR #2595:
URL: https://github.com/apache/helix/pull/2595#discussion_r1296452406
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java:
##########
@@ -417,6 +432,39 @@ private static int
getMinActiveReplica(ResourceControllerDataProvider clusterDat
currentIdealState), currentIdealState, numReplica);
}
+ /**
+ * Given the offline/disabled time, delay, and the last on-demand rebalance
time, this method checks
+ * if the node associated with the offline/disabled time is forced to be
rebalanced by the on-demand
+ * rebalance. There are several cases:
+ * 1. If either the last on-demand rebalance time or the offline/disabled
time is unavailable, then
+ * the node is not forced to be rebalanced.
+ * 2. If the current time surpasses the delayed offline/disabled time, then
the node is not forced
+ * to be rebalanced.
+ * 3. If the last on-demand rebalance time is before the offline/disabled
time, the node is not
+ * forced to be rebalanced.
+ * 4. If the last on-demand rebalance time is after the delayed
offline/disabled time, then the
+ * node is not forced to be rebalanced.
+ * 5. Otherwise, the offline/disabled node should be forced to rebalance.
+ *
+ * @param offlineOrDisabledTime A unix timestamp indicating the most recent
time when a node went
+ * offline or was disabled.
+ * @param delay The delay window configuration of the current cluster
+ * @param lastOnDemandRebalanceTime A unix timestamp representing the most
recent time when an
+ * on-demand rebalance was triggered.
+ * @return a boolean indicating whether a node is forced to be rebalanced
+ */
+ private static boolean isInstanceForcedToBeRebalanced(Long
offlineOrDisabledTime, long delay,
+ long lastOnDemandRebalanceTime) {
+ if (lastOnDemandRebalanceTime == -1 || offlineOrDisabledTime == null
+ || offlineOrDisabledTime <= 0 || System.currentTimeMillis() >
(offlineOrDisabledTime
+ + delay)) {
+ return false;
+ }
+
+ return offlineOrDisabledTime < lastOnDemandRebalanceTime &&
lastOnDemandRebalanceTime < (
Review Comment:
still:
lastOnDemandRebalanceTime < (offlineOrDisabledTime + delay)
As we discussed, even lastOnDemandRebalanceTime > (offlineOrDisabledTime +
delay). It is OK. It means the time already passed. We don't need check this.
--
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]
