Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-5679

Resolve findbugs warnings in core/streaming/pipes/examples

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
          jothipn Jothi Padmanabhan added a comment -

          Patch for review

          Show
          jothipn Jothi Padmanabhan added a comment - Patch for review
          Hide
          sharadag 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
          sharadag 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
          jothipn 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
          jothipn 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
          sharadag Sharad Agarwal added a comment -

          I just committed this. Thanks Jothi.

          Show
          sharadag Sharad Agarwal added a comment - I just committed this. Thanks Jothi.
          Hide
          hudson 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 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
          jorda 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
          jorda 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
          jothipn Jothi Padmanabhan added a comment -

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

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

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

          Show
          chansler Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21. This is just routine.
          Hide
          tlipcon 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
          tlipcon 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:
              jothipn Jothi Padmanabhan
              Reporter:
              jothipn Jothi Padmanabhan
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development