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

Switch to Threaded View
Pig, mail # dev - Review Request 14274: PIG-2672 Optimize the use of DistributedCache


Copy link to this message
-
Re: Review Request 14274: PIG-2672 Optimize the use of DistributedCache
Cheolsoo Park 2013-09-24, 06:30

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

trunk/src/org/apache/pig/PigConstants.java
<https://reviews.apache.org/r/14274/#comment51440>

    Would you mind replacing all the "pig.temp.dir" strings in the code base? I see 4 more matches besides what you're substituting in this patch.
    
    TestAvroStorage.java
    197: pigServerLocal.getPigContext().getProperties().setProperty("pig.temp.dir", TMP_DIR);
    
    PigConstants.java
    53: public static final String PIG_TEMP_DIR = "pig.temp.dir";
    
    TestMultiQueryLocal.java
    73: myPig.getPigContext().getProperties().setProperty("pig.temp.dir", "build/test/tmp/");
    
    TestPigServer.java
    731: out.println("pig.temp.dir=/opt/temp");

trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51434>

    Perhaps  use this utility function instead:
    http://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FilenameUtils.html#getName(java.lang.String)

trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51431>

    Why not return immediately when jarPath is found and remove the boolean flag? i.e.
    
    for (FileStatus stat : statuses) {
      jarPath = stat.getPath();
      if(jarPath.getName().equals(filename)) {
        return jarPath;
      }
    }
    
    ...

trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51433>

    Can you move this line to after the if block, or replace the pathOnDfs?

trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51432>

    Perhaps use this utility function instead:
    http://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FilenameUtils.html#getName(java.lang.String)

trunk/src/org/apache/pig/impl/PigContext.java
<https://reviews.apache.org/r/14274/#comment51436>

    Looks like we can simply use Set<String> instead of Map<Sting, URL> for extraJarOriginalPaths, no?

trunk/test/org/apache/pig/test/TestJobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51437>

    Delete?

trunk/test/org/apache/pig/test/TestJobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51438>

    Not used. Delete?

trunk/test/org/apache/pig/test/TestJobControlCompiler.java
<https://reviews.apache.org/r/14274/#comment51439>

    The following line is missing in the RB diff but it's in the attached the patch:
    
    properties.setProperty(PigConstants.PIG_SHARED_CACHE_ENABLED_KEY, "true");
    
    Just pointing it out.
- Cheolsoo Park
On Sept. 21, 2013, 1:21 a.m., Aniket Mokashi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14274/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2013, 1:21 a.m.)
>
>
> Review request for pig, Cheolsoo Park, DanielWX DanielWX, Dmitriy Ryaboy, Julien Le Dem, and Rohini Palaniswamy.
>
>
> Bugs: PIG-2672
>     https://issues.apache.org/jira/browse/PIG-2672
>
>
> Repository: pig
>
>
> Description
> -------
>
> added jar.cache.location option
>
>
> Diffs
> -----
>
>   trunk/src/org/apache/pig/PigConstants.java 1525188
>   trunk/src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java 1525188
>   trunk/src/org/apache/pig/impl/PigContext.java 1525188
>   trunk/src/org/apache/pig/impl/io/FileLocalizer.java 1525188
>   trunk/test/org/apache/pig/test/TestJobControlCompiler.java 1525188
>
> Diff: https://reviews.apache.org/r/14274/diff/