Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-1864

PipeMapRed.java has uninitialized members log_ and LOGNAME

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: contrib/streaming
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      PipeMapRed.java has members log_ and LOGNAME, which are never initialized and they are used in code for logging in several places.
      They should be removed and PipeMapRed should use commons LogFactory and Log for logging. This would improve code maintainability.

      Also, as per comment , stream.joblog_ configuration property can be removed.

      1. patch-1864.txt
        12 kB
        Amareshwari Sriramadasu

        Activity

        Amareshwari Sriramadasu created issue -
        Amareshwari Sriramadasu made changes -
        Field Original Value New Value
        Summary PipeMapRed.java has unintialized members log_ and LOGNAME PipeMapRed.java has uninitialized members log_ and LOGNAME
        Amareshwari Sriramadasu made changes -
        Assignee Amareshwari Sriramadasu [ amareshwari ]
        Hide
        Ravi Gummadi added a comment -

        One more unused member is debugFailEarly_

        Show
        Ravi Gummadi added a comment - One more unused member is debugFailEarly_
        Hide
        Ravi Gummadi added a comment -

        Also, I don't see a real need for the following as part of code.

        boolean debug_;
        boolean debugFailEarly_;
        boolean debugFailDuring_;
        boolean debugFailLate_;

        If some debug statements are needed, we could add them as

         if (LOG.isDebugEnabled()) {LOG.debug(......);} 

        So should we remove these 4 variables also ?

        Show
        Ravi Gummadi added a comment - Also, I don't see a real need for the following as part of code. boolean debug_; boolean debugFailEarly_; boolean debugFailDuring_; boolean debugFailLate_; If some debug statements are needed, we could add them as if (LOG.isDebugEnabled()) {LOG.debug(......);} So should we remove these 4 variables also ?
        Hide
        Ravi Gummadi added a comment -

        mapredKey_ is not set at all but referenced in getContext(). We need to set it appropriately.

        Show
        Ravi Gummadi added a comment - mapredKey_ is not set at all but referenced in getContext(). We need to set it appropriately.
        Hide
        Amareshwari Sriramadasu added a comment -

        Patch removes un-initialized and unused variables : fs_, debug_, debugFailEarly_, debugFailDuring_, debugFailLate_, jobLog_, log_, LOGNAME and mapredKey_.

        If some debug statements are needed, we could add them as..

        Two of the debug statements do not look relevant, so removed them. Replaced the other one with LOG.debug.

        mapredKey_ is not set at all but referenced in getContext(). We need to set it appropriately.

        The variable is never set from the first version. The next line in getContext() is adding last output. So, removed this.

        Also removed new Date() from getContext(), because getContext() is called from log message and logger will anyway prefix the log with the current time.

        Show
        Amareshwari Sriramadasu added a comment - Patch removes un-initialized and unused variables : fs_, debug_, debugFailEarly_, debugFailDuring_, debugFailLate_, jobLog_, log_, LOGNAME and mapredKey_. If some debug statements are needed, we could add them as.. Two of the debug statements do not look relevant, so removed them. Replaced the other one with LOG.debug. mapredKey_ is not set at all but referenced in getContext(). We need to set it appropriately. The variable is never set from the first version. The next line in getContext() is adding last output. So, removed this. Also removed new Date() from getContext(), because getContext() is called from log message and logger will anyway prefix the log with the current time.
        Amareshwari Sriramadasu made changes -
        Attachment patch-1864.txt [ 12447931 ]
        Amareshwari Sriramadasu made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12447931/patch-1864.txt
        against trunk revision 957437.

        +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 new tests are needed for this patch.
        Also please list what manual steps were performed to verify 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 release audit. The applied patch does not increase the total number of release audit warnings.

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

        -1 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/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/12447931/patch-1864.txt against trunk revision 957437. +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 new tests are needed for this patch. Also please list what manual steps were performed to verify 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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h6.grid.sp2.yahoo.net/586/console This message is automatically generated.
        Amareshwari Sriramadasu made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Amareshwari Sriramadasu made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12447931/patch-1864.txt
        against trunk revision 958902.

        +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 new tests are needed for this patch.
        Also please list what manual steps were performed to verify 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 release audit. The applied patch does not increase the total number of release audit warnings.

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

        -1 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/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/12447931/patch-1864.txt against trunk revision 958902. +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 new tests are needed for this patch. Also please list what manual steps were performed to verify 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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/273/console This message is automatically generated.
        Hide
        Amareshwari Sriramadasu added a comment -

        -1 contrib tests.

        Test failure is due to MAPREDUCE-1834.

        -1 tests included.

        The patch removes unused code. So, no new tests are added.

        Show
        Amareshwari Sriramadasu added a comment - -1 contrib tests. Test failure is due to MAPREDUCE-1834 . -1 tests included. The patch removes unused code. So, no new tests are added.
        Hide
        Ravi Gummadi added a comment -

        Patch looks good.
        +1

        Show
        Ravi Gummadi added a comment - Patch looks good. +1
        Hide
        Amareshwari Sriramadasu added a comment -

        I just committed this!

        Show
        Amareshwari Sriramadasu added a comment - I just committed this!
        Amareshwari Sriramadasu made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Konstantin Shvachko made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        5d 1h 45m 1 Amareshwari Sriramadasu 29/Jun/10 11:10
        Open Open Patch Available Patch Available
        9d 2h 56m 2 Amareshwari Sriramadasu 29/Jun/10 11:10
        Patch Available Patch Available Resolved Resolved
        2d 20h 11m 1 Amareshwari Sriramadasu 02/Jul/10 07:21
        Resolved Resolved Closed Closed
        527d 23h 58m 1 Konstantin Shvachko 12/Dec/11 06:19

          People

          • Assignee:
            Amareshwari Sriramadasu
            Reporter:
            Amareshwari Sriramadasu
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development