You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "junkaixue (via GitHub)" <gi...@apache.org> on 2023/08/15 00:13:03 UTC
[GitHub] [helix] junkaixue commented on a diff in pull request #2595: Implement the on-demand rebalance service
junkaixue commented on code in PR #2595:
URL: https://github.com/apache/helix/pull/2595#discussion_r1294069994
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java:
##########
@@ -417,6 +432,18 @@ private static int getMinActiveReplica(ResourceControllerDataProvider clusterDat
currentIdealState), currentIdealState, numReplica);
}
+ private static boolean isInstanceForcedToBeRebalancded(Long offlineOrDisabledTime, long delay,
Review Comment:
NIT: add some description here. For example, what's the behavior of lastOndemand means here.
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/util/DelayedRebalanceUtil.java:
##########
@@ -417,6 +432,18 @@ private static int getMinActiveReplica(ResourceControllerDataProvider clusterDat
currentIdealState), currentIdealState, numReplica);
}
+ private static boolean isInstanceForcedToBeRebalancded(Long offlineOrDisabledTime, long delay,
+ long lastOnDemandRebalanceTimestamp) {
+ if (lastOnDemandRebalanceTimestamp == -1 || offlineOrDisabledTime == null
+ || offlineOrDisabledTime <= 0) {
+ return false;
+ }
+ long delayedInactiveTime = offlineOrDisabledTime + delay;
+ return offlineOrDisabledTime < lastOnDemandRebalanceTimestamp
+ && lastOnDemandRebalanceTimestamp < Math.min(delayedInactiveTime,
Review Comment:
discussed offline. this piece may not need.
##########
helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java:
##########
@@ -1173,4 +1177,22 @@ public String getInstanceHelixDisabledTimeStamp(String instanceName) {
}
return getDisabledInstances().get(instanceName);
}
+
+ /**
+ * @return: a unix time that represents the last time the on demand rebalance is triggered on
+ * current cluster. Return -1 if the configuration doesn't have such record yet.
Review Comment:
Usually, param/return is under the description
--
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: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org