----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14964/#review27579 -----------------------------------------------------------
Thank you Aniket for the patch! Overall it looks good. Some lines become quite wide. Do you mind breaking them into multiple lines? I have few more minor comments below. trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/InputSizeReducerEstimator.java <https://reviews.apache.org/r/14964/#comment53521> Unused. Please remove. trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java <https://reviews.apache.org/r/14964/#comment53518> White space. trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java <https://reviews.apache.org/r/14964/#comment53524> I think we should not duplicate this catch block. Perhaps we can move it to the outside of the for loop and catch all the exceptions in one place. For example, try { long runningSum = 0; long maxSize = configured value; for (each of replFiles) { runningSum += size of replFile; if (runningSum > maxSize) { throw new IOException(); } } setupDistributedCache(); } catch (IOException e) { throw new VisitorException(e); } trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java <https://reviews.apache.org/r/14964/#comment53517> Please remove the tab. trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java <https://reviews.apache.org/r/14964/#comment53523> Why remove "Internal error"? I think keeping it is consistent with other error messages in this file. trunk/src/org/apache/pig/backend/hadoop/executionengine/util/MapRedUtil.java <https://reviews.apache.org/r/14964/#comment53520> Typo: "if a file" => "if a directory". trunk/src/org/apache/pig/impl/util/Utils.java <https://reviews.apache.org/r/14964/#comment53516> Unused. Please remove. - Cheolsoo Park On Oct. 26, 2013, 1:29 a.m., Aniket Mokashi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14964/ > ----------------------------------------------------------- > > (Updated Oct. 26, 2013, 1:29 a.m.) > > > Review request for pig, Cheolsoo Park, Daniel Dai, Dmitriy Ryaboy, and Julien > Le Dem. > > > Bugs: PIG-3047 > https://issues.apache.org/jira/browse/PIG-3047 > > > Repository: pig > > > Description > ------- > > -Check the size of a relation before adding it to distributed cache in > Replicated join - 1G by default > > > Diffs > ----- > > trunk/conf/pig.properties 1531787 > trunk/src/org/apache/pig/PigConfiguration.java 1531787 > > trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/InputSizeReducerEstimator.java > 1531787 > > trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java > 1531787 > > trunk/src/org/apache/pig/backend/hadoop/executionengine/util/MapRedUtil.java > 1531787 > trunk/src/org/apache/pig/impl/util/Utils.java 1531787 > trunk/test/org/apache/pig/test/PigStorageWithStatistics.java 1531787 > trunk/test/org/apache/pig/test/TestFRJoin2.java 1531787 > > Diff: https://reviews.apache.org/r/14964/diff/ > > > Testing > ------- > > > Thanks, > > Aniket Mokashi > >
