qqu0127 commented on code in PR #2189:
URL: https://github.com/apache/helix/pull/2189#discussion_r940801761
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java:
##########
@@ -216,6 +218,7 @@ private WagedRebalancer(AssignmentMetadataStore
assignmentMetadataStore,
_changeDetector = new ResourceChangeDetector(true);
_baselineCalculateExecutor = Executors.newSingleThreadExecutor();
+ _bestPossibleCalculateExecutor = Executors.newSingleThreadExecutor();
Review Comment:
Let's make sure to close this thread pool.
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java:
##########
@@ -562,6 +590,57 @@ private Map<String, ResourceAssignment> partialRebalance(
}
_partialRebalanceLatency.endMeasuringLatency();
LOG.info("Finish calculating the new best possible assignment.");
+
+ if (isBestPossibleChanged) {
+ LOG.info("Schedule a new rebalance after the new best possible
calculation has finished.");
+ RebalanceUtil.scheduleOnDemandPipeline(clusterData.getClusterName(), 0L,
false);
+ }
+ }
+
+ private Map<String, ResourceAssignment> emergencyRebalance(
+ ResourceControllerDataProvider clusterData, Map<String, Resource>
resourceMap,
+ Set<String> activeNodes, final CurrentStateOutput currentStateOutput,
+ RebalanceAlgorithm algorithm)
+ throws HelixRebalanceException {
+ Map<String, ResourceAssignment> currentBestPossibleAssignment =
+ getBestPossibleAssignment(_assignmentMetadataStore, currentStateOutput,
+ resourceMap.keySet());
+ ClusterModel clusterModel;
+ try {
+ clusterModel = ClusterModelProvider
+ .generateClusterModelForEmergencyRebalance(clusterData, resourceMap,
activeNodes,
+ currentBestPossibleAssignment);
+ } catch (Exception ex) {
+ throw new HelixRebalanceException("Failed to generate cluster model for
emergency rebalance.",
+ HelixRebalanceException.Type.INVALID_CLUSTER_STATUS, ex);
+ }
+
+ // Only calculate if there are illegal placements, meaning the cluster
model is non null;
+ // otherwise, start partial rebalance and return the current best possible.
+ if (clusterModel == null) {
Review Comment:
Just thinking aloud, it feels weird to have a null model. Is there a better
way than using null value? Say, check whether there is illegal placements here?
If it's too much trouble, let's document this nullable in the method
signature.
##########
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java:
##########
@@ -562,6 +590,57 @@ private Map<String, ResourceAssignment> partialRebalance(
}
_partialRebalanceLatency.endMeasuringLatency();
LOG.info("Finish calculating the new best possible assignment.");
+
+ if (isBestPossibleChanged) {
+ LOG.info("Schedule a new rebalance after the new best possible
calculation has finished.");
+ RebalanceUtil.scheduleOnDemandPipeline(clusterData.getClusterName(), 0L,
false);
+ }
+ }
+
+ private Map<String, ResourceAssignment> emergencyRebalance(
+ ResourceControllerDataProvider clusterData, Map<String, Resource>
resourceMap,
+ Set<String> activeNodes, final CurrentStateOutput currentStateOutput,
+ RebalanceAlgorithm algorithm)
+ throws HelixRebalanceException {
+ Map<String, ResourceAssignment> currentBestPossibleAssignment =
+ getBestPossibleAssignment(_assignmentMetadataStore, currentStateOutput,
+ resourceMap.keySet());
+ ClusterModel clusterModel;
+ try {
+ clusterModel = ClusterModelProvider
+ .generateClusterModelForEmergencyRebalance(clusterData, resourceMap,
activeNodes,
+ currentBestPossibleAssignment);
+ } catch (Exception ex) {
+ throw new HelixRebalanceException("Failed to generate cluster model for
emergency rebalance.",
+ HelixRebalanceException.Type.INVALID_CLUSTER_STATUS, ex);
+ }
+
+ // Only calculate if there are illegal placements, meaning the cluster
model is non null;
+ // otherwise, start partial rebalance and return the current best possible.
+ if (clusterModel == null) {
+ // Perform partial rebalance for a new best possible assignment
+ partialRebalance(clusterData, resourceMap, activeNodes,
currentStateOutput, algorithm);
+ return currentBestPossibleAssignment;
+ }
+
+ _asyncPartialRebalanceResult.cancel(true);
Review Comment:
nit: some logs might be useful for cases it's interrupted VS already done?
--
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]
