adoroszlai commented on code in PR #6508:
URL: https://github.com/apache/ozone/pull/6508#discussion_r1559245486


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/CachingSpaceUsageSource.java:
##########
@@ -137,9 +139,24 @@ private void refresh() {
     //only one `refresh` can be running at a certain moment
     if (isRefreshRunning.compareAndSet(false, true)) {
       try {
-        cachedValue.set(source.getUsedSpace());
+        long newUsedSpace = source.getUsedSpace();

Review Comment:
   `refresh()` has already been keeping the original value if `getUsedSpace()` 
throws exception.  So the patch cannot fix negative space.  I think validation 
should be added in `decrementUsedSpace()` instead.



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/CachingSpaceUsageSource.java:
##########
@@ -137,9 +139,24 @@ private void refresh() {
     //only one `refresh` can be running at a certain moment
     if (isRefreshRunning.compareAndSet(false, true)) {
       try {
-        cachedValue.set(source.getUsedSpace());
+        long newUsedSpace = source.getUsedSpace();
+        // Check for negative value before setting it to the cache
+        if (newUsedSpace >= 0) {
+          cachedValue.set(newUsedSpace);
+        } else {
+          LOG.error(
+              "Received negative used space value: {}. Keeping the last known 
good value: {}.",
+              newUsedSpace, cachedValue.get());
+        }
+      } catch (UncheckedIOException e) {
+        // Log the error and do not update the cached value
+        LOG.error(
+            "Error refreshing space usage for {}. Keeping the last known good 
value: {}",
+            source, cachedValue.get(), e);

Review Comment:
   There is no need to duplicate the `catch` block, since 
`UncheckedIOException` is a `RuntimeException`.  (Also, if it weren't, a single 
clause could `catch` multiple exceptions.)



-- 
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]

Reply via email to