rakeshadr commented on code in PR #10892:
URL: https://github.com/apache/ozone/pull/10892#discussion_r3840546510
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -193,6 +212,7 @@ public boolean shouldBeExcluded(ContainerID containerID,
} catch (ContainerNotFoundException e) {
LOG.warn("Container {} does not exist in ContainerManager. Skipping " +
"this container.", container.getContainerID(), e);
+ excludeContainersNotFound.add(containerID);
Review Comment:
Call `addToExcludeNotFoundContainers(containerID)` instead of
`excludeContainersNotFound.add(containerID);`
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -175,11 +192,13 @@ public boolean shouldBeExcluded(ContainerID containerID,
} catch (ContainerNotFoundException e) {
LOG.warn("Could not find Container {} to check if it should be a " +
"candidate container. Excluding it.", containerID);
+ excludeContainersNotFound.add(containerID);
Review Comment:
Call `addToExcludeNotFoundContainers(containerID)` instead of
`excludeContainersNotFound.add(containerID);`
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -369,6 +389,14 @@ Set<ContainerID> getExcludeDueToFailContainers() {
return excludeContainersDueToFailure;
}
+ public void addToExcludeNotFoundContainers(ContainerID container) {
+ this.excludeContainersNotFound.add(container);
Review Comment:
```
// Capped to avoid unbounded growth in long-running or unlimited-iteration
runs.
private static final int MAX_NOT_FOUND_CONTAINERS = 10_000;
```
```
public void addToExcludeNotFoundContainers(ContainerID container) {
if (excludeContainersNotFound.size() < MAX_NOT_FOUND_CONTAINERS) {
excludeContainersNotFound.add(container);
} else {
LOG.warn("ContainerBalancer not found exclude set has reached the cap of
{}. " +
"Container {} will not be added, it may be retried in subsequent
iterations.",
MAX_NOT_FOUND_CONTAINERS, container);
}
}
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -632,7 +633,7 @@ private boolean initializeIteration() {
selectionCriteria = new ContainerBalancerSelectionCriteria(config,
nodeManager, replicationManager, containerManager, findSourceStrategy,
- containerToSourceMap);
+ containerToSourceMap, excludeContainersNotFound);
Review Comment:
Can you add a log `excludeContainersNotFound.size()` count for better
debuggability. Hope this will be non-empty. Either can add at the beginning or
at the end.
```
if (!excludeContainersNotFound.isEmpty()) {
LOG.info("ContainerBalancer iteration {}: {} containers permanently " +
"excluded (ContainerNotFoundException across prior iterations).",
nextIterationIndex, excludeContainersNotFound.size());
}
```
--
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]
