jiajunwang commented on a change in pull request #1167:
URL: https://github.com/apache/helix/pull/1167#discussion_r458956773
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
##########
@@ -643,23 +643,28 @@ private void validateInput(ResourceControllerDataProvider
clusterData,
private Map<String, ResourceAssignment> getBaselineAssignment(
AssignmentMetadataStore assignmentMetadataStore, CurrentStateOutput
currentStateOutput,
Set<String> resources) throws HelixRebalanceException {
- Map<String, ResourceAssignment> currentBaseline = Collections.emptyMap();
+ Map<String, ResourceAssignment> currentBaseline = new HashMap<>();
if (assignmentMetadataStore != null) {
try {
_stateReadLatency.startMeasuringLatency();
- currentBaseline = assignmentMetadataStore.getBaseline();
+ currentBaseline = new HashMap<>(assignmentMetadataStore.getBaseline());
_stateReadLatency.endMeasuringLatency();
} catch (Exception ex) {
throw new HelixRebalanceException(
"Failed to get the current baseline assignment because of
unexpected error.",
HelixRebalanceException.Type.INVALID_REBALANCER_STATUS, ex);
}
}
- if (currentBaseline.isEmpty()) {
+ currentBaseline.keySet().retainAll(resources);
+ if (!currentBaseline.keySet().containsAll(resources)) {
Review comment:
containsAll() will iterate through resources anyway. So just do the loop
without check here.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
##########
@@ -643,23 +643,28 @@ private void validateInput(ResourceControllerDataProvider
clusterData,
private Map<String, ResourceAssignment> getBaselineAssignment(
AssignmentMetadataStore assignmentMetadataStore, CurrentStateOutput
currentStateOutput,
Set<String> resources) throws HelixRebalanceException {
- Map<String, ResourceAssignment> currentBaseline = Collections.emptyMap();
+ Map<String, ResourceAssignment> currentBaseline = new HashMap<>();
if (assignmentMetadataStore != null) {
try {
_stateReadLatency.startMeasuringLatency();
- currentBaseline = assignmentMetadataStore.getBaseline();
+ currentBaseline = new HashMap<>(assignmentMetadataStore.getBaseline());
_stateReadLatency.endMeasuringLatency();
} catch (Exception ex) {
throw new HelixRebalanceException(
"Failed to get the current baseline assignment because of
unexpected error.",
HelixRebalanceException.Type.INVALID_REBALANCER_STATUS, ex);
}
}
- if (currentBaseline.isEmpty()) {
+ currentBaseline.keySet().retainAll(resources);
+ if (!currentBaseline.keySet().containsAll(resources)) {
LOG.warn("The current baseline assignment record is empty. Use the
current states instead.");
- currentBaseline = currentStateOutput.getAssignment(resources);
+ for (Map.Entry<String, ResourceAssignment> entry :
currentStateOutput.getAssignment(resources)
+ .entrySet()) {
+ if (resources.contains(entry.getKey()) &&
!currentBaseline.containsKey(entry.getKey())) {
Review comment:
currentBaseline.putIfAbsent() ?
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
##########
@@ -643,23 +643,28 @@ private void validateInput(ResourceControllerDataProvider
clusterData,
private Map<String, ResourceAssignment> getBaselineAssignment(
AssignmentMetadataStore assignmentMetadataStore, CurrentStateOutput
currentStateOutput,
Set<String> resources) throws HelixRebalanceException {
- Map<String, ResourceAssignment> currentBaseline = Collections.emptyMap();
+ Map<String, ResourceAssignment> currentBaseline = new HashMap<>();
if (assignmentMetadataStore != null) {
try {
_stateReadLatency.startMeasuringLatency();
- currentBaseline = assignmentMetadataStore.getBaseline();
+ currentBaseline = new HashMap<>(assignmentMetadataStore.getBaseline());
_stateReadLatency.endMeasuringLatency();
} catch (Exception ex) {
throw new HelixRebalanceException(
"Failed to get the current baseline assignment because of
unexpected error.",
HelixRebalanceException.Type.INVALID_REBALANCER_STATUS, ex);
}
}
- if (currentBaseline.isEmpty()) {
+ currentBaseline.keySet().retainAll(resources);
+ if (!currentBaseline.keySet().containsAll(resources)) {
LOG.warn("The current baseline assignment record is empty. Use the
current states instead.");
- currentBaseline = currentStateOutput.getAssignment(resources);
+ for (Map.Entry<String, ResourceAssignment> entry :
currentStateOutput.getAssignment(resources)
Review comment:
getAssignment() is very expensive, let's do it for the missing resources
only. If you do it for all resources, the algorithm performance would be
impacted.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
##########
@@ -643,23 +643,28 @@ private void validateInput(ResourceControllerDataProvider
clusterData,
private Map<String, ResourceAssignment> getBaselineAssignment(
AssignmentMetadataStore assignmentMetadataStore, CurrentStateOutput
currentStateOutput,
Set<String> resources) throws HelixRebalanceException {
- Map<String, ResourceAssignment> currentBaseline = Collections.emptyMap();
+ Map<String, ResourceAssignment> currentBaseline = new HashMap<>();
if (assignmentMetadataStore != null) {
try {
_stateReadLatency.startMeasuringLatency();
- currentBaseline = assignmentMetadataStore.getBaseline();
+ currentBaseline = new HashMap<>(assignmentMetadataStore.getBaseline());
_stateReadLatency.endMeasuringLatency();
} catch (Exception ex) {
throw new HelixRebalanceException(
"Failed to get the current baseline assignment because of
unexpected error.",
HelixRebalanceException.Type.INVALID_REBALANCER_STATUS, ex);
}
}
- if (currentBaseline.isEmpty()) {
+ currentBaseline.keySet().retainAll(resources);
+ if (!currentBaseline.keySet().containsAll(resources)) {
LOG.warn("The current baseline assignment record is empty. Use the
current states instead.");
Review comment:
The current baseline assignment record does not contain all resources
assignment....
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
