Hadoop Common
  1. Hadoop Common
  2. HADOOP-4965

DFSClient should log instead of printing into std err.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.17.0
    • Fix Version/s: 0.19.1
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      DFSClient.LeaseChecker.close() uses System.err.println() and Exception.printStackTrace() to output the exception.LOG.error() should be used instead.

      1. TestFileAppend3.patch
        0.5 kB
        Konstantin Shvachko
      2. DFSClientLogging.patch
        1 kB
        Konstantin Shvachko
      3. DFSClientLogging.patch
        1 kB
        Konstantin Shvachko
      4. DFSClientLogging.patch
        0.5 kB
        Konstantin Shvachko

        Issue Links

          Activity

          Hide
          Suresh Srinivas added a comment -

          Previously the file name was printed. Should the file name be printed in the log? Otherwise +1 for the patch.

          Show
          Suresh Srinivas added a comment - Previously the file name was printed. Should the file name be printed in the log? Otherwise +1 for the patch.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          This patch may change the shell commands output.

          Show
          Tsz Wo Nicholas Sze added a comment - This patch may change the shell commands output.
          Hide
          Konstantin Shvachko added a comment -

          This patch

          • prints the file name;
          • stringifyException
          • also closes FileSystem in TestFileAppend3
          Show
          Konstantin Shvachko added a comment - This patch prints the file name; stringifyException also closes FileSystem in TestFileAppend3
          Hide
          Steve Loughran added a comment -

          You shouldn't need to call StringUtils.stringifyException(), because passing it down to the log tools as is lets your logging tool handle the chained strack trace themselves -which can include a more structured output than the stringifyException() call. I'd prefer something like:
          LOG.error("Exception closing file " + src + ": " +ie,ie);

          Show
          Steve Loughran added a comment - You shouldn't need to call StringUtils.stringifyException() , because passing it down to the log tools as is lets your logging tool handle the chained strack trace themselves -which can include a more structured output than the stringifyException() call. I'd prefer something like: LOG.error("Exception closing file " + src + ": " +ie,ie);
          Hide
          Steve Loughran added a comment -

          I thought this bugrep was familiar;

          Show
          Steve Loughran added a comment - I thought this bugrep was familiar;
          Hide
          Konstantin Shvachko added a comment -

          Incorporated Suresh's comment.

          Show
          Konstantin Shvachko added a comment - Incorporated Suresh's comment.
          Hide
          Konstantin Shvachko added a comment -

          Only one unit test failure with this patch. TestMapReduceLocal fails as in HADOOP-4907.

          Show
          Konstantin Shvachko added a comment - Only one unit test failure with this patch. TestMapReduceLocal fails as in HADOOP-4907 .
          Hide
          Suresh Srinivas added a comment -

          +1 for the patch

          Show
          Suresh Srinivas added a comment - +1 for the patch
          Hide
          Konstantin Shvachko added a comment -

          I am removing the DFSClient related code from the patch because it has been resolved by HADOOP-3894. Leaving only the TestFileAppend3 improvement.

          Show
          Konstantin Shvachko added a comment - I am removing the DFSClient related code from the patch because it has been resolved by HADOOP-3894 . Leaving only the TestFileAppend3 improvement.
          Hide
          Konstantin Shvachko added a comment -

          I just committed this.

          Show
          Konstantin Shvachko added a comment - I just committed this.

            People

            • Assignee:
              Konstantin Shvachko
              Reporter:
              Konstantin Shvachko
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development