adoroszlai commented on code in PR #7830:
URL: https://github.com/apache/ozone/pull/7830#discussion_r1955607066
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/RoundRobinVolumeChoosingPolicy.java:
##########
@@ -61,7 +66,7 @@ public HddsVolume chooseVolume(List<HddsVolume> volumes,
int startVolumeIndex = currentVolumeIndex;
while (true) {
- final HddsVolume volume = volumes.get(currentVolumeIndex);
+ final HddsVolume volume =
volumesWithWriteAllowed.get(currentVolumeIndex);
Review Comment:
`currentVolumeIndex` is calculated based on `volumes.size()`, so this could
result in `IndexOutOfBoundsException`.
Volumes should not be eagerly pre-filtered, they can be filtered in the loop.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/CapacityVolumeChoosingPolicy.java:
##########
@@ -58,9 +58,13 @@ public HddsVolume chooseVolume(List<HddsVolume> volumes,
throw new DiskOutOfSpaceException("No more available volumes");
}
+ List<HddsVolume> volumesWithWriteAllowed =
+ volumes.stream().filter(k -> k.getStorageState() !=
StorageVolume.VolumeState.READ_ONLY)
+ .collect(Collectors.toList());
+
AvailableSpaceFilter filter = new AvailableSpaceFilter(maxContainerSize);
- List<HddsVolume> volumesWithEnoughSpace = volumes.stream()
+ List<HddsVolume> volumesWithEnoughSpace = volumesWithWriteAllowed.stream()
.filter(filter)
.collect(Collectors.toList());
Review Comment:
Please combine the streams, apply both filters to the same stream. (Or move
the condition to `AvailableSpaceFilter`.)
--
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]
