symious commented on a change in pull request #3129:
URL: https://github.com/apache/ozone/pull/3129#discussion_r813486442
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -417,24 +425,32 @@ private IterationResult doIteration() {
findSourceStrategy.removeCandidateSourceDataNode(source);
}
}
-
- if (!isMoveGenerated) {
- //no move option is generated, so the cluster can not be
- //balanced any more, just stop iteration and exit
- return IterationResult.CAN_NOT_BALANCE_ANY_MORE;
- }
- return IterationResult.ITERATION_COMPLETED;
} finally {
- checkIterationMoveResults(selectedTargets);
Review comment:
I think finally block better fits the idea of cleaning code, the new
version seems to have too much process logic in it. Personally, I do like the
original method.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -375,26 +378,31 @@ private IterationResult doIteration() {
findSourceStrategy.reInitialize(getPotentialSources(), config, lowerLimit);
List<DatanodeUsageInfo> potentialTargets = getPotentialTargets();
findTargetStrategy.reInitialize(potentialTargets, config, upperLimit);
-
Set<DatanodeDetails> selectedTargets =
new HashSet<>(potentialTargets.size());
moveSelectionToFutureMap = new HashMap<>(unBalancedNodes.size());
boolean isMoveGenerated = false;
+ iterationResult = IterationResult.ITERATION_COMPLETED;
+
try {
// match each overUtilized node with a target
while (true) {
if (!isBalancerRunning()) {
- return IterationResult.ITERATION_INTERRUPTED;
+ iterationResult = IterationResult.ITERATION_INTERRUPTED;
+ break;
}
- IterationResult result = checkConditionsForBalancing();
- if (result != null) {
- return result;
+ if (checkConditionsForBalancing() != null) {
Review comment:
I think the `maxSizeToMovePerIteration` and
`maxDatanodesPercentageToInvolvePerIteration` are more like
ITERATION_STOP_REASON other than ITERATION_RESULT.
Maybe we can add a new field noting the reason and mark the result as
COMPLETE if encounter a valid STOP_REASON?
--
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]
