Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Pig, mail # dev - Review Request 14964: PIG-3047 Check the size of a relation before adding it to distributed cache in Replicated join


Copy link to this message
-
Re: Review Request 14964: PIG-3047 Check the size of a relation before adding it to distributed cache in Replicated join
Cheolsoo Park 2013-10-27, 01:28

-----------------------------------------------------------
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
>
>