HDDS-10160. Cache sort results in ContainerBalancerSelectionCriteria #6032
Conversation
|
@symious It's better to create dev branches in one's own fork instead of in |
|
I guess @symious may accidentally push HDDS-10160 to ozone major repo instead of his own repo. Hi @guohao-rosicky , could Minyu help to review and verify the patch? |
sumitagrawl
left a comment
sumitagrawl
left a comment
There was a problem hiding this comment.
@symious Thanks for working over this, given few comments
| NavigableSet<ContainerID> containerIDSet = | ||
| new TreeSet<>(orderContainersByUsedBytes().reversed()); | ||
| // Initialize containerSet for node | ||
| if (!setMap.containsKey(node)) { |
There was a problem hiding this comment.
ContaienrBalancerSelectionCriteria is applicable within one iteration
- all filtering can be done before setting to setMap
- Next filtering can be basic including selectedContainers and shouldBeExcluded
Suggested to use getCandidateContainers() to update setMap wrapping in another method.
| "containers for Container Balancer.", node.toString(), e); | ||
| return containerIDSet; | ||
| setMap.remove(node); | ||
| return new TreeSet<>(); |
There was a problem hiding this comment.
can return Collections.emptySet()
| new TreeSet<>(orderContainersByUsedBytes().reversed()); | ||
| // Initialize containerSet for node | ||
| if (!setMap.containsKey(node)) { | ||
| NavigableSet<ContainerID> newSet = |
There was a problem hiding this comment.
can return Set<> and can change NavigableSet<> to Set in usages, but implementation can refer TreeMap.
| NavigableSet<ContainerID> newSet = | ||
| new TreeSet<>(orderContainersByUsedBytes().reversed()); | ||
| try { | ||
| newSet.addAll(nodeManager.getContainers(node)); |
There was a problem hiding this comment.
IMO, sorting can be done on filtered containers only to avoid sorting of containers not meeting criteria.
|
@adoroszlai @ChenSammi Sorry for mixing the repo, should be using the wrong repo when working on the release. |
|
Will continue this thread on #6050. |
4 participants
What changes were proposed in this pull request?
HDDS-10160. Cache sort results in ContainerBalancerSelectionCriteria
The sort of all the containers is very time consuming, this patch is to cache the sort result and improve the balancer speed.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10160
How was this patch tested?
Original balancer tests running successfully would be enough.