siddhantsangwan commented on code in PR #8590:
URL: https://github.com/apache/ozone/pull/8590#discussion_r2144305412
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java:
##########
@@ -194,6 +195,12 @@ public VolumeInfoMetrics getVolumeInfoStats() {
return volumeInfoMetrics;
}
+ public boolean isVolumeFull() {
+ SpaceUsageSource currentUsage = getCurrentUsage();
+ // if the volume is failed, this method will implicitly return true
because available space will be 0
+ return currentUsage.getAvailable() -
getFreeSpaceToSpare(currentUsage.getCapacity()) <= 0;
+ }
Review Comment:
Thanks for the review @adoroszlai. I get your point and that's what I wanted
to do initially as well. But since this is on the write path, I wanted to avoid
paying the cost of building two extra objects (`StorageLocationReport.Builder`
and `StorageLocationReport`) to achieve the same thing.
Also, to centralise the calculation in `VolumeUsage`, `HddsVolume` first
needs to get the capacity from `VolumeUsage` to get min free space, then pass
it back to `VolumeUsage` for it to compute whether the volume is full. I wanted
to avoid making this extra method call as well.
These are small optimisations, but I chose these over potentially cleaner
code and to avoid doing too much refactoring in this PR (in the interest of
time, and to reduce the scope). Let me know what you think - I'm open to using
`StorageLocationReport` and centralising in `VolumeUsage` if you think overall
that's better.
--
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]
