Hadoop Common
  1. Hadoop Common
  2. HADOOP-5679

Resolve findbugs warnings in core/streaming/pipes/examples

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed

      Description

      Towards a solution for HADOOP-5628, we need to resolve all findbugs warnings. This jira will try to resolve the findbugs warnings where ever possible and suppress them where resolution is not possible.

      1. hadoop-5679.patch
        22 kB
        Jothi Padmanabhan

        Issue Links

          Activity

          Hide
          Jothi Padmanabhan added a comment -

          Patch for review

          Show
          Jothi Padmanabhan added a comment - Patch for review
          Hide
          Sharad Agarwal added a comment -

          Patch looks good.
          Minor nit: The signature of public method ProgramDriver#driver(String[] args) has changed. So should we make this as incompatible change ?

          Show
          Sharad Agarwal added a comment - Patch looks good. Minor nit: The signature of public method ProgramDriver#driver(String[] args) has changed. So should we make this as incompatible change ?
          Hide
          Jothi Padmanabhan added a comment -

          Ant tests passed on my local box.

          Test Patch result

          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 3 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          [exec]
          [exec] -1 release audit. The applied patch generated 481 release audit warnings (more than the trunk's current 474 warnings).

          Show
          Jothi Padmanabhan added a comment - Ant tests passed on my local box. Test Patch result [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] -1 release audit. The applied patch generated 481 release audit warnings (more than the trunk's current 474 warnings).
          Hide
          Sharad Agarwal added a comment -

          I just committed this. Thanks Jothi.

          Show
          Sharad Agarwal added a comment - I just committed this. Thanks Jothi.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk #830 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/830/)
          . Resolve findbugs warnings in core/streaming/pipes/examples. Contributed by Jothi Padmanabhan.

          Show
          Hudson added a comment - Integrated in Hadoop-trunk #830 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/830/ ) . Resolve findbugs warnings in core/streaming/pipes/examples. Contributed by Jothi Padmanabhan.
          Hide
          Jordà Polo added a comment - - edited

          Running local jobs seems to fail for me since this patch was applied ("Mkdirs failed to create...").

          The new code checks if either mkdirs() on hadoop.tmp.dir failed OR if it isn't a directory... but it doesn't take into account that the directory may already exist. If the directory existed before, it will always fail and exit since mkdirs() only returns "true" if it is able to create the directory.

          Wouldn't an AND be more appropriate here? I mean (!b && !tmpDir.isDirectory()) instead of (!b || !tmpDir.isDirectory()) in src/core/org/apache/hadoop/util/RunJar.java, line 111.

          EDIT: The original code (before applying hadoop-5679.patch) looks fine to me too: (!tmpDir.isDirectory()).

          Show
          Jordà Polo added a comment - - edited Running local jobs seems to fail for me since this patch was applied ("Mkdirs failed to create..."). The new code checks if either mkdirs() on hadoop.tmp.dir failed OR if it isn't a directory... but it doesn't take into account that the directory may already exist. If the directory existed before, it will always fail and exit since mkdirs() only returns "true" if it is able to create the directory. Wouldn't an AND be more appropriate here? I mean (!b && !tmpDir.isDirectory()) instead of (!b || !tmpDir.isDirectory()) in src/core/org/apache/hadoop/util/RunJar.java, line 111. EDIT: The original code (before applying hadoop-5679.patch) looks fine to me too: (!tmpDir.isDirectory()) .
          Hide
          Jothi Padmanabhan added a comment -

          Yes, this is a bug. HADOOP-5809 resolves this.

          Show
          Jothi Padmanabhan added a comment - Yes, this is a bug. HADOOP-5809 resolves this.
          Hide
          Robert Chansler added a comment -

          Editorial pass over all release notes prior to publication of 0.21. This is just routine.

          Show
          Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21. This is just routine.
          Hide
          Todd Lipcon added a comment -

          Though ProgramDriver is marked as private and Unstable, it was in use by HBase - see HBASE-4179. So, this is an incompatible change, I think. We will probably work around it in HBase by forking the class, but worth being aware of.

          Show
          Todd Lipcon added a comment - Though ProgramDriver is marked as private and Unstable, it was in use by HBase - see HBASE-4179 . So, this is an incompatible change, I think. We will probably work around it in HBase by forking the class, but worth being aware of.

            People

            • Assignee:
              Jothi Padmanabhan
              Reporter:
              Jothi Padmanabhan
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development