Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.20.0
    • Component/s: conf, fs, record
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Removed from class org.apache.hadoop.fs.RawLocalFileSystem deprecated methods public String getName(), public void lock(Path p, boolean shared) and public void release(Path p).

      Description

      Findbugs generates several errors related to unused return values, thread synchronization and ambiguous method calls

      1. HADOOP-4253.patch
        13 kB
        Suresh Srinivas
      2. HADOOP-4253.patch
        12 kB
        Suresh Srinivas
      3. HADOOP-4253.patch
        11 kB
        Suresh Srinivas

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        4h 19m 1 Owen O'Malley 29/Sep/08 23:30
        Open Open Patch Available Patch Available
        6d 19h 6m 2 Suresh Srinivas 30/Sep/08 23:30
        Patch Available Patch Available Resolved Resolved
        5d 12h 12m 1 Johan Oskarsson 06/Oct/08 11:43
        Resolved Resolved Closed Closed
        199d 8h 34m 1 Nigel Daley 23/Apr/09 20:17
        Owen O'Malley made changes -
        Component/s contrib/streaming [ 12310972 ]
        Owen O'Malley made changes -
        Component/s mapred [ 12310690 ]
        Owen O'Malley made changes -
        Component/s dfs [ 12310710 ]
        Nigel Daley made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Robert Chansler made changes -
        Release Note Removed from class org.apache.hadoop.fs.RawLocalFileSystem deprecated methods public String getName(), public void lock(Path p, boolean shared) and public void release(Path p).
        Removed from class org.apache.hadoop.fs.RawLocalFileSystem deprecated methods public String getName(), public void lock(Path p, boolean shared) and public void release(Path p).
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Robert Chansler made changes -
        Release Note Incompatible changes:
        - Following deprecated methods in org.apache.hadoop.fs.RawLocalFileSystem are removed:
          public String getName()
          public void lock(Path p, boolean shared)
          public void release(Path p)
        Removed from class org.apache.hadoop.fs.RawLocalFileSystem deprecated methods public String getName(), public void lock(Path p, boolean shared) and public void release(Path p).
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Hide
        Robert Chansler added a comment -

        Edit release note for publication.

        Show
        Robert Chansler added a comment - Edit release note for publication.
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #626 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/626/ )
        Johan Oskarsson made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Hide
        Johan Oskarsson added a comment -

        I just committed this. Thanks, Suresh!

        Show
        Johan Oskarsson added a comment - I just committed this. Thanks, Suresh!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12391226/HADOOP-4253.patch
        against trunk revision 700628.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no tests are needed for this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12391226/HADOOP-4253.patch against trunk revision 700628. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3412/console This message is automatically generated.
        Suresh Srinivas made changes -
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Release Note Following are the incompatible changes:
        - Following deprectated methods in org.apache.hadoop.fs.RawLocalFileSystem are removed:
          public String getName()
          public void lock(Path p, boolean shared)
          public void release(Path p)
        Incompatible changes:
        - Following deprecated methods in org.apache.hadoop.fs.RawLocalFileSystem are removed:
          public String getName()
          public void lock(Path p, boolean shared)
          public void release(Path p)
        Status Open [ 1 ] Patch Available [ 10002 ]
        Suresh Srinivas made changes -
        Attachment HADOOP-4253.patch [ 12391226 ]
        Hide
        Suresh Srinivas added a comment -

        Thanks for the comments. I have attached a new patch that incorporates the suggested changes.

        BTW changing to this.getName() still results in findbugs warnings. Method call this.getName() should be attributed only to the super class in this case and cannot refer to the outer class. I have reported this as a findbugs bug to keep track of this.

        Show
        Suresh Srinivas added a comment - Thanks for the comments. I have attached a new patch that incorporates the suggested changes. BTW changing to this.getName() still results in findbugs warnings. Method call this.getName() should be attributed only to the super class in this case and cannot refer to the outer class. I have reported this as a findbugs bug to keep track of this.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        > The TaskTracker threads, I'd use this.getName() rather that super.getName(). I think it is clearer what the intent is.

        this.getName() still generates a findbugs warning.

        Show
        Tsz Wo Nicholas Sze added a comment - > The TaskTracker threads, I'd use this.getName() rather that super.getName(). I think it is clearer what the intent is. this.getName() still generates a findbugs warning.
        Owen O'Malley made changes -
        Fix Version/s 0.20.0 [ 12313438 ]
        Status Patch Available [ 10002 ] Open [ 1 ]
        Fix Version/s 0.19.0 [ 12313211 ]
        Hide
        Owen O'Malley added a comment -

        Let's just change this in 0.20.

        Other comments:

        • In JarBuilder, I think it is better to just remove the in.close() from addFileStream, since the addNamedStream already closes it.
        • The TaskTracker threads, I'd use this.getName() rather that super.getName(). I think it is clearer what the intent is.
        Show
        Owen O'Malley added a comment - Let's just change this in 0.20. Other comments: In JarBuilder, I think it is better to just remove the in.close() from addFileStream, since the addNamedStream already closes it. The TaskTracker threads, I'd use this.getName() rather that super.getName(). I think it is clearer what the intent is.
        Suresh Srinivas made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Release Note Following are the incompatible changes:
        - Following deprectated methods in org.apache.hadoop.fs.RawLocalFileSystem are removed:
          public String getName()
          public void lock(Path p, boolean shared)
          public void release(Path p)
        Hadoop Flags [Reviewed, Incompatible change] [Incompatible change, Reviewed]
        Tsz Wo Nicholas Sze made changes -
        Hadoop Flags [Incompatible change, Reviewed]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        +1 Patch looks good.

        Need release note since it removed some (deprecated) public APIs

        Show
        Tsz Wo Nicholas Sze added a comment - +1 Patch looks good. Need release note since it removed some (deprecated) public APIs
        Suresh Srinivas made changes -
        Attachment HADOOP-4253.patch [ 12390967 ]
        Hide
        Suresh Srinivas added a comment -

        Incorporated the following changes suggest by Chris and Nicholas:

        • Reverted few of the files as the changes were not simple. They will be addressed separately.
        • RawLocalFileSystem deprectated method lock, release and related data are removed.
        • TaskTracker.MapEventFetcherThread prints the super class Thread.getName() instead of enclosing class TaskTracker.getName().
        Show
        Suresh Srinivas added a comment - Incorporated the following changes suggest by Chris and Nicholas: Reverted few of the files as the changes were not simple. They will be addressed separately. RawLocalFileSystem deprectated method lock, release and related data are removed. TaskTracker.MapEventFetcherThread prints the super class Thread.getName() instead of enclosing class TaskTracker.getName().
        Tsz Wo Nicholas Sze made changes -
        Component/s fs [ 12310689 ]
        Component/s record [ 12310970 ]
        Component/s contrib/streaming [ 12310972 ]
        Component/s mapred [ 12310690 ]
        Component/s dfs [ 12310710 ]
        Component/s conf [ 12310711 ]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        The patch span over various components including conf, fs, dfs, mapred, record and streaming. I think it is better to divide this issue into sub-issues.

        Show
        Tsz Wo Nicholas Sze added a comment - The patch span over various components including conf, fs, dfs, mapred, record and streaming. I think it is better to divide this issue into sub-issues.
        Suresh Srinivas made changes -
        Field Original Value New Value
        Attachment HADOOP-4253.patch [ 12390800 ]
        Hide
        Suresh Srinivas added a comment -

        Changes to fix the following findbugs warnings:

        • Move Stream creation to try block and ensure the stream is closed in finally block
        • DateFormat is not thread safe. Adding synchronization to avoid threading issues.
        • Added synchronized blocks to fix synchronization issues flagged by findbugs
        • Added retrun value checks in some cases to ensure the error returned by calling a method is handled.
        • In TaskTracker, the inner class MapEventsFetcherThread that extends Thread has ambiguous method getName() called. The method getName() is available in the super class Thread and the enclosing class TaskTracker. To avoid the ambiguity, calling the getName() method of the enclosing class.
        Show
        Suresh Srinivas added a comment - Changes to fix the following findbugs warnings: Move Stream creation to try block and ensure the stream is closed in finally block DateFormat is not thread safe. Adding synchronization to avoid threading issues. Added synchronized blocks to fix synchronization issues flagged by findbugs Added retrun value checks in some cases to ensure the error returned by calling a method is handled. In TaskTracker, the inner class MapEventsFetcherThread that extends Thread has ambiguous method getName() called. The method getName() is available in the super class Thread and the enclosing class TaskTracker. To avoid the ambiguity, calling the getName() method of the enclosing class.
        Suresh Srinivas created issue -

          People

          • Assignee:
            Suresh Srinivas
            Reporter:
            Suresh Srinivas
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved:

              Development