xinglin commented on code in PR #5817:
URL: https://github.com/apache/hadoop/pull/5817#discussion_r1256116824


##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BlockChecksumHelper.java:
##########
@@ -493,7 +493,8 @@ void compute() throws IOException {
               checksumBlock(block, idx, liveBlkInfo.getToken(),
                   liveBlkInfo.getDn());
             } catch (IOException ioe) {
-              LOG.warn("Exception while reading checksum", ioe);
+              LOG.warn("Exception while reading checksum for block {} at index 
{} " +
+                  "in blockGroup {}", block, idx, blockGroup, ioe);

Review Comment:
   Hi,
   
   We only have a definition as `warn(final String format, final Throwable t) 
`, which accepts a throwable object for LOG.warn(). For the others, the 
exception is just treated as a regular object. 
   
   
   ```
     public void warn(final String format) {
       this.logger.logIfEnabled(FQCN, Level.WARN, 
(org.apache.logging.log4j.Marker)null, format);
     }
   
     public void warn(final String format, final Object o) {
       this.logger.logIfEnabled(FQCN, Level.WARN, 
(org.apache.logging.log4j.Marker)null, format, o);
     }
   
     public void warn(final String format, final Object arg1, final Object 
arg2) {
       this.logger.logIfEnabled(FQCN, Level.WARN, 
(org.apache.logging.log4j.Marker)null, format, arg1, arg2);
     }
   
     public void warn(final String format, final Object... args) {
       this.logger.logIfEnabled(FQCN, Level.WARN, 
(org.apache.logging.log4j.Marker)null, format, args);
     }
   
     public void warn(final String format, final Throwable t) {
       this.logger.logIfEnabled(FQCN, Level.WARN, 
(org.apache.logging.log4j.Marker)null, format, t);
     }
   ```
   
   Maybe we should do something like the following:
   
   ```
         String msg = String.format("Exception while reading checksum for block 
%s at index %d " +
                     "in blockGroup %s", block, idx, blockGroup);
         LOG.warn(msg, 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