adoroszlai commented on code in PR #6050:
URL: https://github.com/apache/ozone/pull/6050#discussion_r1467448678
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -79,38 +84,37 @@ private boolean
isContainerReplicatingOrDeleting(ContainerID 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.
- * 6. If the {@link LegacyReplicationManager} is enabled, then the container
should not be an EC container.
- * @param node DatanodeDetails for which to find candidate containers.
- * @return NavigableSet of candidate containers that satisfy the criteria.
+ * Get ContainerID Set for the Datanode, it will be returned as NavigableSet
+ * Since sorting will be time-consuming, the Set will be cached.
+ *
+ * @param node source datanode
+ * @return cached Navigable ContainerID Set
*/
- public NavigableSet<ContainerID> getCandidateContainers(
- DatanodeDetails node, long sizeMovedAlready) {
- NavigableSet<ContainerID> containerIDSet =
- new TreeSet<>(orderContainersByUsedBytes().reversed());
- 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);
+ public Set<ContainerID> getContainerIDSet(DatanodeDetails node) {
+ // Check if the node is registered at the beginning
+ if (!nodeManager.isNodeRegistered(node)) {
+ return Collections.emptySet();
}
- if (selectedContainers != null) {
- containerIDSet.removeAll(selectedContainers);
+
+ try {
+ // Initialize containerSet for node
+ setMap.computeIfAbsent(node, n -> {
+ try {
+ addNodeToSetMap(n);
+ return setMap.get(n);
+ } catch (NodeNotFoundException e) {
+ LOG.warn("Could not find Datanode {} while selecting candidate " +
+ "containers for Container Balancer.", n.toString(), e);
+ return null;
+ }
+ });
+ } catch (Exception e) {
+ LOG.error("An unexpected error occurred while processing the node.", e);
+ setMap.remove(node);
+ return Collections.emptySet();
}
- containerIDSet.removeIf(
- containerID -> shouldBeExcluded(containerID, node, sizeMovedAlready));
- return containerIDSet;
+ return setMap.get(node);
Review Comment:
Sorry, I just realized that this is a bit more complex than needed.
`addNodeToSetMap` is internally updating `setMap`, while being called via
`computeIfAbsent`.
This can be simplified to:
```java
Set<ContainerID> containers = setMap.computeIfAbsent(node,
this::getCandidateContainers);
return containers != null ? containers : Collections.emptySet();
```
With a small change in `addNodeToSetMap` (renamed to
`getCandidateContainers`, as it no longer changes `setMap`):
```java
private NavigableSet<ContainerID> getCandidateContainers(DatanodeDetails
node) {
NavigableSet<ContainerID> newSet =
new TreeSet<>(orderContainersByUsedBytes().reversed());
try {
Set<ContainerID> idSet = nodeManager.getContainers(node);
if (excludeContainers != null) {
idSet.removeAll(excludeContainers);
}
if (selectedContainers != null) {
idSet.removeAll(selectedContainers);
}
newSet.addAll(idSet);
return newSet;
} catch (NodeNotFoundException e) {
LOG.warn("Could not find Datanode {} while selecting candidate " +
"containers for Container Balancer.", node, e);
return null;
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
