jiajunwang commented on a change in pull request #1359:
URL: https://github.com/apache/helix/pull/1359#discussion_r486724878
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java
##########
@@ -347,36 +347,39 @@ private boolean
computeSingleResourceBestPossibleState(ClusterEvent event,
LogUtil.logError(logger, _eventId, "Error computing assignment for
resource " + resourceName
+ ". no rebalancer found. rebalancer: " + rebalancer + "
mappingCalculator: "
+ mappingCalculator);
- }
-
- if (rebalancer != null && mappingCalculator != null) {
- ResourceAssignment partitionStateAssignment = null;
+ } else {
try {
HelixManager manager =
event.getAttribute(AttributeName.helixmanager.name());
rebalancer.init(manager);
idealState =
rebalancer.computeNewIdealState(resourceName, idealState,
currentStateOutput, cache);
- output.setPreferenceLists(resourceName,
idealState.getPreferenceLists());
+ // Check if calculation is done successfully
+ if (!checkBestPossibleStateCalculation(idealState)) {
+ LogUtil.logWarn(logger, _eventId,
+ "The calculated idealState is not valid, resource: " +
resourceName);
+ return false;
+ }
// Use the internal MappingCalculator interface to compute the final
assignment
// The next release will support rebalancers that compute the mapping
from start to finish
- partitionStateAssignment = mappingCalculator
+ ResourceAssignment partitionStateAssignment = mappingCalculator
.computeBestPossiblePartitionState(cache, idealState, resource,
currentStateOutput);
if (partitionStateAssignment == null) {
LogUtil.logWarn(logger, _eventId,
- "PartitionStateAssignment is null, resource: " + resourceName);
+ "The calculated partitionStateAssignment is null, resource: " +
resourceName);
return false;
}
+ // Set the calculated result to the output.
+ output.setPreferenceLists(resourceName,
idealState.getPreferenceLists());
Review comment:
Yeah, this is a behavior change. But the original behavior is the
problem here. If any value is set in the output, then the later stages will
just use it without any doubt. This is very dangerous.
I checked later stages like intermediate state calc, and message generating,
they can handle a null output gracefully. But they cannot handle the output
with the wrong information gracefully.
----------------------------------------------------------------
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]
