[jira] [Reopened] (HDFS-8840) Inconsistent log level practice

[songwanging (JIRA)]

 [ 
https://issues.apache.org/jira/browse/HDFS-8840?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

songwanging reopened HDFS-8840:
---

we can use LOG.isFatalEnabled() instead of LOG.isDebugEnabled() here, this 
is not a good practice of log.

 Inconsistent log level practice
 ---

 Key: HDFS-8840
 URL: https://issues.apache.org/jira/browse/HDFS-8840
 Project: Hadoop HDFS
  Issue Type: Improvement
Affects Versions: 2.6.0, 2.5.1, 2.5.2, 2.7.1
Reporter: songwanging
Assignee: Jagadesh Kiran N
Priority: Minor
 Attachments: HDFS-8840-00.patch


 In method checkLogsAvailableForRead() of class: 
 hadoop-2.7.1-src\hadoop-hdfs-project\hadoop-hdfs\src\main\java\org\apache\hadoop\hdfs\server\namenode\ha\BootstrapStandby.java
 The log level is not correct, after checking LOG.isDebugEnabled(), we 
 should use LOG.debug(msg, e);, while now we use  LOG.fatal(msg, e);. Log 
 level is inconsistent.
 the source code of this method is:
 private boolean checkLogsAvailableForRead(FSImage image, long imageTxId, long 
 curTxIdOnOtherNode) {
   ...
 } catch (IOException e) {
...
   if (LOG.isDebugEnabled()) {
 LOG.fatal(msg, e);
   } else {
 LOG.fatal(msg);
   }
   return false;
 }
   }



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (HDFS-8840) Inconsistent log level practice

[songwanging (JIRA)]

songwanging created HDFS-8840:
-

 Summary: Inconsistent log level practice
 Key: HDFS-8840
 URL: https://issues.apache.org/jira/browse/HDFS-8840
 Project: Hadoop HDFS
  Issue Type: Improvement
Affects Versions: 2.7.1, 2.5.2, 2.5.1, 2.6.0
Reporter: songwanging
Priority: Minor


In method checkLogsAvailableForRead() of class: 
hadoop-2.7.1-src\hadoop-hdfs-project\hadoop-hdfs\src\main\java\org\apache\hadoop\hdfs\server\namenode\ha\BootstrapStandby.java

The log level is not correct, after checking LOG.isDebugEnabled(), we should 
use LOG.debug(msg, e);, while now we use  LOG.fatal(msg, e);. Log level is 
inconsistent.

the source code of this method is:
private boolean checkLogsAvailableForRead(FSImage image, long imageTxId, long 
curTxIdOnOtherNode) {

  ...
} catch (IOException e) {
   ...
  if (LOG.isDebugEnabled()) {
LOG.fatal(msg, e);
  } else {
LOG.fatal(msg);
  }
  return false;
}
  }




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (HDFS-8842) Catch throwable

[songwanging (JIRA)]

songwanging created HDFS-8842:
-

 Summary: Catch throwable 
 Key: HDFS-8842
 URL: https://issues.apache.org/jira/browse/HDFS-8842
 Project: Hadoop HDFS
  Issue Type: Bug
Reporter: songwanging
Priority: Critical


We came across a few instances where the code catches Throwable, but fails to 
rethrow anything.
Throwable is the parent type of Exception and Error, so catching Throwable 
means catching both Exceptions as well as Errors. An Exception is something you 
could recover (like IOException), an Error is something more serious and 
usually you could'nt recover easily (like ClassNotFoundError) so it doesn't 
make much sense to catch an Error. 
We should convert Throwable to Exception.

For example:

In method tryGetPid(Process p) of class: 
hadoop-2.7.1-src\hadoop-common-project\hadoop-common\src\main\java\org\apache\hadoop\ha\ShellCommandFencer.java

code:
private static String tryGetPid(Process p) {
try {
...
} catch (Throwable t) {
  LOG.trace(Unable to determine pid for  + p, t);
  return null;
}
  }

In method uncaughtException(Thread t, Throwable e) of class: 
hadoop-2.7.1-src\hadoop-yarn-project\hadoop-yarn\hadoop-yarn-common\src\main\java\org\apache\hadoop\yarn\YarnUncaughtExceptionHandler.java

code:
public void uncaughtException(Thread t, Throwable e) {
   ...
  try {
LOG.fatal(Thread  + t +  threw an Error.  Shutting down now..., e);
  } catch (Throwable err) {
//We don't want to not exit because of an issue with logging
  }
...
try {
  System.err.println(Halting due to Out Of Memory Error...);
} catch (Throwable err) {
  //Again we done want to exit because of logging issues.
}
 ...
}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (HDFS-8841) Catch throwable return null

[songwanging (JIRA)]

songwanging created HDFS-8841:
-

 Summary: Catch throwable return null
 Key: HDFS-8841
 URL: https://issues.apache.org/jira/browse/HDFS-8841
 Project: Hadoop HDFS
  Issue Type: Bug
Reporter: songwanging
Priority: Minor


In method map of class: 
\hadoop-2.7.1-src\hadoop-tools\hadoop-extras\src\main\java\org\apache\hadoop\tools\DistCpV1.java.

This method has this code:

 public void map(LongWritable key,
FilePair value,
OutputCollectorWritableComparable?, Text out,
Reporter reporter) throws IOException {
 ...
} catch (Throwable ex) {
  // ignore, we are just cleaning up
  LOG.debug(Ignoring cleanup exception, ex);
}
   
  }
} 
...
}

Throwable is the parent type of Exception and Error, so catching Throwable 
means catching both Exceptions as well as Errors. An Exception is something you 
could recover (like IOException), an Error is something more serious and 
usually you could'nt recover easily (like ClassNotFoundError) so it doesn't 
make much sense to catch an Error.

We should  convert to catch Exception instead.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)