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 2021/07/20 06:08:31 UTC
[GitHub] [ozone] siddhantsangwan opened a new pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
siddhantsangwan opened a new pull request #2441:
URL: https://github.com/apache/ozone/pull/2441
## What changes were proposed in this pull request?
After determining over and under utilized nodes, container matches source with target and identifies containers to move. A source is either over or within threshold utilized, while a target is under utilized. The grouping of a target datanode and container to move is being called `ContainerMoveSelection`. The selection must satisfy selection criteria, such as:
1. Containers should not be undergoing replication
2. Container should not already be selected for balancing
3. etc. as mentioned in the design doc
The selection criteria currently not satisfied is: Try to move containers that are not following Placement Policy.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4929
## How was this patch tested?
Added Unit tests: `TestContainerBalancer`
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686021766
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -44,23 +54,70 @@
" of the entire cluster) no more than the threshold value.")
private String threshold = "0.1";
- @Config(key = "datanodes.balanced.max", type = ConfigType.INT,
- defaultValue = "5", tags = {ConfigTag.BALANCER}, description = "The " +
- "maximum number of datanodes that should be balanced. Container " +
- "Balancer will not balance more number of datanodes than this limit.")
- private int maxDatanodesToBalance = 5;
+ @Config(key = "datanodes.involved.max.per.iteration", type = ConfigType.AUTO,
Review comment:
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: 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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-883103755
@lokeshj1703 @JacksonYao287 @ChenSammi please take a look!
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680951355
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
Review comment:
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: 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680951696
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
Review comment:
Makes sense, changed this.
--
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 edited a comment on pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 edited a comment on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-885002741
please merge the latest master branch and try again, #2446 seems may the test case not flaky
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r676319758
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,25 +104,22 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration();
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
Review comment:
Okay!
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-884878420
@JacksonYao287 it seems `TestReplicationManager.testMovePrerequisites` is failing. I created a Jira, can you take a look? https://issues.apache.org/jira/browse/HDDS-5484
--
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] lokeshj1703 commented on a change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r676495944
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
+ LOG.error("Container Balancer is already running.");
return false;
}
-
+
+ ozoneConfiguration = new OzoneConfiguration();
this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
LOG.info("Starting Container Balancer...{}", this);
+
//we should start a new balancer thread async
//and response to cli as soon as possible
//TODO: this is a temporary implementation
//modify this later
- currentBalancingThread = new Thread(() -> balance());
+ currentBalancingThread = new Thread(this::balance);
currentBalancingThread.start();
////////////////////////
} finally {
lock.unlock();
}
-
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
- for (int i = 0; i < idleIteration; i++) {
- if (!initializeIteration()) {
+ countDatanodesBalanced = 0;
+ totalSizeMoved = 0;
+ int i = 0;
+ do {
+ this.idleIteration = config.getIdleIteration();
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
+ this.maxSizeToMove = config.getMaxSizeToMove();
+
+ if (!initializeIteration() || !doIteration()) {
//balancer should be stopped immediately
break;
}
- // unBalancedNodes is not cleared since the next iteration uses this
- // iteration's unBalancedNodes to find out how many nodes were balanced
- overUtilizedNodes.clear();
- underUtilizedNodes.clear();
- withinThresholdUtilizedNodes.clear();
- }
+ i++;
+ } while (i < idleIteration);
Review comment:
We should have a configurable sleep period between iterations. We should also check that sleep period is greater than du refresh period for datanodes.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,25 +104,22 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration();
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
Review comment:
It might make sense to have an Iteration class to store iteration related fields. ContainerBalancer should ideally store fields used across iterations.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -200,6 +228,8 @@ private boolean initializeIteration() {
// find over and under utilized nodes
for (DatanodeUsageInfo datanodeUsageInfo : datanodeUsageInfos) {
double utilization = calculateUtilization(datanodeUsageInfo);
+ LOG.info("Utilization for node {} is {}",
+ datanodeUsageInfo.getDatanodeDetails().getUuidString(), utilization);
Review comment:
This can be a debug log. Similarly there are other statements which can be made into debug log.
Further it can be enclosed under `LOG.isDebugEnabled` check.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -141,18 +200,231 @@ public void unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced() {
*/
@Test
public void containerBalancerShouldStopWhenMaxDatanodesToBalanceIsReached() {
- balancerConfiguration.setMaxDatanodesToBalance(2);
+ balancerConfiguration.setMaxDatanodesToBalance(4);
+ balancerConfiguration.setThreshold(0.01);
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ Assert.assertFalse(containerBalancer.isBalancerRunning());
+ containerBalancer.stop();
+ }
+
+ @Test
+ public void containerBalancerShouldSelectOnlyClosedContainers() {
+ // make all containers open, balancer should not select any of them
+ for (ContainerInfo containerInfo : cidToInfoMap.values()) {
+ containerInfo.setState(HddsProtos.LifeCycleState.OPEN);
+ }
balancerConfiguration.setThreshold(0.1);
containerBalancer.start(balancerConfiguration);
// waiting for balance completed.
// TODO: this is a temporary implementation for now
// modify this after balancer is fully completed
try {
- Thread.sleep(3000);
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ containerBalancer.stop();
+
+ // balancer should have identified unbalanced nodes
+ Assert.assertFalse(containerBalancer.getUnBalancedNodes().isEmpty());
+ // no container should have been selected
+ Assert.assertTrue(containerBalancer.getSourceToTargetMap().isEmpty());
+
+ // now, close all containers
+ for (ContainerInfo containerInfo : cidToInfoMap.values()) {
+ containerInfo.setState(HddsProtos.LifeCycleState.CLOSED);
+ }
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ containerBalancer.stop();
+ // check whether all selected containers are closed
+ for (ContainerMoveSelection moveSelection:
+ containerBalancer.getSourceToTargetMap().values()) {
+ Assert.assertSame(
+ cidToInfoMap.get(moveSelection.getContainerID()).getState(),
+ HddsProtos.LifeCycleState.CLOSED);
+ }
+ }
+
+ @Test
+ public void containerBalancerShouldStopWhenMaxSizeToMoveIsReached() {
+ balancerConfiguration.setThreshold(0.01);
+ balancerConfiguration.setMaxSizeToMove(10 * OzoneConsts.GB);
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
} catch (InterruptedException e) {}
Assert.assertFalse(containerBalancer.isBalancerRunning());
+ containerBalancer.stop();
Review comment:
Can we add an assertion respect to MaxSize?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -44,23 +49,39 @@
" of the entire cluster) no more than the threshold value.")
private String threshold = "0.1";
- @Config(key = "datanodes.balanced.max", type = ConfigType.INT,
+ @Config(key = "datanodes.balanced.max.per.iteration", type = ConfigType.INT,
Review comment:
It might be better to have it as a percentage of total datanodes in the cluster.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT,
+ StorageUnit.BYTES) > maxSizeToMove) {
+ LOG.info("Hit max size to move limit. {} bytes have already been moved " +
+ "and the limit is {} bytes.", totalSizeMoved, maxSizeToMove);
+ return false;
+ }
Review comment:
We should also check that after move target and source are within threshold of average utilisation.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
+ LOG.error("Container Balancer is already running.");
return false;
}
-
+
+ ozoneConfiguration = new OzoneConfiguration();
this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
LOG.info("Starting Container Balancer...{}", this);
+
//we should start a new balancer thread async
//and response to cli as soon as possible
//TODO: this is a temporary implementation
//modify this later
- currentBalancingThread = new Thread(() -> balance());
+ currentBalancingThread = new Thread(this::balance);
currentBalancingThread.start();
////////////////////////
} finally {
lock.unlock();
}
-
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
- for (int i = 0; i < idleIteration; i++) {
- if (!initializeIteration()) {
+ countDatanodesBalanced = 0;
+ totalSizeMoved = 0;
+ int i = 0;
+ do {
+ this.idleIteration = config.getIdleIteration();
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
+ this.maxSizeToMove = config.getMaxSizeToMove();
+
+ if (!initializeIteration() || !doIteration()) {
//balancer should be stopped immediately
break;
}
- // unBalancedNodes is not cleared since the next iteration uses this
- // iteration's unBalancedNodes to find out how many nodes were balanced
- overUtilizedNodes.clear();
- underUtilizedNodes.clear();
- withinThresholdUtilizedNodes.clear();
- }
+ i++;
+ } while (i < idleIteration);
+
balancerRunning.compareAndSet(true, false);
Review comment:
Better to call stop here.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.hadoop.hdds.scm.container.balancer;
+
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManagerV2;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ReplicationManager;
+import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.TreeSet;
+
+/**
+ * The selection criteria for selecting containers that will be moved and
+ * selecting datanodes that containers will move to.
+ */
+public class ContainerBalancerSelectionCriteria {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerBalancerSelectionCriteria.class);
+
+ private ContainerBalancerConfiguration balancerConfiguration;
+ private NodeManager nodeManager;
+ private ReplicationManager replicationManager;
+ private ContainerManagerV2 containerManagerV2;
+ private Set<ContainerID> selectedContainers;
+ private Set<ContainerID> excludeContainers;
+
+ public ContainerBalancerSelectionCriteria(
+ ContainerBalancerConfiguration balancerConfiguration,
+ NodeManager nodeManager,
+ ReplicationManager replicationManager,
+ ContainerManagerV2 containerManagerV2) {
+ this.balancerConfiguration = balancerConfiguration;
+ this.nodeManager = nodeManager;
+ this.replicationManager = replicationManager;
+ this.containerManagerV2 = containerManagerV2;
+ selectedContainers = new HashSet<>();
+ excludeContainers = balancerConfiguration.getExcludeContainers();
+ }
+
+ /**
+ * Checks whether container is currently undergoing replication or deletion.
+ *
+ * @param containerID Container to check.
+ * @return true if container is replicating or deleting, otherwise false.
+ */
+ private boolean isContainerReplicatingOrDeleting(ContainerID containerID) {
+ return replicationManager.isContainerReplicatingOrDeleting(containerID);
+ }
+
+ /**
+ * Gets containers that are suitable for moving based on the following
+ * required criteria:
+ * 1. Container must not be undergoing replication.
+ * 2. Container must not already be selected for balancing.
+ * 3. Container size should be closer to 5GB.
+ * 4. Container must not be in the configured exclude containers list.
+ * 5. Container should be closed.
+ *
+ * @param node DatanodeDetails for which to find candidate containers.
+ * @return NavigableSet of candidate containers that satisfy the criteria.
+ */
+ public NavigableSet<ContainerID> getCandidateContainers(
+ DatanodeDetails node) {
+ NavigableSet<ContainerID> containerIDSet =
+ new TreeSet<>(orderContainersByUsedBytes());
+ try {
+ containerIDSet.addAll(nodeManager.getContainers(node));
+ } catch (NodeNotFoundException e) {
+ LOG.warn("Could not find Datanode {} while selecting candidate " +
+ "containers for Container Balancer.", node.toString(), e);
+ return containerIDSet;
+ }
+ if (excludeContainers != null) {
+ containerIDSet.removeAll(excludeContainers);
+ }
+ if (selectedContainers != null) {
+ containerIDSet.removeAll(selectedContainers);
+ }
+
+ // remove not closed containers
+ containerIDSet.removeIf(containerID -> {
+ try {
+ return containerManagerV2.getContainer(containerID).getState() !=
+ HddsProtos.LifeCycleState.CLOSED;
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not retrieve ContainerInfo for container {} for " +
+ "checking LifecycleState in ContainerBalancer. Excluding this " +
+ "container.", containerID.toString(), e);
+ return true;
+ }
+ });
+
+ containerIDSet.removeIf(this::isContainerReplicatingOrDeleting);
+ return containerIDSet;
+ }
+
+ /**
+ * Checks if the first container has more used space than second.
+ * @param first first container to compare
+ * @param second second container to compare
+ * @return An integer greater than 0 if first is more used, 0 if they're
+ * the same containers or a container is not found, and a value less than 0
+ * if first is not more used than second.
+ */
+ private int isContainerMoreUsed(ContainerID first,
Review comment:
I think lower utilised containers would be available earlier in the set.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -141,18 +200,231 @@ public void unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced() {
*/
@Test
public void containerBalancerShouldStopWhenMaxDatanodesToBalanceIsReached() {
- balancerConfiguration.setMaxDatanodesToBalance(2);
+ balancerConfiguration.setMaxDatanodesToBalance(4);
+ balancerConfiguration.setThreshold(0.01);
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ Assert.assertFalse(containerBalancer.isBalancerRunning());
Review comment:
Can we also add an assertion in the test to know that 4 datanodes have been balanced?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -136,14 +158,49 @@ public long getMaxSizeToMove() {
}
/**
- * Sets the value of maximum size that will be moved by Container Balancer.
+ * Sets the value of maximum size that will be moved by Container Balancer
+ * in one iteration.
*
* @param maxSizeToMove maximum number of Bytes
*/
public void setMaxSizeToMove(long maxSizeToMove) {
this.maxSizeToMove = maxSizeToMove;
}
+ public long getMaxSizeEnteringTarget() {
+ return maxSizeEnteringTarget;
+ }
+
+ public void setMaxSizeEnteringTarget(long maxSizeEnteringTarget) {
+ this.maxSizeEnteringTarget = maxSizeEnteringTarget;
+ }
+
+ public long getMaxSizeLeavingSource() {
+ return maxSizeLeavingSource;
+ }
+
+ public void setMaxSizeLeavingSource(long maxSizeLeavingSource) {
+ this.maxSizeLeavingSource = maxSizeLeavingSource;
+ }
+
+ public Set<ContainerID> getExcludeContainers() {
+ if (excludeContainers.isEmpty()) {
+ return new HashSet<>();
+ }
+ return Arrays.stream(excludeContainers.split(", |,"))
Review comment:
We can split it by ',' and then trim the whitespace to fetch long value.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
Review comment:
We will need to support multiple container move from a source container.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
+ LOG.error("Container Balancer is already running.");
return false;
}
-
+
+ ozoneConfiguration = new OzoneConfiguration();
this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
LOG.info("Starting Container Balancer...{}", this);
+
//we should start a new balancer thread async
//and response to cli as soon as possible
//TODO: this is a temporary implementation
//modify this later
- currentBalancingThread = new Thread(() -> balance());
+ currentBalancingThread = new Thread(this::balance);
currentBalancingThread.start();
////////////////////////
} finally {
lock.unlock();
}
-
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
- for (int i = 0; i < idleIteration; i++) {
- if (!initializeIteration()) {
+ countDatanodesBalanced = 0;
+ totalSizeMoved = 0;
+ int i = 0;
+ do {
+ this.idleIteration = config.getIdleIteration();
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
+ this.maxSizeToMove = config.getMaxSizeToMove();
+
+ if (!initializeIteration() || !doIteration()) {
Review comment:
If `initializeIteration` and `doIteration` are called for every iteration, it is better to have them into separate statements.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -351,6 +491,87 @@ public static double calculateUtilization(
stat.getCapacity().get().doubleValue();
}
+ boolean canSizeLeaveSource(DatanodeDetails source, long size) {
+ if (sizeLeavingNode.containsKey(source)) {
+ return sizeLeavingNode.get(source) + size <=
+ config.getMaxSizeLeavingSource();
+ }
+ return false;
+ }
+
+ /**
+ * Checks if specified size can enter specified target datanode
+ * according to configuration.
+ * @param target target datanode in which size is entering
+ * @param size size in bytes
+ * @return true if size can enter target, else false
+ */
+ boolean canSizeEnterTarget(DatanodeDetails target,
+ long size) {
+ if (sizeEnteringNode.containsKey(target)) {
+ return sizeEnteringNode.get(target) + size <=
+ config.getMaxSizeEnteringTarget();
+ }
+ return false;
+ }
+
+ /**
+ * Get potential targets for container move. Potential targets are under
+ * utilized and within threshold utilized nodes.
+ * @return A list of potential target DatanodeDetails.
+ */
+ private List<DatanodeDetails> getPotentialTargets() {
+ List<DatanodeDetails> potentialTargets = new ArrayList<>(
+ underUtilizedNodes.size() + withinThresholdUtilizedNodes.size());
+
+ underUtilizedNodes
+ .forEach(node -> potentialTargets.add(node.getDatanodeDetails()));
+ withinThresholdUtilizedNodes
+ .forEach(node -> potentialTargets.add(node.getDatanodeDetails()));
+ return potentialTargets;
+ }
+
+ /**
+ * Updates conditions for balancing, such as total size moved by balancer,
+ * total size that has entered a datanode, and total size that has left a
+ * datanode in this iteration.
+ * @param source source datanode
+ * @param moveSelection selected target datanode and container
+ */
+ private void incSizeSelectedForMoving(DatanodeDetails source,
+ ContainerMoveSelection moveSelection) {
+ DatanodeDetails target =
+ moveSelection.getTargetNode();
+ ContainerInfo container;
+ try {
+ container =
+ containerManager.getContainer(moveSelection.getContainerID());
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not find Container {} while matching source and " +
+ "target nodes in ContainerBalancer",
+ moveSelection.getContainerID(), e);
+ return;
+ }
+ long size = container.getUsedBytes();
+ totalSizeMoved += size;
+
+ // update sizeLeavingNode map with the recent moveSelection
+ if (sizeLeavingNode.containsKey(source)) {
Review comment:
contains check should not be required for sizeLeavingNode and sizeEnteringNode since these maps are already initialised.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
Review comment:
We can check container size directly?
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-884724266
thanks @siddhantsangwan for this work, can you please fix the CI first?
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680950369
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -44,23 +49,39 @@
" of the entire cluster) no more than the threshold value.")
private String threshold = "0.1";
- @Config(key = "datanodes.balanced.max", type = ConfigType.INT,
+ @Config(key = "datanodes.balanced.max.per.iteration", type = ConfigType.INT,
Review comment:
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: 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686499179
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
if you could not fix it in this PR, please ignore or remove this test case for now , i will fix and add it back after this patch is merge.
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-896478834
> these several maps can also be initialized at the constructor function and reused with clear
I'm initializing these maps here since the required initial size for these maps can change for every iteration. And resizing hash maps is a bit expensive. So should we prefer moving them to the constructor or we can leave them here for the garbage collector?
--
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] lokeshj1703 commented on pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-894206856
It would also be a good idea to print the number of datanodes involved and total size moved after an 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680949399
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -136,14 +158,49 @@ public long getMaxSizeToMove() {
}
/**
- * Sets the value of maximum size that will be moved by Container Balancer.
+ * Sets the value of maximum size that will be moved by Container Balancer
+ * in one iteration.
*
* @param maxSizeToMove maximum number of Bytes
*/
public void setMaxSizeToMove(long maxSizeToMove) {
this.maxSizeToMove = maxSizeToMove;
}
+ public long getMaxSizeEnteringTarget() {
+ return maxSizeEnteringTarget;
+ }
+
+ public void setMaxSizeEnteringTarget(long maxSizeEnteringTarget) {
+ this.maxSizeEnteringTarget = maxSizeEnteringTarget;
+ }
+
+ public long getMaxSizeLeavingSource() {
+ return maxSizeLeavingSource;
+ }
+
+ public void setMaxSizeLeavingSource(long maxSizeLeavingSource) {
+ this.maxSizeLeavingSource = maxSizeLeavingSource;
+ }
+
+ public Set<ContainerID> getExcludeContainers() {
+ if (excludeContainers.isEmpty()) {
+ return new HashSet<>();
+ }
+ return Arrays.stream(excludeContainers.split(", |,"))
Review comment:
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: 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r682220995
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
Review comment:
after a deep thought , i think we may add the lock back. although `compareAndSet` can avoid concurrent `start` and concurrent `stop` , but it could not avoid concurrent `start` and `stop`. so i think the solution here is that, we use a lock to protect `start` and `stop` option , and thus `balancerRunning` is also protected by this lock ,so we can make `balancerRunning` `boolen volitile` , which is good too for `isBalancerRunning`
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindTargetGreedy.java
##########
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container.balancer;
+
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.ContainerPlacementStatus;
+import org.apache.hadoop.hdds.scm.PlacementPolicy;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManagerV2;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ContainerReplica;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.stream.Collectors;
+
+/**
+ * Find a target giving preference to more under-utilized nodes.
+ */
+public class FindTargetGreedy implements FindTargetStrategy {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(FindTargetGreedy.class);
+
+ private ContainerManagerV2 containerManager;
+ private PlacementPolicy placementPolicy;
+
+ public FindTargetGreedy(
+ ContainerManagerV2 containerManager,
+ PlacementPolicy placementPolicy) {
+ this.containerManager = containerManager;
+ this.placementPolicy = placementPolicy;
+ }
+
+ /**
+ * Find a {@link ContainerMoveSelection} consisting of a target and
+ * container to move for a source datanode. Favours more under-utilized nodes.
+ * @param source Datanode to find a target for
+ * @param potentialTargets Collection of potential target datanodes
+ * @param candidateContainers Set of candidate containers satisfying
+ * selection criteria
+ * {@link ContainerBalancerSelectionCriteria}
+ * @param canSizeEnterTarget A functional interface whose apply
+ * (DatanodeDetails, Long) method returns true if the size specified in the
+ * second argument can enter the specified DatanodeDetails node
+ * @return Found target and container
+ */
+ @Override
+ public ContainerMoveSelection findTargetForContainerMove(
+ DatanodeDetails source, Collection<DatanodeDetails> potentialTargets,
+ Set<ContainerID> candidateContainers,
+ BiFunction<DatanodeDetails, Long, Boolean> canSizeEnterTarget) {
+ for (DatanodeDetails target : potentialTargets) {
+ for (ContainerID container : candidateContainers) {
+ Set<ContainerReplica> replicas;
+ ContainerInfo containerInfo;
+
+ try {
+ replicas = containerManager.getContainerReplicas(container);
+ containerInfo = containerManager.getContainer(container);
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not get Container {} from Container Manager for " +
+ "obtaining replicas in Container Balancer.", container, e);
+ continue;
+ }
+
+ if (replicas.stream().noneMatch(
+ replica -> replica.getDatanodeDetails().equals(target)) &&
+ containerMoveSatisfiesPlacementPolicy(container, replicas, source,
+ target) &&
+ canSizeEnterTarget.apply(target, containerInfo.getUsedBytes())) {
Review comment:
maybe later we can take networkTopology in to account, for all the candidate targets , we always choose the nearest one in networkTopology. this can be done in a new jira, maybe i can help doing this
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
+ LOG.error("Container Balancer is already running.");
+ return false;
+ }
+ ozoneConfiguration = new OzoneConfiguration();
+ this.config = balancerConfiguration;
+ LOG.info("Starting Container Balancer...{}", this);
- //TODO: this is a temporary implementation
- //modify this later
- currentBalancingThread = new Thread(() -> balance());
- currentBalancingThread.start();
- ////////////////////////
- } finally {
- lock.unlock();
- }
+ //we should start a new balancer thread async
+ //and response to cli as soon as possible
+ //TODO: this is a temporary implementation
+ //modify this later
+ currentBalancingThread = new Thread(this::balance);
+ currentBalancingThread.start();
+ ////////////////////////
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
+ this.idleIteration = config.getIdleIteration();
for (int i = 0; i < idleIteration; i++) {
+ countDatanodesInvolvedPerIteration = 0;
+ sizeMovedPerIteration = 0;
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToInvolvePerIteration =
+ config.getMaxDatanodesToInvolvePerIteration();
+ this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
Review comment:
as you mentioned, i am not sure why these parameters can be changed while balancer is running. i think they are set once in each balance thread. please correct me if i am wrong
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
+ LOG.error("Container Balancer is already running.");
+ return false;
+ }
+ ozoneConfiguration = new OzoneConfiguration();
+ this.config = balancerConfiguration;
+ LOG.info("Starting Container Balancer...{}", this);
- //TODO: this is a temporary implementation
- //modify this later
- currentBalancingThread = new Thread(() -> balance());
- currentBalancingThread.start();
- ////////////////////////
- } finally {
- lock.unlock();
- }
+ //we should start a new balancer thread async
+ //and response to cli as soon as possible
+ //TODO: this is a temporary implementation
+ //modify this later
+ currentBalancingThread = new Thread(this::balance);
+ currentBalancingThread.start();
+ ////////////////////////
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
+ this.idleIteration = config.getIdleIteration();
for (int i = 0; i < idleIteration; i++) {
+ countDatanodesInvolvedPerIteration = 0;
+ sizeMovedPerIteration = 0;
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToInvolvePerIteration =
+ config.getMaxDatanodesToInvolvePerIteration();
+ this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
+
+ // stop balancing if iteration is not initialized
if (!initializeIteration()) {
- //balancer should be stopped immediately
- break;
+ stop();
+ return;
+ }
+ doIteration();
+ if (!isBalancerRunning()) {
+ return;
+ }
+
+ // wait for configured time before starting next iteration, unless
+ // this was the final iteration
+ if (i != idleIteration - 1) {
+ synchronized (this) {
Review comment:
why we need `synchronized` here?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -180,6 +198,16 @@ private boolean initializeIteration() {
return false;
}
+ this.clusterCapacity = 0L;
+ this.clusterUsed = 0L;
+ this.clusterRemaining = 0L;
+
+ this.selectedContainers = new HashSet<>();
+ this.overUtilizedNodes = new ArrayList<>();
+ this.underUtilizedNodes = new ArrayList<>();
+ this.withinThresholdUtilizedNodes = new ArrayList<>();
+ this.unBalancedNodes = new ArrayList<>();
Review comment:
>Also since those lists need to be initialized at least once, we need to create them here. Instead, if this initialization is moved to the constructor, we will need to construct every time we want to restart balancer.
maybe we can construct these list at the constructor function of balancer, and then reuse them with `clear()`
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -63,19 +84,28 @@
private ContainerBalancerMetrics metrics;
private long clusterCapacity;
private long clusterUsed;
- private long clusterRemaining;
private double clusterAvgUtilisation;
private final AtomicBoolean balancerRunning = new AtomicBoolean(false);
private Thread currentBalancingThread;
private Lock lock;
+ private ContainerBalancerSelectionCriteria selectionCriteria;
+ private Map<DatanodeDetails, ContainerMoveSelection> sourceToTargetMap;
+ private Map<DatanodeDetails, Long> sizeLeavingNode;
+ private Map<DatanodeDetails, Long> sizeEnteringNode;
+ private Set<ContainerID> selectedContainers;
+ private FindTargetStrategy findTargetStrategy;
+ private PlacementPolicy placementPolicy;
Review comment:
`placementPolicy` seems not used
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -224,86 +291,284 @@ private boolean initializeIteration() {
withinThresholdUtilizedNodes.add(datanodeUsageInfo);
}
}
+ metrics.setDatanodesNumToBalance(new LongMetric(countDatanodesToBalance));
+ // TODO update dataSizeToBalanceGB metric with overLoadedBytes and
+ // underLoadedBytes
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
+ }
+
+ LOG.info("Container Balancer has identified {} Over-Utilized and {} " +
+ "Under-Utilized Datanodes that need to be balanced.",
+ overUtilizedNodes.size(), underUtilizedNodes.size());
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
+ return true;
+ }
+
+ private IterationResult doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+ moveSelectionToFutureMap = new HashMap<>(unBalancedNodes.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets =
+ updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+ }
+
+ // check move results
+ for (Map.Entry<ContainerMoveSelection,
+ CompletableFuture<ReplicationManager.MoveResult>>
+ futureEntry : moveSelectionToFutureMap.entrySet()) {
+ ContainerMoveSelection moveSelection = futureEntry.getKey();
+ CompletableFuture<ReplicationManager.MoveResult> future =
+ futureEntry.getValue();
try {
- Thread.sleep(50);
+ ReplicationManager.MoveResult result = future.get(
+ config.getMoveTimeout().toMillis(), TimeUnit.MILLISECONDS);
+ if (result == ReplicationManager.MoveResult.COMPLETED) {
+ metrics.incrementMovedContainersNum(1);
+ //TODO update metrics with size moved in this iteration
+ }
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
+ LOG.warn("Container move for container {} was interrupted.",
+ moveSelection.getContainerID(), e);
+ } catch (ExecutionException e) {
+ LOG.warn("Container move for container {} completed exceptionally.",
+ moveSelection.getContainerID(), e);
+ } catch (TimeoutException e) {
+ LOG.warn("Container move for container {} timed out.",
+ moveSelection.getContainerID(), e);
}
- /////////////////////////////
+ }
+ return IterationResult.ITERATION_COMPLETED;
+ }
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
- return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
- }
+ /**
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ *
+ * @param potentialTargets Collection of potential targets to move
+ * container to
+ * @return ContainerMoveSelection containing the selected target and container
+ */
+ private ContainerMoveSelection matchSourceWithTarget(
+ DatanodeDetails source, Collection<DatanodeDetails> potentialTargets) {
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
}
- return true;
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
+ * Checks if limits maxDatanodesToInvolvePerIteration and
+ * maxSizeToMovePerIteration have not been hit.
*
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * @return {@link IterationResult#MAX_DATANODES_TO_INVOLVE_REACHED} if reached
+ * max datanodes to involve limit,
+ * {@link IterationResult#MAX_SIZE_TO_MOVE_REACHED} if reached max size to
+ * move limit, or null if balancing can continue
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private IterationResult checkConditionsForBalancing() {
+ if (countDatanodesInvolvedPerIteration + 2 >
+ maxDatanodesToInvolvePerIteration * totalNodesInCluster) {
Review comment:
better to rename `maxDatanodesToInvolvePerIteration` to `maxDatanodesRatioToInvolvePerIteration`, it is a little confused
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680988635
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
Review comment:
It seems the method name "checkConditionsForBalancing" is a bit confusing. Here, it's only checking the `maxDatanodesToInvolvePerIteration` and `maxSizeToMovePerIteration` limits. We can probably move this method to the Iteration class when it's introduced in [HDDS-5518](https://issues.apache.org/jira/browse/HDDS-5518).
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680983443
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
Review comment:
Created a jira for this issue: https://issues.apache.org/jira/browse/HDDS-5526
--
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] lokeshj1703 commented on pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-896691237
@siddhantsangwan Thanks for the contribution! @JacksonYao287 Thanks for the reviews! I have committed the PR to master branch.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r679752927
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
Review comment:
I've created a separate jira for this: https://issues.apache.org/jira/browse/HDDS-5517
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r685739597
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
+ LOG.error("Container Balancer is already running.");
+ return false;
+ }
+ ozoneConfiguration = new OzoneConfiguration();
+ this.config = balancerConfiguration;
+ LOG.info("Starting Container Balancer...{}", this);
- //TODO: this is a temporary implementation
- //modify this later
- currentBalancingThread = new Thread(() -> balance());
- currentBalancingThread.start();
- ////////////////////////
- } finally {
- lock.unlock();
- }
+ //we should start a new balancer thread async
+ //and response to cli as soon as possible
+ //TODO: this is a temporary implementation
+ //modify this later
+ currentBalancingThread = new Thread(this::balance);
+ currentBalancingThread.start();
+ ////////////////////////
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
+ this.idleIteration = config.getIdleIteration();
for (int i = 0; i < idleIteration; i++) {
+ countDatanodesInvolvedPerIteration = 0;
+ sizeMovedPerIteration = 0;
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToInvolvePerIteration =
+ config.getMaxDatanodesToInvolvePerIteration();
+ this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
+
+ // stop balancing if iteration is not initialized
if (!initializeIteration()) {
- //balancer should be stopped immediately
- break;
+ stop();
+ return;
+ }
+ doIteration();
+ if (!isBalancerRunning()) {
+ return;
+ }
+
+ // wait for configured time before starting next iteration, unless
+ // this was the final iteration
+ if (i != idleIteration - 1) {
+ synchronized (this) {
Review comment:
The `wait` method requires that the current thread owns this object's monitor
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686503901
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
Sure, I'm ignoring it for now
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-883103755
@lokeshj1703 @JacksonYao287 @ChenSammi please take a look!
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686445250
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -200,6 +228,8 @@ private boolean initializeIteration() {
// find over and under utilized nodes
for (DatanodeUsageInfo datanodeUsageInfo : datanodeUsageInfos) {
double utilization = calculateUtilization(datanodeUsageInfo);
+ LOG.info("Utilization for node {} is {}",
+ datanodeUsageInfo.getDatanodeDetails().getUuidString(), utilization);
if (utilization > upperLimit) {
overUtilizedNodes.add(datanodeUsageInfo);
countDatanodesToBalance += 1;
Review comment:
`canSizeLeaveSource`, `getClusterAvgUtilisation` seem not used now, we can remove them
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-887778709
@lokeshj1703 @JacksonYao287 Thanks for your reviews. I have integrated with `ReplicationManager.move` and will address the rest of your comments in following commits.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r675438614
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
Review comment:
better to log how many overUtilizedNodes and underUtilizedNodes also
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -200,6 +228,8 @@ private boolean initializeIteration() {
// find over and under utilized nodes
for (DatanodeUsageInfo datanodeUsageInfo : datanodeUsageInfos) {
double utilization = calculateUtilization(datanodeUsageInfo);
+ LOG.info("Utilization for node {} is {}",
+ datanodeUsageInfo.getDatanodeDetails().getUuidString(), utilization);
if (utilization > upperLimit) {
overUtilizedNodes.add(datanodeUsageInfo);
countDatanodesToBalance += 1;
Review comment:
remove `countDatanodesToBalance` , `overLoadedBytes`, `underLoadedBytes` ， no use here
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
Review comment:
before starting balancer thread, we need to check whether the current scm is out of safe mode and is ratis leader?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
Review comment:
seems we do not need a lock here. `balancerRunning.compareAndSet` can take the same effect for avoid current balancer thread
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
Review comment:
maybe. we also. need to check is scm Leader now?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
+ LOG.error("Container Balancer is already running.");
return false;
}
-
+
+ ozoneConfiguration = new OzoneConfiguration();
this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
LOG.info("Starting Container Balancer...{}", this);
+
//we should start a new balancer thread async
//and response to cli as soon as possible
//TODO: this is a temporary implementation
//modify this later
- currentBalancingThread = new Thread(() -> balance());
+ currentBalancingThread = new Thread(this::balance);
currentBalancingThread.start();
////////////////////////
} finally {
lock.unlock();
}
-
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
- for (int i = 0; i < idleIteration; i++) {
- if (!initializeIteration()) {
+ countDatanodesBalanced = 0;
+ totalSizeMoved = 0;
+ int i = 0;
+ do {
+ this.idleIteration = config.getIdleIteration();
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
+ this.maxSizeToMove = config.getMaxSizeToMove();
Review comment:
move these code out of the loop, and `for` is preferred for loop
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -180,6 +198,16 @@ private boolean initializeIteration() {
return false;
}
+ this.clusterCapacity = 0L;
+ this.clusterUsed = 0L;
+ this.clusterRemaining = 0L;
+
+ this.selectedContainers = new HashSet<>();
+ this.overUtilizedNodes = new ArrayList<>();
+ this.underUtilizedNodes = new ArrayList<>();
+ this.withinThresholdUtilizedNodes = new ArrayList<>();
+ this.unBalancedNodes = new ArrayList<>();
Review comment:
remove this
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,25 +104,22 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration();
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
Review comment:
remove `clusterRemaining ` in current code , seems not used
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
Review comment:
suppose totalSizeMoved = 97G ,maxSizeToMove = 100G , and we have a closed 2G ( < default 5G) container replica, can we move it?
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
Review comment:
we should call replicationManager#move here??
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
Review comment:
may be we can clear the HashMap for the sake of reuse , not to create new one everytime
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r676319513
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
Review comment:
According to this implementation, no. I've done this to avoid searching for a target datanode and container (and fail early) in case the maximum limit is about to be reached. Alternatively, we can perform this check after getting a `ContainerMoveSelection`.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680950553
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -351,6 +491,87 @@ public static double calculateUtilization(
stat.getCapacity().get().doubleValue();
}
+ boolean canSizeLeaveSource(DatanodeDetails source, long size) {
+ if (sizeLeavingNode.containsKey(source)) {
+ return sizeLeavingNode.get(source) + size <=
+ config.getMaxSizeLeavingSource();
+ }
+ return false;
+ }
+
+ /**
+ * Checks if specified size can enter specified target datanode
+ * according to configuration.
+ * @param target target datanode in which size is entering
+ * @param size size in bytes
+ * @return true if size can enter target, else false
+ */
+ boolean canSizeEnterTarget(DatanodeDetails target,
+ long size) {
+ if (sizeEnteringNode.containsKey(target)) {
+ return sizeEnteringNode.get(target) + size <=
+ config.getMaxSizeEnteringTarget();
+ }
+ return false;
+ }
+
+ /**
+ * Get potential targets for container move. Potential targets are under
+ * utilized and within threshold utilized nodes.
+ * @return A list of potential target DatanodeDetails.
+ */
+ private List<DatanodeDetails> getPotentialTargets() {
+ List<DatanodeDetails> potentialTargets = new ArrayList<>(
+ underUtilizedNodes.size() + withinThresholdUtilizedNodes.size());
+
+ underUtilizedNodes
+ .forEach(node -> potentialTargets.add(node.getDatanodeDetails()));
+ withinThresholdUtilizedNodes
+ .forEach(node -> potentialTargets.add(node.getDatanodeDetails()));
+ return potentialTargets;
+ }
+
+ /**
+ * Updates conditions for balancing, such as total size moved by balancer,
+ * total size that has entered a datanode, and total size that has left a
+ * datanode in this iteration.
+ * @param source source datanode
+ * @param moveSelection selected target datanode and container
+ */
+ private void incSizeSelectedForMoving(DatanodeDetails source,
+ ContainerMoveSelection moveSelection) {
+ DatanodeDetails target =
+ moveSelection.getTargetNode();
+ ContainerInfo container;
+ try {
+ container =
+ containerManager.getContainer(moveSelection.getContainerID());
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not find Container {} while matching source and " +
+ "target nodes in ContainerBalancer",
+ moveSelection.getContainerID(), e);
+ return;
+ }
+ long size = container.getUsedBytes();
+ totalSizeMoved += size;
+
+ // update sizeLeavingNode map with the recent moveSelection
+ if (sizeLeavingNode.containsKey(source)) {
Review comment:
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: 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r685750819
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
+ LOG.error("Container Balancer is already running.");
+ return false;
+ }
+ ozoneConfiguration = new OzoneConfiguration();
+ this.config = balancerConfiguration;
+ LOG.info("Starting Container Balancer...{}", this);
- //TODO: this is a temporary implementation
- //modify this later
- currentBalancingThread = new Thread(() -> balance());
- currentBalancingThread.start();
- ////////////////////////
- } finally {
- lock.unlock();
- }
+ //we should start a new balancer thread async
+ //and response to cli as soon as possible
+ //TODO: this is a temporary implementation
+ //modify this later
+ currentBalancingThread = new Thread(this::balance);
+ currentBalancingThread.start();
+ ////////////////////////
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
+ this.idleIteration = config.getIdleIteration();
for (int i = 0; i < idleIteration; i++) {
+ countDatanodesInvolvedPerIteration = 0;
+ sizeMovedPerIteration = 0;
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToInvolvePerIteration =
+ config.getMaxDatanodesToInvolvePerIteration();
+ this.maxSizeToMovePerIteration = config.getMaxSizeToMovePerIteration();
Review comment:
You're right. Balancer can be stopped and restarted if these parameters need to be edited. I'll change this.
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-885333220
> it can be reproduced on `master`, too, in about 5% of runs
@adoroszlai thank for pointing out this , i will fix this flaky case in that jira
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r676317453
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
+ LOG.error("Container Balancer is already running.");
return false;
}
-
+
+ ozoneConfiguration = new OzoneConfiguration();
this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
LOG.info("Starting Container Balancer...{}", this);
+
//we should start a new balancer thread async
//and response to cli as soon as possible
//TODO: this is a temporary implementation
//modify this later
- currentBalancingThread = new Thread(() -> balance());
+ currentBalancingThread = new Thread(this::balance);
currentBalancingThread.start();
////////////////////////
} finally {
lock.unlock();
}
-
return true;
}
/**
* Balances the cluster.
*/
private void balance() {
- for (int i = 0; i < idleIteration; i++) {
- if (!initializeIteration()) {
+ countDatanodesBalanced = 0;
+ totalSizeMoved = 0;
+ int i = 0;
+ do {
+ this.idleIteration = config.getIdleIteration();
+ this.threshold = config.getThreshold();
+ this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
+ this.maxSizeToMove = config.getMaxSizeToMove();
Review comment:
`idleIteration` should be moved out, but the rest should stay inside the loop, right? `threshold` and other parameters can be changed while balancer is running. So their values should be checked before every 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 pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-891007701
@JacksonYao287 @lokeshj1703 I've addressed your reviews and created some new jiras wherever required.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r675538940
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
Review comment:
yes, I will add that in another commit
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686018632
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindTargetGreedy.java
##########
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.container.balancer;
+
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.scm.ContainerPlacementStatus;
+import org.apache.hadoop.hdds.scm.PlacementPolicy;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManagerV2;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ContainerReplica;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.stream.Collectors;
+
+/**
+ * Find a target giving preference to more under-utilized nodes.
+ */
+public class FindTargetGreedy implements FindTargetStrategy {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(FindTargetGreedy.class);
+
+ private ContainerManagerV2 containerManager;
+ private PlacementPolicy placementPolicy;
+
+ public FindTargetGreedy(
+ ContainerManagerV2 containerManager,
+ PlacementPolicy placementPolicy) {
+ this.containerManager = containerManager;
+ this.placementPolicy = placementPolicy;
+ }
+
+ /**
+ * Find a {@link ContainerMoveSelection} consisting of a target and
+ * container to move for a source datanode. Favours more under-utilized nodes.
+ * @param source Datanode to find a target for
+ * @param potentialTargets Collection of potential target datanodes
+ * @param candidateContainers Set of candidate containers satisfying
+ * selection criteria
+ * {@link ContainerBalancerSelectionCriteria}
+ * @param canSizeEnterTarget A functional interface whose apply
+ * (DatanodeDetails, Long) method returns true if the size specified in the
+ * second argument can enter the specified DatanodeDetails node
+ * @return Found target and container
+ */
+ @Override
+ public ContainerMoveSelection findTargetForContainerMove(
+ DatanodeDetails source, Collection<DatanodeDetails> potentialTargets,
+ Set<ContainerID> candidateContainers,
+ BiFunction<DatanodeDetails, Long, Boolean> canSizeEnterTarget) {
+ for (DatanodeDetails target : potentialTargets) {
+ for (ContainerID container : candidateContainers) {
+ Set<ContainerReplica> replicas;
+ ContainerInfo containerInfo;
+
+ try {
+ replicas = containerManager.getContainerReplicas(container);
+ containerInfo = containerManager.getContainer(container);
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not get Container {} from Container Manager for " +
+ "obtaining replicas in Container Balancer.", container, e);
+ continue;
+ }
+
+ if (replicas.stream().noneMatch(
+ replica -> replica.getDatanodeDetails().equals(target)) &&
+ containerMoveSatisfiesPlacementPolicy(container, replicas, source,
+ target) &&
+ canSizeEnterTarget.apply(target, containerInfo.getUsedBytes())) {
Review comment:
Yes, we can do this in another jira.
--
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] lokeshj1703 merged pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
lokeshj1703 merged pull request #2441:
URL: https://github.com/apache/ozone/pull/2441
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r685121622
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,79 +114,90 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
- this.config = new ContainerBalancerConfiguration();
+ this.config = new ContainerBalancerConfiguration(ozoneConfiguration);
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
+ this.placementPolicy = placementPolicy;
- this.clusterCapacity = 0L;
- this.clusterUsed = 0L;
- this.clusterRemaining = 0L;
-
- this.overUtilizedNodes = new ArrayList<>();
- this.underUtilizedNodes = new ArrayList<>();
- this.unBalancedNodes = new ArrayList<>();
- this.withinThresholdUtilizedNodes = new ArrayList<>();
this.lock = new ReentrantLock();
+ findTargetStrategy =
+ new FindTargetGreedy(containerManager, placementPolicy);
}
+
/**
* Starts ContainerBalancer. Current implementation is incomplete.
*
* @param balancerConfiguration Configuration values.
*/
- public boolean start(
- ContainerBalancerConfiguration balancerConfiguration) {
- lock.lock();
- try {
- if (!balancerRunning.compareAndSet(false, true)) {
- LOG.info("Container Balancer is already running.");
- return false;
- }
-
- this.config = balancerConfiguration;
- this.idleIteration = config.getIdleIteration();
- this.threshold = config.getThreshold();
- this.maxDatanodesToBalance = config.getMaxDatanodesToBalance();
- this.maxSizeToMoveInGB = config.getMaxSizeToMove();
- this.unBalancedNodes = new ArrayList<>();
- LOG.info("Starting Container Balancer...{}", this);
- //we should start a new balancer thread async
- //and response to cli as soon as possible
+ public boolean start(ContainerBalancerConfiguration balancerConfiguration) {
+ if (!balancerRunning.compareAndSet(false, true)) {
Review comment:
Makes sense, changing this.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r678157317
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,25 +104,22 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration();
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
Review comment:
So you're suggesting fields such as `sizeMovedPerIteration` should be in the Iteration class? What about methods being used inside an iteration, such as `matchSourceWithTarget`?
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-896014962
It looks like `testContainerBalancerCLIOperations` is expecting the wrong behavior. Container Balancer does not find any unbalanced nodes in the cluster set up by this test and hence stops, but the test expects it to continue running.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686450821
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
Review comment:
can you handle this? these can also be initialized at the constructor function and reused with `clear`
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -200,6 +228,8 @@ private boolean initializeIteration() {
// find over and under utilized nodes
for (DatanodeUsageInfo datanodeUsageInfo : datanodeUsageInfos) {
double utilization = calculateUtilization(datanodeUsageInfo);
+ LOG.info("Utilization for node {} is {}",
+ datanodeUsageInfo.getDatanodeDetails().getUuidString(), utilization);
if (utilization > upperLimit) {
overUtilizedNodes.add(datanodeUsageInfo);
countDatanodesToBalance += 1;
Review comment:
can you handle this? by the way, `canSizeLeaveSource`, `getClusterAvgUtilisation` seem not used now, we can remove them
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -219,7 +505,7 @@ private double createNodesInCluster() {
for (double utilization : nodeUtilizations) {
// select a random index from 0 to capacities.length
- int index = ThreadLocalRandom.current().nextInt(0, capacities.length);
Review comment:
`createNodesInCluster` seems not used
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
yes , that is a temporary implementation before balancer is fully completed and it is just for testing cli command, so it is may not Compatible with current balancer logic. could you also fix it in 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 a change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r685988635
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -224,86 +291,284 @@ private boolean initializeIteration() {
withinThresholdUtilizedNodes.add(datanodeUsageInfo);
}
}
+ metrics.setDatanodesNumToBalance(new LongMetric(countDatanodesToBalance));
+ // TODO update dataSizeToBalanceGB metric with overLoadedBytes and
+ // underLoadedBytes
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
+ }
+
+ LOG.info("Container Balancer has identified {} Over-Utilized and {} " +
+ "Under-Utilized Datanodes that need to be balanced.",
+ overUtilizedNodes.size(), underUtilizedNodes.size());
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
+ return true;
+ }
+
+ private IterationResult doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+ moveSelectionToFutureMap = new HashMap<>(unBalancedNodes.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets =
+ updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+ }
+
+ // check move results
+ for (Map.Entry<ContainerMoveSelection,
+ CompletableFuture<ReplicationManager.MoveResult>>
+ futureEntry : moveSelectionToFutureMap.entrySet()) {
+ ContainerMoveSelection moveSelection = futureEntry.getKey();
+ CompletableFuture<ReplicationManager.MoveResult> future =
+ futureEntry.getValue();
try {
- Thread.sleep(50);
+ ReplicationManager.MoveResult result = future.get(
+ config.getMoveTimeout().toMillis(), TimeUnit.MILLISECONDS);
+ if (result == ReplicationManager.MoveResult.COMPLETED) {
+ metrics.incrementMovedContainersNum(1);
+ //TODO update metrics with size moved in this iteration
+ }
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
+ LOG.warn("Container move for container {} was interrupted.",
+ moveSelection.getContainerID(), e);
+ } catch (ExecutionException e) {
+ LOG.warn("Container move for container {} completed exceptionally.",
+ moveSelection.getContainerID(), e);
+ } catch (TimeoutException e) {
+ LOG.warn("Container move for container {} timed out.",
+ moveSelection.getContainerID(), e);
}
- /////////////////////////////
+ }
+ return IterationResult.ITERATION_COMPLETED;
+ }
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
- return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
- }
+ /**
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ *
+ * @param potentialTargets Collection of potential targets to move
+ * container to
+ * @return ContainerMoveSelection containing the selected target and container
+ */
+ private ContainerMoveSelection matchSourceWithTarget(
+ DatanodeDetails source, Collection<DatanodeDetails> potentialTargets) {
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
}
- return true;
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
+ * Checks if limits maxDatanodesToInvolvePerIteration and
+ * maxSizeToMovePerIteration have not been hit.
*
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * @return {@link IterationResult#MAX_DATANODES_TO_INVOLVE_REACHED} if reached
+ * max datanodes to involve limit,
+ * {@link IterationResult#MAX_SIZE_TO_MOVE_REACHED} if reached max size to
+ * move limit, or null if balancing can continue
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private IterationResult checkConditionsForBalancing() {
+ if (countDatanodesInvolvedPerIteration + 2 >
+ maxDatanodesToInvolvePerIteration * totalNodesInCluster) {
Review comment:
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: 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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680909336
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -141,18 +200,231 @@ public void unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced() {
*/
@Test
public void containerBalancerShouldStopWhenMaxDatanodesToBalanceIsReached() {
- balancerConfiguration.setMaxDatanodesToBalance(2);
+ balancerConfiguration.setMaxDatanodesToBalance(4);
+ balancerConfiguration.setThreshold(0.01);
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ Assert.assertFalse(containerBalancer.isBalancerRunning());
Review comment:
Adding this assertion. Also adding an enum for `IterationResult`. Once the `Iteration` class is added with [HDDS-5518](https://issues.apache.org/jira/browse/HDDS-5518), we can also check that the iteration returns a result showing this limit has been reached.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686450821
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
Review comment:
these several maps can also be initialized at the constructor function and reused with `clear`
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r679763189
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -84,25 +104,22 @@ public ContainerBalancer(
ContainerManagerV2 containerManager,
ReplicationManager replicationManager,
OzoneConfiguration ozoneConfiguration,
- final SCMContext scmContext) {
+ final SCMContext scmContext,
+ PlacementPolicy placementPolicy) {
this.nodeManager = nodeManager;
this.containerManager = containerManager;
this.replicationManager = replicationManager;
this.ozoneConfiguration = ozoneConfiguration;
this.config = new ContainerBalancerConfiguration();
this.metrics = new ContainerBalancerMetrics();
this.scmContext = scmContext;
Review comment:
Created another jira for this issue: https://issues.apache.org/jira/browse/HDDS-5518
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680336062
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -180,6 +198,16 @@ private boolean initializeIteration() {
return false;
}
+ this.clusterCapacity = 0L;
+ this.clusterUsed = 0L;
+ this.clusterRemaining = 0L;
+
+ this.selectedContainers = new HashSet<>();
+ this.overUtilizedNodes = new ArrayList<>();
+ this.underUtilizedNodes = new ArrayList<>();
+ this.withinThresholdUtilizedNodes = new ArrayList<>();
+ this.unBalancedNodes = new ArrayList<>();
Review comment:
`clusterCapacity`, `clusterUsed` and `clusterRemaining ` will be renewed in `calculateAvgUtilization` at every iteration, seems no need to set the to 0 here specially.
and other variables can be renewed using `clear()` , no need to create a new object every time
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680988635
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
Review comment:
It seems the method name "checkConditionsForBalancing" is a bit confusing. Here, it's only checking the `maxDatanodesToInvolvePerIteration` and `maxSizeToMovePerIteration` to limits. We can probably move this method to the Iteration class when it's introduced in [HDDS-5518](https://issues.apache.org/jira/browse/HDDS-5518).
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-896692975
Thanks for the reviews!
> We also need to stop balancer when SCM is stopped.
Yes, I've created [HDDS-5603](https://issues.apache.org/jira/browse/HDDS-5603) for this.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680673055
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE,
+ ScmConfigKeys.OZONE_SCM_CONTAINER_SIZE_DEFAULT,
+ StorageUnit.BYTES) > maxSizeToMove) {
+ LOG.info("Hit max size to move limit. {} bytes have already been moved " +
+ "and the limit is {} bytes.", totalSizeMoved, maxSizeToMove);
+ return false;
+ }
Review comment:
I think we can generally assume that the configuration limits `maxSizeLeavingSource` and `maxSizeEnteringTarget` will prevent a source from becoming under-utilized and target from becoming over-utilized.
--
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 #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-885002741
please merge the latest master branch and try again, this [PR](https://github.com/apache/ozone/pull/2446) seems may the test case not flaky
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r678258702
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -226,78 +256,188 @@ private boolean initializeIteration() {
}
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
-
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
- try {
- Thread.sleep(50);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- /////////////////////////////
+ }
+
+ LOG.info("Container Balancer has identified Datanodes that need to be" +
+ " balanced.");
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
+ return true;
+ }
+
+ private boolean doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ if (!checkConditionsForBalancing()) {
+ LOG.info("Conditions for balancing failed. Stopping Container " +
+ "Balancer...");
+ return false;
+ }
+
+ ContainerMoveSelection moveSelection = matchSourceWithTarget(source,
+ potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ // move container
+ // if move successful, do the following
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
}
}
return true;
}
/**
- * Performs binary search to determine if the specified listToSearch
- * contains the specified node.
- *
- * @param listToSearch List of DatanodeUsageInfo to be searched.
- * @param node DatanodeUsageInfo to be searched for.
- * @return true if the specified node is present in listToSearch, otherwise
- * false.
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ * @param potentialTargets List of potential targets to move container to
+ * @return ContainerMoveSelection containing the selected target and container
*/
- private boolean containsNode(
- List<DatanodeUsageInfo> listToSearch, DatanodeUsageInfo node) {
- int index = 0;
- Comparator<DatanodeUsageInfo> comparator =
- DatanodeUsageInfo.getMostUsedByRemainingRatio();
- int size = listToSearch.size();
- if (size == 0) {
- return false;
+ private ContainerMoveSelection matchSourceWithTarget(DatanodeDetails source,
+ List<DatanodeDetails> potentialTargets) {
+
+ NavigableSet<ContainerID> candidateContainers =
+ selectionCriteria.getCandidateContainers(source);
+
+ if (candidateContainers.isEmpty()) {
+ LOG.info("ContainerBalancer could not find any candidate containers for" +
+ " datanode {}", source.getUuidString());
+ return null;
+ }
+ LOG.info("ContainerBalancer is finding suitable target for source " +
+ "datanode {}", source.getUuidString());
+ ContainerMoveSelection moveSelection =
+ findTargetStrategy.findTargetForContainerMove(
+ source, potentialTargets, candidateContainers,
+ this::canSizeEnterTarget);
+
+ if (moveSelection == null) {
+ LOG.info("ContainerBalancer could not find a suitable target for " +
+ "source node {}.", source.getUuidString());
+ return null;
}
- if (comparator.compare(listToSearch.get(0),
- listToSearch.get(size - 1)) < 0) {
- index =
- Collections.binarySearch(listToSearch, node, comparator.reversed());
- } else {
- index = Collections.binarySearch(listToSearch, node, comparator);
+ LOG.info("ContainerBalancer matched source datanode {} with target " +
+ "datanode {} for container move.", source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ return moveSelection;
+ }
+
+ /**
+ * Checks if limits maxDatanodesToBalance and maxSizeToMove have not been hit.
+ * @return true if conditions pass and balancing can continue, else false
+ */
+ private boolean checkConditionsForBalancing() {
+ if (countDatanodesBalanced + 2 > maxDatanodesToBalance) {
+ LOG.info("Hit max datanodes to balance limit. {} datanodes have already" +
+ " been balanced and the limit is {}.", countDatanodesBalanced,
+ maxDatanodesToBalance);
+ return false;
}
- return index >= 0 && listToSearch.get(index).equals(node);
+ if (totalSizeMoved + (long) ozoneConfiguration.getStorageSize(
Review comment:
Yes, if we check after selecting a target datanode and container. In that case, some extra work would be performed if the check eventually fails.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r675532970
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -113,49 +130,50 @@ public boolean start(
lock.lock();
try {
if (!balancerRunning.compareAndSet(false, true)) {
Review comment:
This check was moved to the `initializeIteration` method as per [this comment](https://github.com/apache/ozone/pull/2230#discussion_r635006627) in #2230
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680925885
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -180,6 +198,16 @@ private boolean initializeIteration() {
return false;
}
+ this.clusterCapacity = 0L;
+ this.clusterUsed = 0L;
+ this.clusterRemaining = 0L;
+
+ this.selectedContainers = new HashSet<>();
+ this.overUtilizedNodes = new ArrayList<>();
+ this.underUtilizedNodes = new ArrayList<>();
+ this.withinThresholdUtilizedNodes = new ArrayList<>();
+ this.unBalancedNodes = new ArrayList<>();
Review comment:
Since one purpose of `intitalizeIteration` is to initialize the fields necessary for balancing, it seems clearer to initialize these values here instead of `calculateAvgUtilization`. Also since those lists need to be initialized at least once, we need to create them here. Instead, if this initialization is moved to the constructor, we will need to construct every time we want to restart balancer.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686499179
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
if you could not fix it in this PR, please ignore or remove this test case for now , i will fix and add it back after this patch is 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] siddhantsangwan commented on pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-883103755
@lokeshj1703 @JacksonYao287 @ChenSammi please take a look!
--
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] adoroszlai commented on pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#issuecomment-885012630
@JacksonYao287 it can be reproduced on `master`, too, in about 5% of runs (see repro patch in HDDS-5484).
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r678252724
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -180,6 +198,16 @@ private boolean initializeIteration() {
return false;
}
+ this.clusterCapacity = 0L;
+ this.clusterUsed = 0L;
+ this.clusterRemaining = 0L;
+
+ this.selectedContainers = new HashSet<>();
+ this.overUtilizedNodes = new ArrayList<>();
+ this.underUtilizedNodes = new ArrayList<>();
+ this.withinThresholdUtilizedNodes = new ArrayList<>();
+ this.unBalancedNodes = new ArrayList<>();
Review comment:
Where do you suggest these lists should be initialized then?
--
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] lokeshj1703 commented on a change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
lokeshj1703 commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r684164492
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java
##########
@@ -131,28 +190,255 @@ public void unBalancedNodesListShouldBeEmptyWhenClusterIsBalanced() {
balancerConfiguration.setThreshold(0.99);
containerBalancer.start(balancerConfiguration);
- Assert.assertEquals(0, containerBalancer.getUnBalancedNodes().size());
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {}
+
containerBalancer.stop();
+ Assert.assertEquals(0, containerBalancer.getUnBalancedNodes().size());
}
/**
- * Checks whether ContainerBalancer stops when the limit of
- * MaxDatanodesToBalance is reached.
+ * ContainerBalancer should not involve more datanodes than the
+ * MaxDatanodesToInvolvePerIteration limit.
*/
@Test
- public void containerBalancerShouldStopWhenMaxDatanodesToBalanceIsReached() {
- balancerConfiguration.setMaxDatanodesToBalance(2);
+ public void containerBalancerShouldObeyMaxDatanodesToInvolveLimit() {
+ balancerConfiguration.setMaxDatanodesToInvolvePerIteration(0.3d);
+ balancerConfiguration.setMaxSizeToMovePerIteration(100 * OzoneConsts.GB);
+ balancerConfiguration.setThreshold(0.01);
+ balancerConfiguration.setIdleIteration(1);
+ containerBalancer.start(balancerConfiguration);
+
+ // waiting for balance completed.
+ // TODO: this is a temporary implementation for now
+ // modify this after balancer is fully completed
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException e) {}
+
+ Assert.assertFalse(
Review comment:
NIT: We could also add assertions for metrics here. Can be done in a separate PR.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -224,86 +291,284 @@ private boolean initializeIteration() {
withinThresholdUtilizedNodes.add(datanodeUsageInfo);
}
}
+ metrics.setDatanodesNumToBalance(new LongMetric(countDatanodesToBalance));
+ // TODO update dataSizeToBalanceGB metric with overLoadedBytes and
+ // underLoadedBytes
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
+ }
+
+ LOG.info("Container Balancer has identified {} Over-Utilized and {} " +
+ "Under-Utilized Datanodes that need to be balanced.",
+ overUtilizedNodes.size(), underUtilizedNodes.size());
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
+ return true;
+ }
+
+ private IterationResult doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+ moveSelectionToFutureMap = new HashMap<>(unBalancedNodes.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
Review comment:
NIT: This is not a failure. We can just print the enum to indicate end of iteration.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -224,86 +291,284 @@ private boolean initializeIteration() {
withinThresholdUtilizedNodes.add(datanodeUsageInfo);
}
}
+ metrics.setDatanodesNumToBalance(new LongMetric(countDatanodesToBalance));
+ // TODO update dataSizeToBalanceGB metric with overLoadedBytes and
+ // underLoadedBytes
Collections.reverse(underUtilizedNodes);
- long countDatanodesBalanced = 0;
- // count number of nodes that were balanced in previous iteration
- for (DatanodeUsageInfo node : unBalancedNodes) {
- if (!containsNode(overUtilizedNodes, node) &&
- !containsNode(underUtilizedNodes, node)) {
- countDatanodesBalanced += 1;
- }
- }
- // calculate total number of nodes that have been balanced so far
- countDatanodesBalanced =
- metrics.incrementDatanodesNumBalanced(countDatanodesBalanced);
-
unBalancedNodes = new ArrayList<>(
overUtilizedNodes.size() + underUtilizedNodes.size());
+ unBalancedNodes.addAll(overUtilizedNodes);
+ unBalancedNodes.addAll(underUtilizedNodes);
- if (countDatanodesBalanced + countDatanodesToBalance >
- maxDatanodesToBalance) {
- LOG.info("Approaching Max Datanodes To Balance limit in Container " +
- "Balancer. Stopping Balancer.");
+ if (unBalancedNodes.isEmpty()) {
+ LOG.info("Did not find any unbalanced Datanodes.");
return false;
- } else {
- unBalancedNodes.addAll(overUtilizedNodes);
- unBalancedNodes.addAll(underUtilizedNodes);
+ }
+
+ LOG.info("Container Balancer has identified {} Over-Utilized and {} " +
+ "Under-Utilized Datanodes that need to be balanced.",
+ overUtilizedNodes.size(), underUtilizedNodes.size());
+
+ selectionCriteria = new ContainerBalancerSelectionCriteria(config,
+ nodeManager, replicationManager, containerManager);
+ sourceToTargetMap = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+
+ // initialize maps to track how much size is leaving and entering datanodes
+ sizeLeavingNode = new HashMap<>(overUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ overUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeLeavingNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+
+ sizeEnteringNode = new HashMap<>(underUtilizedNodes.size() +
+ withinThresholdUtilizedNodes.size());
+ underUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
+ withinThresholdUtilizedNodes.forEach(datanodeUsageInfo -> sizeEnteringNode
+ .put(datanodeUsageInfo.getDatanodeDetails(), 0L));
- //for now, we just sleep to simulate the execution of balancer
- //this if for acceptance test now. modify this later when balancer
- //if fully completed
+ return true;
+ }
+
+ private IterationResult doIteration() {
+ List<DatanodeDetails> potentialTargets = getPotentialTargets();
+ Set<DatanodeDetails> selectedTargets =
+ new HashSet<>(potentialTargets.size());
+ moveSelectionToFutureMap = new HashMap<>(unBalancedNodes.size());
+
+ // match each overUtilized node with a target
+ for (DatanodeUsageInfo datanodeUsageInfo : overUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(), source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets = updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+
+ // if not all underUtilized nodes have been selected, try to match
+ // withinThresholdUtilized nodes with underUtilized nodes
+ if (selectedTargets.size() < underUtilizedNodes.size()) {
+ potentialTargets.removeAll(selectedTargets);
+ Collections.reverse(withinThresholdUtilizedNodes);
+
+ for (DatanodeUsageInfo datanodeUsageInfo : withinThresholdUtilizedNodes) {
+ DatanodeDetails source = datanodeUsageInfo.getDatanodeDetails();
+ IterationResult result = checkConditionsForBalancing();
+ if (result != null) {
+ LOG.info("Conditions for balancing failed. Exiting current " +
+ "iteration...");
+ return result;
+ }
+
+ ContainerMoveSelection moveSelection =
+ matchSourceWithTarget(source, potentialTargets);
+
+ if (moveSelection != null) {
+ LOG.info("ContainerBalancer is trying to move container {} from " +
+ "source datanode {} to target datanode {}",
+ moveSelection.getContainerID().toString(),
+ source.getUuidString(),
+ moveSelection.getTargetNode().getUuidString());
+
+ if (moveContainer(source, moveSelection)) {
+ // consider move successful for now, and update selection criteria
+ potentialTargets =
+ updateTargetsAndSelectionCriteria(potentialTargets,
+ selectedTargets, moveSelection, source);
+ }
+ }
+ }
+ }
+
+ // check move results
+ for (Map.Entry<ContainerMoveSelection,
+ CompletableFuture<ReplicationManager.MoveResult>>
+ futureEntry : moveSelectionToFutureMap.entrySet()) {
+ ContainerMoveSelection moveSelection = futureEntry.getKey();
+ CompletableFuture<ReplicationManager.MoveResult> future =
+ futureEntry.getValue();
try {
- Thread.sleep(50);
+ ReplicationManager.MoveResult result = future.get(
+ config.getMoveTimeout().toMillis(), TimeUnit.MILLISECONDS);
+ if (result == ReplicationManager.MoveResult.COMPLETED) {
+ metrics.incrementMovedContainersNum(1);
+ //TODO update metrics with size moved in this iteration
+ }
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
+ LOG.warn("Container move for container {} was interrupted.",
+ moveSelection.getContainerID(), e);
+ } catch (ExecutionException e) {
+ LOG.warn("Container move for container {} completed exceptionally.",
+ moveSelection.getContainerID(), e);
+ } catch (TimeoutException e) {
+ LOG.warn("Container move for container {} timed out.",
+ moveSelection.getContainerID(), e);
}
- /////////////////////////////
+ }
+ return IterationResult.ITERATION_COMPLETED;
+ }
- if (unBalancedNodes.isEmpty()) {
- LOG.info("Did not find any unbalanced Datanodes.");
- return false;
- } else {
- LOG.info("Container Balancer has identified Datanodes that need to be" +
- " balanced.");
- }
+ /**
+ * Match a source datanode with a target datanode and identify the container
+ * to move.
+ *
+ * @param potentialTargets Collection of potential targets to move
+ * container to
+ * @return ContainerMoveSelection containing the selected target and container
+ */
+ private ContainerMoveSelection matchSourceWithTarget(
Review comment:
NIT: There are a few info logs in matchSourceWithTarget and checkConditionsForBalancing which can be converted to debug.
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerConfiguration.java
##########
@@ -44,23 +54,70 @@
" of the entire cluster) no more than the threshold value.")
private String threshold = "0.1";
- @Config(key = "datanodes.balanced.max", type = ConfigType.INT,
- defaultValue = "5", tags = {ConfigTag.BALANCER}, description = "The " +
- "maximum number of datanodes that should be balanced. Container " +
- "Balancer will not balance more number of datanodes than this limit.")
- private int maxDatanodesToBalance = 5;
+ @Config(key = "datanodes.involved.max.per.iteration", type = ConfigType.AUTO,
Review comment:
NIT: rename `max` to `ratio` or sth better.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r680949311
##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java
##########
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.hadoop.hdds.scm.container.balancer;
+
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
+import org.apache.hadoop.hdds.scm.container.ContainerInfo;
+import org.apache.hadoop.hdds.scm.container.ContainerManagerV2;
+import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
+import org.apache.hadoop.hdds.scm.container.ReplicationManager;
+import org.apache.hadoop.hdds.scm.node.NodeManager;
+import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.NavigableSet;
+import java.util.Set;
+import java.util.TreeSet;
+
+/**
+ * The selection criteria for selecting containers that will be moved and
+ * selecting datanodes that containers will move to.
+ */
+public class ContainerBalancerSelectionCriteria {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ContainerBalancerSelectionCriteria.class);
+
+ private ContainerBalancerConfiguration balancerConfiguration;
+ private NodeManager nodeManager;
+ private ReplicationManager replicationManager;
+ private ContainerManagerV2 containerManagerV2;
+ private Set<ContainerID> selectedContainers;
+ private Set<ContainerID> excludeContainers;
+
+ public ContainerBalancerSelectionCriteria(
+ ContainerBalancerConfiguration balancerConfiguration,
+ NodeManager nodeManager,
+ ReplicationManager replicationManager,
+ ContainerManagerV2 containerManagerV2) {
+ this.balancerConfiguration = balancerConfiguration;
+ this.nodeManager = nodeManager;
+ this.replicationManager = replicationManager;
+ this.containerManagerV2 = containerManagerV2;
+ selectedContainers = new HashSet<>();
+ excludeContainers = balancerConfiguration.getExcludeContainers();
+ }
+
+ /**
+ * Checks whether container is currently undergoing replication or deletion.
+ *
+ * @param containerID Container to check.
+ * @return true if container is replicating or deleting, otherwise false.
+ */
+ private boolean isContainerReplicatingOrDeleting(ContainerID containerID) {
+ return replicationManager.isContainerReplicatingOrDeleting(containerID);
+ }
+
+ /**
+ * Gets containers that are suitable for moving based on the following
+ * required criteria:
+ * 1. Container must not be undergoing replication.
+ * 2. Container must not already be selected for balancing.
+ * 3. Container size should be closer to 5GB.
+ * 4. Container must not be in the configured exclude containers list.
+ * 5. Container should be closed.
+ *
+ * @param node DatanodeDetails for which to find candidate containers.
+ * @return NavigableSet of candidate containers that satisfy the criteria.
+ */
+ public NavigableSet<ContainerID> getCandidateContainers(
+ DatanodeDetails node) {
+ NavigableSet<ContainerID> containerIDSet =
+ new TreeSet<>(orderContainersByUsedBytes());
+ try {
+ containerIDSet.addAll(nodeManager.getContainers(node));
+ } catch (NodeNotFoundException e) {
+ LOG.warn("Could not find Datanode {} while selecting candidate " +
+ "containers for Container Balancer.", node.toString(), e);
+ return containerIDSet;
+ }
+ if (excludeContainers != null) {
+ containerIDSet.removeAll(excludeContainers);
+ }
+ if (selectedContainers != null) {
+ containerIDSet.removeAll(selectedContainers);
+ }
+
+ // remove not closed containers
+ containerIDSet.removeIf(containerID -> {
+ try {
+ return containerManagerV2.getContainer(containerID).getState() !=
+ HddsProtos.LifeCycleState.CLOSED;
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not retrieve ContainerInfo for container {} for " +
+ "checking LifecycleState in ContainerBalancer. Excluding this " +
+ "container.", containerID.toString(), e);
+ return true;
+ }
+ });
+
+ containerIDSet.removeIf(this::isContainerReplicatingOrDeleting);
+ return containerIDSet;
+ }
+
+ /**
+ * Checks if the first container has more used space than second.
+ * @param first first container to compare
+ * @param second second container to compare
+ * @return An integer greater than 0 if first is more used, 0 if they're
+ * the same containers or a container is not found, and a value less than 0
+ * if first is not more used than second.
+ */
+ private int isContainerMoreUsed(ContainerID first,
Review comment:
Fixed this.
--
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 change in pull request #2441: HDDS-4929. Select target datanodes and containers to move for Container Balancer
Posted by GitBox <gi...@apache.org>.
siddhantsangwan commented on a change in pull request #2441:
URL: https://github.com/apache/ozone/pull/2441#discussion_r686027933
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestContainerBalancerOperations.java
##########
@@ -99,7 +99,7 @@ public void testContainerBalancerCLIOperations() throws Exception {
// test normally start , and stop it before balance is completed
containerBalancerClient.startContainerBalancer(threshold, idleiterations,
- maxDatanodesToBalance, maxSizeToMoveInGB);
+ maxDatanodesRatioToInvolvePerIteration, maxSizeToMovePerIterationInGB);
running = containerBalancerClient.getContainerBalancerStatus();
assertTrue(running);
Review comment:
Balancing completes and exits before asserting that balancer is running because no unbalanced nodes are found.
--
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