You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/27 06:15:41 UTC
[GitHub] [ozone] siddhantsangwan opened a new pull request, #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
siddhantsangwan opened a new pull request, #3895:
URL: https://github.com/apache/ozone/pull/3895
## What changes were proposed in this pull request?
Currently, we stop an iteration of balancer if we are one datanode or 5GB away from the limits "datanodes.involved.max.percentage.per.iteration" and "size.moved.max.per.iteration" respectively. This is a pre-emptive check.
This PR changes this by trying to dynamically adapt to these limits if possible instead of breaking out of the loop right away. If we're nearing the limits - that is, we're one DN away - then we restrict potential source datanodes to the ones that have already been selected. If we've reached the limits, then we restrict potential sources and targets both to datanodes that have already been selected. The iteration can continue because we aren't involving any new datanodes (Changes made in `ContainerBalancerTask`, `FindSourceStrategy` and `FindTargetStrategy`). We adapt to the max size to move limit by filtering out candidate containers according to size (changes made in `ContainerBalancerSelectionCriteria`).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5526
## How was this patch tested?
Existing tests
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1024179113
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,36 +678,68 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
/**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
+ * Restricts potential target datanodes to nodes that have
+ * already been selected if balancer is one datanode away from
+ * "datanodes.involved.max.percentage.per.iteration" limit.
+ * @return true if potential targets were restricted, else false
*/
- private boolean checkIterationLimits() {
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we're nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
+ /* We're one datanode away from reaching the limit. Restrict potential
+ targets to targets that have already been selected.
+ */
+ findTargetStrategy.resetPotentialTargets(selectedTargets);
Review Comment:
if we reach here, i think we can restrict any of potential targets or sources, as long as we make sure we will involve at most one more datanode.
for example , when all the selected targets can not be a candidate target again(they may be overutilized after receiving a container replica) , we can restrict potential sources to sources that have already been selected, so that we may select a new target which will not be overutilized after receiving a container replica, meanwhile we involve only one more datanode in this balance iteration.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1024932724
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,36 +678,68 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
/**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
+ * Restricts potential target datanodes to nodes that have
+ * already been selected if balancer is one datanode away from
+ * "datanodes.involved.max.percentage.per.iteration" limit.
+ * @return true if potential targets were restricted, else false
*/
- private boolean checkIterationLimits() {
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we're nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
+ /* We're one datanode away from reaching the limit. Restrict potential
+ targets to targets that have already been selected.
+ */
+ findTargetStrategy.resetPotentialTargets(selectedTargets);
Review Comment:
Looking into it, I don't see a straightforward way to do this. @JacksonYao287 Since this PR has been open for quite a while, do you think we should create a different JIRA to investigate restricting sources when targets are unavailable?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on PR #3895:
URL: https://github.com/apache/ozone/pull/3895#issuecomment-1303269404
@JacksonYao287 Thanks for reviewing. I agree that one datanode would not make much difference in large clusters; this bug is more apparent in small clusters.
This change is more about correctness. While the previous code is simpler logic, it's not entirely correct. Suppose 9 out of 10 datanodes have been involved in balancing. Now, users can expect this iteration to continue if some datanodes are still under and over utilized. But because of the current logic, this iteration will be stopped.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sumitagrawl commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1006601010
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,37 +680,69 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
- /**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
- */
- private boolean checkIterationLimits() {
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we've reached max size to move limit
+ if (reachedMaxSizeToMovePerIteration()) {
+ // return false because we cannot adapt if we've reached max size to
+ // move limit
+ return false;
+ }
+
+ // check if we've nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
- return true;
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
+ /* We're one datanode away from reaching the limit. Restrict potential
+ targets to targets that have already been selected.
+ */
+ findTargetStrategy.resetPotentialTargets(selectedTargets);
+ LOG.debug("Approaching max datanodes to involve limit. {} datanodes " +
+ "have already been selected for balancing and the limit is " +
+ "{}. Only already selected targets can be selected as targets" +
+ " now.",
+ countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
}
- if (sizeScheduledForMoveInLatestIteration +
- (long) ozoneConfiguration.getStorageSize(
- ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
- ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT,
- StorageUnit.BYTES) > maxSizeToMovePerIteration) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max size to move limit. {} bytes have already been " +
- "scheduled for balancing and the limit is {} bytes.",
- sizeScheduledForMoveInLatestIteration,
- maxSizeToMovePerIteration);
- }
- return true;
+
+ // we either didn't need to or were able to adapt, so return true
+ return true;
+ }
+
+ private boolean adaptOnReachingIterationLimits() {
+ // check if we've reached max size to move limit
+ if (reachedMaxSizeToMovePerIteration()) {
+ // return false because we cannot adapt if we've reached max size to
+ // move limit
+ return false;
}
- return false;
+
+ // check if we've reached max datanodes to involve limit
+ int maxDatanodesToInvolve =
+ (int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
+ if (countDatanodesInvolvedPerIteration == maxDatanodesToInvolve) {
Review Comment:
check should be >= as every iteration, as countDatanodesInvolvedPerIteration can be increased by 0-2 in range as per selection of source and target.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,37 +680,69 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
- /**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
- */
- private boolean checkIterationLimits() {
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we've reached max size to move limit
+ if (reachedMaxSizeToMovePerIteration()) {
+ // return false because we cannot adapt if we've reached max size to
+ // move limit
+ return false;
+ }
+
+ // check if we've nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
- return true;
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
Review Comment:
the check should involve " + 2 > " as range of increase can be 0-2 for source and target, eg, if max is 2, countDatanodeInvolved is also 2, it will fail
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on PR #3895:
URL: https://github.com/apache/ozone/pull/3895#issuecomment-1322907084
thanks @siddhantsangwan for the work and @ChenSammi @sumitagrawl for the review, merged
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3895:
URL: https://github.com/apache/ozone/pull/3895#issuecomment-1322055918
The patch LGTM too. + 1.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1008091260
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,37 +680,69 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
- /**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
- */
- private boolean checkIterationLimits() {
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we've reached max size to move limit
+ if (reachedMaxSizeToMovePerIteration()) {
+ // return false because we cannot adapt if we've reached max size to
+ // move limit
+ return false;
+ }
+
+ // check if we've nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
- return true;
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
Review Comment:
Yes, the range is 0-2.
- If we're 2 datanodes away from the limit, we don't need to take any action.
- This check is intended for the case when we're 1 datanode away from the limit. In this case we can allow only one new datanode at max to be selected. So, we restrict potential sources - that means only a new target can be selected now.
- In your example where max is 2 and count is 2 (we've reached the limit), yes this check will fail but the check in `adaptOnReachingIterationLimits` will pass. It will restrict both sources and targets, so no new datanodes will be selected.
Does this make sense or have I misunderstood your comment?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sumitagrawl commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1019014857
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -467,10 +465,13 @@ private IterationResult doIteration() {
findSourceStrategy.reInitialize(getPotentialSources(), config, lowerLimit);
List<DatanodeUsageInfo> potentialTargets = getPotentialTargets();
findTargetStrategy.reInitialize(potentialTargets, config, upperLimit);
+ findSourceStrategy.reInitialize(getPotentialSources(), config, lowerLimit);
Review Comment:
duplicate code as present 2 line earlier same statement, need keep only one
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1027069971
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,36 +678,68 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
/**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
+ * Restricts potential target datanodes to nodes that have
+ * already been selected if balancer is one datanode away from
+ * "datanodes.involved.max.percentage.per.iteration" limit.
+ * @return true if potential targets were restricted, else false
*/
- private boolean checkIterationLimits() {
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we're nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
+ /* We're one datanode away from reaching the limit. Restrict potential
+ targets to targets that have already been selected.
+ */
+ findTargetStrategy.resetPotentialTargets(selectedTargets);
Review Comment:
agree, we can create another jira to do this.
@lokeshj1703 do you have any comments?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on PR #3895:
URL: https://github.com/apache/ozone/pull/3895#issuecomment-1305145954
@sumitagrawl Pointed out a bug in the logic - let me fix that and update this PR.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on PR #3895:
URL: https://github.com/apache/ozone/pull/3895#issuecomment-1308368046
@sumitagrawl I've updated the PR, can you please check?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] siddhantsangwan commented on a diff in pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on code in PR #3895:
URL: https://github.com/apache/ozone/pull/3895#discussion_r1024824381
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -663,36 +678,68 @@ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source) {
return moveSelection;
}
+ private boolean reachedMaxSizeToMovePerIteration() {
+ // since candidate containers in ContainerBalancerSelectionCriteria are
+ // filtered out according to this limit, balancer should not have crossed it
+ if (sizeScheduledForMoveInLatestIteration >= maxSizeToMovePerIteration) {
+ LOG.warn("Reached max size to move limit. {} bytes have already been" +
+ " scheduled for balancing and the limit is {} bytes.",
+ sizeScheduledForMoveInLatestIteration, maxSizeToMovePerIteration);
+ return true;
+ }
+ return false;
+ }
+
/**
- * Checks if limits maxDatanodesPercentageToInvolvePerIteration and
- * maxSizeToMovePerIteration have been hit.
- *
- * @return true if a limit was hit, else false
+ * Restricts potential target datanodes to nodes that have
+ * already been selected if balancer is one datanode away from
+ * "datanodes.involved.max.percentage.per.iteration" limit.
+ * @return true if potential targets were restricted, else false
*/
- private boolean checkIterationLimits() {
+ private boolean adaptWhenNearingIterationLimits() {
+ // check if we're nearing max datanodes to involve
int maxDatanodesToInvolve =
(int) (maxDatanodesRatioToInvolvePerIteration * totalNodesInCluster);
- if (countDatanodesInvolvedPerIteration + 2 > maxDatanodesToInvolve) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Hit max datanodes to involve limit. {} datanodes have" +
- " already been scheduled for balancing and the limit is {}.",
- countDatanodesInvolvedPerIteration, maxDatanodesToInvolve);
- }
+ if (countDatanodesInvolvedPerIteration + 1 == maxDatanodesToInvolve) {
+ /* We're one datanode away from reaching the limit. Restrict potential
+ targets to targets that have already been selected.
+ */
+ findTargetStrategy.resetPotentialTargets(selectedTargets);
Review Comment:
Yes, that's a good point. Let me see how to do that.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] JacksonYao287 merged pull request #3895: HDDS-5526. ContainerBalancer#checkConditionsForBalancing pre-emptively checks iteration limits.
Posted by GitBox <gi...@apache.org>.
JacksonYao287 merged PR #3895:
URL: https://github.com/apache/ozone/pull/3895
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org