Mailing-List: contact mapreduce-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mapreduce-issues@hadoop.apache.org
Date: Fri, 21 Oct 2011 13:52:33 +0000 (UTC)
From: "jiraposter@reviews.apache.org (Commented) (JIRA)" <jira@apache.org>
To: mapreduce-issues@hadoop.apache.org
Message-ID: <33934192.958.1319205153382.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <800380816.37669.1316225288868.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (MAPREDUCE-3028) Support job end notification in
 .next /0.23
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/MAPREDUCE-3028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13132676#comment-13132676 ] 

jiraposter@reviews.apache.org commented on MAPREDUCE-3028:
----------------------------------------------------------


bq.  On 2011-10-20 21:33:43, Robert Evans wrote:
bq.  > branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java, line 34
bq.  > <https://reviews.apache.org/r/2468/diff/2/?file=51833#file51833line34>
bq.  >
bq.  >     It is not that common of a practice to have your test extend what it is testing.  I don't think it is bad, I just think it would be more accepted, if you made the values you want to access package, instead of protected, or if you added in getters for those values.
bq.  
bq.  Ravi Prakash wrote:
bq.      The values I wanted to test should be private to the class since I don't want other objects to mess with them. I chose protected so that they'd be easier to test :(

That is why I suggested the getters. That way they are read only to the outside world.  Also You could add in the getters yourself in a separate sub class.  I think your tests are fine.  I was just commenting that they are different from all of the other tests, and it may get shot down because of it.


bq.  On 2011-10-20 21:33:43, Robert Evans wrote:
bq.  > branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java, line 134
bq.  > <https://reviews.apache.org/r/2468/diff/2/?file=51833#file51833line134>
bq.  >
bq.  >     Just a note that this can be done with mockito and spy instead of overriding this directly, so you would not have to subclass the original class.
bq.  
bq.  Ravi Prakash wrote:
bq.      It was just easier this way, don't you think?

It may have been easier to write but it was a bit more difficult to understand what was happening.  If you do need to subclass it I would prefer to see it happen as a separate sub class from the test, but that is just me. 


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2468/#review2727
-----------------------------------------------------------


On 2011-10-20 23:35:05, Ravi Prakash wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2468/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-20 23:35:05)
bq.  
bq.  
bq.  Review request for Tom Graves, Robert Evans, Jonathan Eagles, and Mark Holderbaugh.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Job end notification for 0.23 / next. I'm going to work on unit tests while you folks review the code.
bq.  
bq.  The only new thing added to 0.20 is that multiple recipients can be configured to receive the job-end notification by supplying multiple URLs (one parameter separated by a regex)
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java PRE-CREATION 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/MRAppMaster.java 1186838 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java PRE-CREATION 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobConf.java 1186838 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java 1186838 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java 1186838 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/util/ConfigUtil.java 1186838 
bq.    branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/resources/mapred-default.xml 1186838 
bq.  
bq.  Diff: https://reviews.apache.org/r/2468/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Manually tested changes on a single node cluster. Going to add unit tests.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ravi
bq.  
bq.


> Support job end notification in .next /0.23
> -------------------------------------------
>
>                 Key: MAPREDUCE-3028
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-3028
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: mrv2
>    Affects Versions: 0.23.0
>            Reporter: Mohammad Kamrul Islam
>            Assignee: Ravi Prakash
>            Priority: Blocker
>             Fix For: 0.23.0
>
>         Attachments: MAPREDUCE-3028.branch-0.23.patch, MAPREDUCE-3028.patch, MAPREDUCE-3028.patch
>
>
> Oozie primarily depends on  the job end notification to determine when the job finishes. In the current version,  job end notification is implemented in job tracker. Since job tracker will be removed in the upcoming hadoop release (.next), we wander where this support will move. I think this best effort notification could be implemented in the new Application Manager as one of the last step of job completion.
> Whatever implementation will it be, Oozie badly needs this feature to be continued in next releases as well.
>  

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira