Gargi-jais11 commented on code in PR #9628:
URL: https://github.com/apache/ozone/pull/9628#discussion_r2684754671


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/io/RandomAccessFileChannel.java:
##########


Review Comment:
   This is not related to your changes but I found a bug over here.
   Here, `blockFile` is set to null before it was used in the catch blocks 
   for error logging. This would cause log messages to show `null` instead of 
the actual file path when closing fails making debugging difficult.
   **Solution:** Save the file reference so that error messages can properly 
display the file path.
   ```
   public synchronized void close() {
       if (blockFile == null) {
         return;
       }
       final File fileToClose = blockFile;
       blockFile = null;
       try {
          channel.close();
          channel = null;
       } catch (IOException e) {
         LOG.warn("Failed to close channel for {}", fileToClose, e);
       }
       try {
        raf.close();
        raf = null;
       } catch (IOException e) {
         LOG.warn("Failed to close RandomAccessFile for {}", fileToClose, e);
       }
     }
   }
   ```



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