sumitagrawl commented on code in PR #6032:
URL: https://github.com/apache/ozone/pull/6032#discussion_r1461446103
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -92,15 +96,31 @@ private boolean
isContainerReplicatingOrDeleting(ContainerID containerID) {
*/
public NavigableSet<ContainerID> getCandidateContainers(
DatanodeDetails node, long sizeMovedAlready) {
- NavigableSet<ContainerID> containerIDSet =
- new TreeSet<>(orderContainersByUsedBytes().reversed());
+ // Initialize containerSet for node
+ if (!setMap.containsKey(node)) {
Review Comment:
ContaienrBalancerSelectionCriteria is applicable within one iteration
1. all filtering can be done before setting to setMap
2. Next filtering can be basic including selectedContainers and
shouldBeExcluded
Suggested to use getCandidateContainers() to update setMap wrapping in
another method.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -92,15 +96,31 @@ private boolean
isContainerReplicatingOrDeleting(ContainerID containerID) {
*/
public NavigableSet<ContainerID> getCandidateContainers(
DatanodeDetails node, long sizeMovedAlready) {
- NavigableSet<ContainerID> containerIDSet =
- new TreeSet<>(orderContainersByUsedBytes().reversed());
+ // Initialize containerSet for node
+ if (!setMap.containsKey(node)) {
+ NavigableSet<ContainerID> newSet =
+ new TreeSet<>(orderContainersByUsedBytes().reversed());
+ try {
+ newSet.addAll(nodeManager.getContainers(node));
+ } catch (NodeNotFoundException e) {
+ LOG.warn("Could not find Datanode {} while selecting candidate " +
+ "containers for Container Balancer.", node.toString(), e);
+ return newSet;
+ }
+ setMap.put(node, newSet);
+ }
+
+ // In case the node is removed
try {
- containerIDSet.addAll(nodeManager.getContainers(node));
+ nodeManager.getContainers(node);
} catch (NodeNotFoundException e) {
LOG.warn("Could not find Datanode {} while selecting candidate " +
"containers for Container Balancer.", node.toString(), e);
- return containerIDSet;
+ setMap.remove(node);
+ return new TreeSet<>();
Review Comment:
can return Collections.emptySet()
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -92,15 +96,31 @@ private boolean
isContainerReplicatingOrDeleting(ContainerID containerID) {
*/
public NavigableSet<ContainerID> getCandidateContainers(
DatanodeDetails node, long sizeMovedAlready) {
- NavigableSet<ContainerID> containerIDSet =
- new TreeSet<>(orderContainersByUsedBytes().reversed());
+ // Initialize containerSet for node
+ if (!setMap.containsKey(node)) {
+ NavigableSet<ContainerID> newSet =
+ new TreeSet<>(orderContainersByUsedBytes().reversed());
+ try {
+ newSet.addAll(nodeManager.getContainers(node));
Review Comment:
IMO, sorting can be done on filtered containers only to avoid sorting of
containers not meeting criteria.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerSelectionCriteria.java:
##########
@@ -92,15 +96,31 @@ private boolean
isContainerReplicatingOrDeleting(ContainerID containerID) {
*/
public NavigableSet<ContainerID> getCandidateContainers(
DatanodeDetails node, long sizeMovedAlready) {
- NavigableSet<ContainerID> containerIDSet =
- new TreeSet<>(orderContainersByUsedBytes().reversed());
+ // Initialize containerSet for node
+ if (!setMap.containsKey(node)) {
+ NavigableSet<ContainerID> newSet =
Review Comment:
can return Set<> and can change NavigableSet<> to Set in usages, but
implementation can refer TreeMap.
--
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]
