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

Enhance -list-blacklisted-trackers to display host name, blacklisted reason and blacklist report.

    Details

    • Type: Improvement Improvement
    • 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
    • Release Note:
      Hide
      Enhanced -list-blacklisted-trackers to include the reason for blacklisting a node. Modified JobSubmissionProtocol's version as the ClusterStatus is changed to have a new class. The format of the -list-blacklisted-trackers command line interface has also changed to show the reason.
      Show
      Enhanced -list-blacklisted-trackers to include the reason for blacklisting a node. Modified JobSubmissionProtocol's version as the ClusterStatus is changed to have a new class. The format of the -list-blacklisted-trackers command line interface has also changed to show the reason.

      Description

      Currently, the -list-blacklisted-trackers in the mapred job option list only tracker name. We should enhance it to display as hostname, reason for blacklisting and blacklist report.

      1. mapreduce-766-ydist.patch
        15 kB
        Sreekanth Ramakrishnan
      2. mapreduce-766-6.patch
        20 kB
        Sreekanth Ramakrishnan
      3. mapreduce-766-5.patch
        20 kB
        Sreekanth Ramakrishnan
      4. blacklist3.png
        19 kB
        Sreekanth Ramakrishnan
      5. mapreduce-766-4.patch
        18 kB
        Sreekanth Ramakrishnan
      6. mapreduce-766-3.patch
        16 kB
        Sreekanth Ramakrishnan
      7. mapreduce-766-2.patch
        13 kB
        Sreekanth Ramakrishnan
      8. mapreduce-766-1.patch
        13 kB
        Sreekanth Ramakrishnan

        Activity

        Sreekanth Ramakrishnan created issue -
        Sreekanth Ramakrishnan made changes -
        Field Original Value New Value
        Assignee Sreekanth Ramakrishnan [ sreekanth ]
        Hide
        Sreekanth Ramakrishnan added a comment -

        Attaching a patch which displays blacklist information for the list of all the blacklisted trackers.

        Show
        Sreekanth Ramakrishnan added a comment - Attaching a patch which displays blacklist information for the list of all the blacklisted trackers.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-1.patch [ 12413628 ]
        Hide
        rahul k singh added a comment -

        Changes look good to me
        A few very minor changes:

        1.Change comment in JobSubmissionProtocol "encapsulates blacklist reasons and report"
        to "encapsulates reasons and report for blacklisted node."

        2.ClusterStatus
        In getHostName method change comment @return "tracker name" should be "hostname"
        toString() method use StringBuilder instead of StringBuffer.
        In method setHostName in comment @param need to be changed frmo trackerName to hostName.

        3.JobClient.
        In listBlackListedTrackers.
        can we change header like below.
        BlackListedNode Reason Report

        4.TestTaskTrackerBlackListing.
        Add comments to testcase.

        Show
        rahul k singh added a comment - Changes look good to me A few very minor changes: 1.Change comment in JobSubmissionProtocol "encapsulates blacklist reasons and report" to "encapsulates reasons and report for blacklisted node." 2.ClusterStatus In getHostName method change comment @return "tracker name" should be "hostname" toString() method use StringBuilder instead of StringBuffer. In method setHostName in comment @param need to be changed frmo trackerName to hostName. 3.JobClient. In listBlackListedTrackers. can we change header like below. BlackListedNode Reason Report 4.TestTaskTrackerBlackListing. Add comments to testcase.
        Hide
        Sreekanth Ramakrishnan added a comment -

        Attaching patch incorporating Rahul's comments.

        Show
        Sreekanth Ramakrishnan added a comment - Attaching patch incorporating Rahul's comments.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-2.patch [ 12413672 ]
        Hide
        rahul k singh added a comment -

        Looking good:
        minor comments :

        1.In ClusterStatus.

        1.BlackListedInfo is a package private class make it public.

        2.all the member variables need to private.

        3."public Collection<String> getBlacklistedTrackerNames" need to be changed , it currently passes hostname , it should be
        passing the tracker names.

        ClusterStatus constructor "ClusterStatus(
        Collection<String> activeTrackers,
        Collection<BlackListInfo> blacklistedTrackers, long ttExpiryInterval,
        int maps, int reduces, int maxMaps, int maxReduces,
        JobTracker.State state, int numDecommissionNodes) "

        is crossing 80 chars

        Show
        rahul k singh added a comment - Looking good: minor comments : 1.In ClusterStatus. 1.BlackListedInfo is a package private class make it public. 2.all the member variables need to private. 3."public Collection<String> getBlacklistedTrackerNames" need to be changed , it currently passes hostname , it should be passing the tracker names. ClusterStatus constructor "ClusterStatus( Collection<String> activeTrackers, Collection<BlackListInfo> blacklistedTrackers, long ttExpiryInterval, int maps, int reduces, int maxMaps, int maxReduces, JobTracker.State state, int numDecommissionNodes) " is crossing 80 chars
        Hide
        Sreekanth Ramakrishnan added a comment -

        Incorporated Rahuls comments.

        Show
        Sreekanth Ramakrishnan added a comment - Incorporated Rahuls comments.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-3.patch [ 12413786 ]
        Hide
        rahul k singh added a comment -

        changes look fine to me .

        +1.

        Show
        rahul k singh added a comment - changes look fine to me . +1.
        Sreekanth Ramakrishnan 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/12413786/mapreduce-766-3.patch
        against trunk revision 795489.

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +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-vesta.apache.org/413/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/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/12413786/mapreduce-766-3.patch against trunk revision 795489. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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-vesta.apache.org/413/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-vesta.apache.org/413/console This message is automatically generated.
        Hide
        Sreekanth Ramakrishnan added a comment -

        The streaming test case failure was not introduced due the the patch.

        Show
        Sreekanth Ramakrishnan added a comment - The streaming test case failure was not introduced due the the patch.
        Hide
        Hemanth Yamijala added a comment -

        Looking good. A few minor points:

        • We are changing the way we print information about blacklisted trackers. Does this need to be backwards compatible ? Similarly, maybe we need not deprecate getBlacklistedTrackerNames()
        • The print format should be parse friendly. We are adding white space to separate reasons for blacklisting and the report can have newlines. These will make it difficult to parse. Can we make these comma separated ?
        • getBlackListedTracker should be getBlackListedTrackers. Similarly getBlackListedTrackerInfo() should be getBlackListedTrackersInfo() - the former seems like it is for a specific tracker.
        • The test class is not applying, possibly due to the commit of MAPREDUCE-682. Also, can you enhance the test case to test for a condition when no nodes are blacklisted, and when there is more than one reason to blacklist ? Maybe one of the trackers can be blacklisted for more than one reason.
        Show
        Hemanth Yamijala added a comment - Looking good. A few minor points: We are changing the way we print information about blacklisted trackers. Does this need to be backwards compatible ? Similarly, maybe we need not deprecate getBlacklistedTrackerNames() The print format should be parse friendly. We are adding white space to separate reasons for blacklisting and the report can have newlines. These will make it difficult to parse. Can we make these comma separated ? getBlackListedTracker should be getBlackListedTrackers. Similarly getBlackListedTrackerInfo() should be getBlackListedTrackersInfo() - the former seems like it is for a specific tracker. The test class is not applying, possibly due to the commit of MAPREDUCE-682 . Also, can you enhance the test case to test for a condition when no nodes are blacklisted, and when there is more than one reason to blacklist ? Maybe one of the trackers can be blacklisted for more than one reason.
        Hide
        Sreekanth Ramakrishnan added a comment -

        Attaching a patch incorporating Hemanths comments.

        • Changed, the separator between reasons for blacklisting to "," and stripping off all "\n" in the black list report and changing it to ":" for better parsability.
        • Added test case to check the behavior if no tracker is blacklisted and if a tracker is blacklisted for multiple reasons.
        Show
        Sreekanth Ramakrishnan added a comment - Attaching a patch incorporating Hemanths comments. Changed, the separator between reasons for blacklisting to "," and stripping off all "\n" in the black list report and changing it to ":" for better parsability. Added test case to check the behavior if no tracker is blacklisted and if a tracker is blacklisted for multiple reasons.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-4.patch [ 12414306 ]
        Hide
        Sreekanth Ramakrishnan added a comment -

        Screen shot for the cli.

        Show
        Sreekanth Ramakrishnan added a comment - Screen shot for the cli.
        Sreekanth Ramakrishnan made changes -
        Attachment blacklist3.png [ 12414324 ]
        Hide
        Sreekanth Ramakrishnan added a comment -

        Attaching latest patch with changes from Hemanth offline.

        ClusterStatus.java : Modified documentation, removed a constructor, made the set methods package private.
        JobTracker.java: Added a check for sb.length() > 0 before replacing the final ',', also renamed an API to getFaultReport - replaced occurrences including jsp and test case.
        Fixed the test case to check if the correct replaced string is being used in the BlacklistInfo instance.

        Show
        Sreekanth Ramakrishnan added a comment - Attaching latest patch with changes from Hemanth offline. ClusterStatus.java : Modified documentation, removed a constructor, made the set methods package private. JobTracker.java: Added a check for sb.length() > 0 before replacing the final ',', also renamed an API to getFaultReport - replaced occurrences including jsp and test case. Fixed the test case to check if the correct replaced string is being used in the BlacklistInfo instance.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-5.patch [ 12414739 ]
        Hide
        Hemanth Yamijala added a comment -

        +1 except for a minor nit. A System.out.println in the test case should be removed. Please run it through hudson or equivalent locally.

        Show
        Hemanth Yamijala added a comment - +1 except for a minor nit. A System.out.println in the test case should be removed. Please run it through hudson or equivalent locally.
        Hide
        Sreekanth Ramakrishnan added a comment -

        Attaching patch removing the System.out.println() statement.

        Running test patch and tests.

        Show
        Sreekanth Ramakrishnan added a comment - Attaching patch removing the System.out.println() statement. Running test patch and tests.
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-6.patch [ 12415004 ]
        Sreekanth Ramakrishnan made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Sreekanth Ramakrishnan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Sreekanth Ramakrishnan added a comment -

        output from ant test-patch

             [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 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        Show
        Sreekanth Ramakrishnan added a comment - output from ant test-patch [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 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Sreekanth Ramakrishnan added a comment -

        All tests passed locally, both core and contrib.

        Show
        Sreekanth Ramakrishnan added a comment - All tests passed locally, both core and contrib.
        Hide
        Hemanth Yamijala added a comment -

        I just committed this. Thanks, Sreekanth !

        Show
        Hemanth Yamijala added a comment - I just committed this. Thanks, Sreekanth !
        Hemanth Yamijala made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Incompatible change, Reviewed]
        Release Note Enhanced -list-blacklisted-trackers to include the reason for blacklisting a node. Modified JobSubmissionProtocol's version as the ClusterStatus is changed to have a new class. The format of the -list-blacklisted-trackers command line interface has also changed to show the reason.
        Fix Version/s 0.21.0 [ 12314045 ]
        Resolution Fixed [ 1 ]
        Hide
        Sreekanth Ramakrishnan added a comment -

        patch for Yahoo! distribution

        Show
        Sreekanth Ramakrishnan added a comment - patch for Yahoo! distribution
        Sreekanth Ramakrishnan made changes -
        Attachment mapreduce-766-ydist.patch [ 12415112 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #35 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/35/)

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #35 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/35/ )
        Tom White 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
        10d 4h 53m 1 Sreekanth Ramakrishnan 30/Jul/09 09:34
        Open Open Patch Available Patch Available
        4d 1h 2m 2 Sreekanth Ramakrishnan 30/Jul/09 09:36
        Patch Available Patch Available Resolved Resolved
        1d 41m 1 Hemanth Yamijala 31/Jul/09 10:17
        Resolved Resolved Closed Closed
        389d 11h 57m 1 Tom White 24/Aug/10 22:14

          People

          • Assignee:
            Sreekanth Ramakrishnan
            Reporter:
            Sreekanth Ramakrishnan
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development