[ https://issues.apache.org/jira/browse/MAPREDUCE-2584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13048232#comment-13048232 ]
jirapos...@reviews.apache.org commented on MAPREDUCE-2584: ---------------------------------------------------------- bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, line 400 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line400> bq. > bq. > nit: the javadoc style guide says this should be: bq. > @param job the job to check for bq. > bq. > (so it formats nicely) Done. bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 401-403 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line401> bq. > bq. > I'm generally against putting empty @throws in JavaDoc - it doesn't tell you anything the method signature doesn't already say. I'd just delete these lines Agreed. It came on by default in Eclipse, and I had let them be instead of filling it up. Removed. bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 417-420 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line417> bq. > bq. > same comments apply Removed. bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 439-440 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line439> bq. > bq. > rather than this "note" why not just change the above line to "Checks if the map-output key and value classes have serializers and deserializers associated with them."? Fixed to a more direct message. Sometimes my commenting language is pretty verbose and confusing, thanks for correcting! bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, line 470 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line470> bq. > bq. > missing a space in this exception method. Added. bq. On 2011-06-12 02:31:03, Todd Lipcon wrote: bq. > src/test/mapred/org/apache/hadoop/mapreduce/TestMRJobClient.java, line 87 bq. > <https://reviews.apache.org/r/885/diff/1/?file=21002#file21002line87> bq. > bq. > add an assert that e.getMessage().contains("Couldn't find a serializer") perhaps? Added as well. - Harsh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/885/#review806 ----------------------------------------------------------- On 2011-06-11 22:16:24, Harsh Chouraria wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/885/ bq. ----------------------------------------------------------- bq. bq. (Updated 2011-06-11 22:16:24) bq. bq. bq. Review request for hadoop-mapreduce. bq. bq. bq. Summary bq. ------- bq. bq. As discussed on HADOOP-7328, MapReduce can handle serializers in a much better way in case of bad configuration, improper imports (Some odd Text class instead of the Writable Text set as key), etc.. bq. bq. This issue covers the MapReduce parts of the improvements (made to MapOutputBuffer and possible early-check of serializer availability pre-submit) that provide more information than just an NPE as is the current case. bq. bq. bq. This addresses bug MAPREDUCE-2584. bq. http://issues.apache.org/jira/browse/MAPREDUCE-2584 bq. bq. bq. Diffs bq. ----- bq. bq. src/java/org/apache/hadoop/mapred/MapTask.java 21599c2 bq. src/java/org/apache/hadoop/mapreduce/JobSubmitter.java 751d528 bq. src/test/mapred/org/apache/hadoop/mapreduce/TestMRJobClient.java 5fa329a bq. bq. Diff: https://reviews.apache.org/r/885/diff bq. bq. bq. Testing bq. ------- bq. bq. Added a test case that expects a failure if no io.serializers are present. bq. bq. bq. Thanks, bq. bq. Harsh bq. bq. > Check for serializers early, and give out more information regarding missing > serializers > ---------------------------------------------------------------------------------------- > > Key: MAPREDUCE-2584 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-2584 > Project: Hadoop Map/Reduce > Issue Type: Improvement > Components: task > Affects Versions: 0.20.2 > Reporter: Harsh J > Assignee: Harsh J > Labels: serializers, tasks > Fix For: 0.23.0 > > > As discussed on HADOOP-7328, MapReduce can handle serializers in a much > better way in case of bad configuration, improper imports (Some odd Text > class instead of the Writable Text set as key), etc.. > This issue covers the MapReduce parts of the improvements (made to IFile, > MapOutputBuffer, etc. and possible early-check of serializer availability > pre-submit) that provide more information than just an NPE as is the current > case. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira