steveloughran commented on a change in pull request #2963:
URL: https://github.com/apache/hadoop/pull/2963#discussion_r632532149



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/IOStatisticsLogging.java
##########
@@ -249,6 +250,26 @@ public static void logIOStatisticsAtDebug(
     logIOStatisticsAtDebug(LOG, message, source);
   }
 
+  /**
+   * A method to log IOStatistics from a source.
+   *
+   * @param log    Logger for logging.
+   * @param level  LOG level.
+   * @param source Source to LOG.
+   */
+  public static void loggingIOStatistics(Logger log, String level,

Review comment:
       use a name like `logIOStatisticsAtLevel()`

##########
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
##########
@@ -331,7 +331,7 @@ public synchronized void close() throws IOException {
       }
     }
     if (LOG.isDebugEnabled()) {
-      LOG.debug("Closing AbfsOutputStream ", toString());
+      LOG.debug("Closing AbfsOutputStream : {}", toString());

Review comment:
       switch to `LOG.debug("Closing AbfsOutputStream : {}", this)` and you can 
then remove the is debug enabled check.

##########
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java
##########
@@ -490,7 +489,12 @@ public synchronized void close() throws IOException {
     // does all the delete-on-exit calls, and may be slow.
     super.close();
     LOG.debug("AzureBlobFileSystem.close");
-    IOUtils.cleanupWithLogger(LOG, abfsStore, delegationTokenManager);
+    if (getConf() != null) {
+      String iostatisticsLoggingLevel =
+          this.getConf().getTrimmed(IOSTATISTICS_LOGGING_LEVEL,

Review comment:
       nit: remove `this.`

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/IOStatisticsLogging.java
##########
@@ -249,6 +250,26 @@ public static void logIOStatisticsAtDebug(
     logIOStatisticsAtDebug(LOG, message, source);
   }
 
+  /**
+   * A method to log IOStatistics from a source.
+   *
+   * @param log    Logger for logging.
+   * @param level  LOG level.
+   * @param source Source to LOG.
+   */
+  public static void loggingIOStatistics(Logger log, String level,
+      Object source) {
+    if (level.toLowerCase().equals(IOSTATISTICS_LOGGING_LEVEL_INFO)) {

Review comment:
       proposed: use a switch() and support error and warn too; the lower case 
locale must be in EN_US. (trivia: the code as is won't work in turkey). 




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to