sumitagrawl commented on code in PR #7339:
URL: https://github.com/apache/ozone/pull/7339#discussion_r1853680233
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/AbstractFindTargetGreedy.java:
##########
@@ -46,7 +45,7 @@ public abstract class AbstractFindTargetGreedy implements
FindTargetStrategy {
private Logger logger;
private ContainerManager containerManager;
private PlacementPolicyValidateProxy placementPolicyValidateProxy;
- private Map<DatanodeDetails, Long> sizeEnteringNode;
+ private ConcurrentHashMap<DatanodeDetails, Long> sizeEnteringNode;
Review Comment:
Reference can be as Map<> which can refer concurrentHashMap
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -361,23 +355,20 @@ public List<ContainerBalancerTaskIterationStatusInfo>
getCurrentIterationsStatis
findTargetStrategy.getSizeEnteringNodes()
.entrySet()
.stream()
- .filter(Objects::nonNull)
.filter(datanodeDetailsLongEntry ->
datanodeDetailsLongEntry.getValue() > 0)
- .collect(Collectors.toMap(
- entry -> entry.getKey().getUuid(),
- entry -> entry.getValue() / OzoneConsts.GB
- )
+ .collect(
+ ConcurrentHashMap::new,
+ (map, entry) -> map.put(entry.getKey().getUuid(),
entry.getValue() / OzoneConsts.GB),
+ ConcurrentHashMap::putAll
),
findSourceStrategy.getSizeLeavingNodes()
.entrySet()
.stream()
- .filter(Objects::nonNull)
.filter(datanodeDetailsLongEntry ->
datanodeDetailsLongEntry.getValue() > 0)
.collect(
- Collectors.toMap(
- entry -> entry.getKey().getUuid(),
- entry -> entry.getValue() / OzoneConsts.GB
- )
+ ConcurrentHashMap::new,
+ (map, entry) -> map.put(entry.getKey().getUuid(),
entry.getValue() / OzoneConsts.GB),
+ ConcurrentHashMap::putAll
)
);
List<ContainerBalancerTaskIterationStatusInfo> resultList = new
ArrayList<>(iterationsStatistic);
Review Comment:
we need do this at top and need use this for identifying last iteration
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/AbstractFindTargetGreedy.java:
##########
@@ -280,7 +279,7 @@ NodeManager getNodeManager() {
}
@Override
- public Map<DatanodeDetails, Long> getSizeEnteringNodes() {
+ public ConcurrentHashMap<DatanodeDetails, Long> getSizeEnteringNodes() {
Review Comment:
Need return Map<> only
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/FindTargetStrategy.java:
##########
@@ -70,5 +70,5 @@ void reInitialize(List<DatanodeUsageInfo> potentialDataNodes,
*/
void resetPotentialTargets(@Nonnull Collection<DatanodeDetails> targets);
- Map<DatanodeDetails, Long> getSizeEnteringNodes();
+ ConcurrentHashMap<DatanodeDetails, Long> getSizeEnteringNodes();
Review Comment:
Need use Map<> itself, change all place. But object created can be
ConcurrentHashMap instead of HashMap
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java:
##########
@@ -361,23 +355,20 @@ public List<ContainerBalancerTaskIterationStatusInfo>
getCurrentIterationsStatis
findTargetStrategy.getSizeEnteringNodes()
.entrySet()
.stream()
- .filter(Objects::nonNull)
.filter(datanodeDetailsLongEntry ->
datanodeDetailsLongEntry.getValue() > 0)
- .collect(Collectors.toMap(
- entry -> entry.getKey().getUuid(),
- entry -> entry.getValue() / OzoneConsts.GB
- )
+ .collect(
+ ConcurrentHashMap::new,
Review Comment:
We can use Map itself only here, no code change is required, if base map is
threadsafe. Here new map no need be threadsafe.
--
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]
