pig-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rohini Palaniswamy <rohini.adi...@gmail.com>
Subject Re: Review Request 65239: PIG-5253: Pig Hadoop 3 support
Date Tue, 06 Feb 2018 15:26:26 GMT


> On Jan. 26, 2018, 4:58 p.m., Rohini Palaniswamy wrote:
> > test/org/apache/pig/parser/TestQueryParserUtils.java
> > Line 63 (original), 63 (patched)
> > <https://reviews.apache.org/r/65239/diff/1/?file=1942594#file1942594line63>
> >
> >     What is swebhdfs? secure webhdfs?
> 
> Nandor Kollar wrote:
>     Yes, it is secure webhdfs (https://hadoop.apache.org/docs/stable/api/org/apache/hadoop/fs/SWebHdfs.html).
Not sure why I changed to secure. :) Should we use non-secure instead? And is it fine to test
webhdfs instead of hftp here for Hadoop 2 too?

webhdfs is already there in the same test further down. If you could just move the swebdhfs
right after webhdfs that would be nice.


> On Jan. 26, 2018, 4:58 p.m., Rohini Palaniswamy wrote:
> > test/org/apache/pig/test/MiniCluster.java
> > Line 107 (original), 112-114 (patched)
> > <https://reviews.apache.org/r/65239/diff/1/?file=1942595#file1942595line112>
> >
> >     Refer to YarnMiniCluster for what is written into each of the files.
> >     
> >     Actually this whole class can be deleted and YarnMiniCluster used instead for
EXECTYPE_MR. There might be one or two settings missed which will have to be copied to YarnMiniCluster.
If that works without any issues, would prefer that.
> 
> Nandor Kollar wrote:
>     How about having a new class (or rename MiniCluster) called MapReduceMiniCluster,
which extends YarnMiniCluster, but with a very minimal code in the setup method? This way
if any MR specific property has to be set, then only that minicluster class is changed, and
it won't have any effect on Tez and Spark exec types.

+1 for the idea. Renaming MiniCluster to MapReduceMiniCluster and making it extend YarnMiniCluster
sounds better.


- Rohini


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


On Jan. 19, 2018, 4:14 p.m., Nandor Kollar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65239/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 4:14 p.m.)
> 
> 
> Review request for pig, Daniel Dai, Koji Noguchi, Rohini Palaniswamy, and Adam Szita.
> 
> 
> Repository: pig-git
> 
> 
> Description
> -------
> 
> This is an initial patch that adds Hadoop 3 support to Pig in addition to Hadoop 2.
> 
> Major modifications:
>  * No breaking API change was introduced in Hadoop 3, the current code compiles with
Hadoop 3
>  * hadoopversion property tells which mode the tests should run, the default is hadoop
2
>  * Hadoop 3 introduced a security fix, only whitelisted environment variables are passed
to MiniCluster
>  * In Hadoop 3 hadoop-site.xml is deprecated, and is replaced by core-site.xml, hdfs-site.xml
and mapred-site.xml. I decided to write the config into all of these files in MiniCluster.java
(into hadoop-site.xml too to stay compatible with Hadoop 2) for the shake of simplicity, we
might want to have different files for Hadoop 2 and separate the properties for Hadoop 3.
>  * TestErrorHandling.java: small format change in error message, modified the assert
so it works on both on Hadoop 2 and Hadoop 3
>  * HadoopShims: code is identical with Hadoop 2, not sure if we need shims any more.
I think we should move it to the src instead.
>  * Split properties into 3 files: common properties, Hadoop 2 and Hadoop 3 specific properties
>  * ivy.xml: new config for Hadoop 3
>  * build.xml: new target to package both hadoop2 and hadoop3 - not sure that this is
needed, if we move shims, the I think we don't need this target
>  * HBase unit test fails on Hadoop 3 (as per https://hbase.apache.org/book.html HBase
1.x is not tested against Hadoop 2)
> 
> 
> Diffs
> -----
> 
>   bin/pig 3fcf165106cccbe75fc1c61ea74732456ae50fc7 
>   bin/pig.py b6c396579c54359f430c6e74d055ec7f27ae2197 
>   build.xml 8bcbe5e4d60b793412dd5490518928b17308da3b 
>   ivy.xml 3ac675190f15528674361eda924af61fc1d07613 
>   ivy/libraries-common.properties PRE-CREATION 
>   ivy/libraries-h2.properties PRE-CREATION 
>   ivy/libraries-h3.properties PRE-CREATION 
>   ivy/libraries.properties 800b75edea300d6ff4d0a55481a1b3ed5e3be6ea 
>   shims/src/hadoop3/org/apache/pig/backend/hadoop/executionengine/shims/HadoopShims.java
PRE-CREATION 
>   shims/test/hadoop3/.gitignore PRE-CREATION 
>   test/e2e/pig/build.xml 384639dfeddaa31ab7b2891ee1ed3602e1f0c08d 
>   test/org/apache/pig/parser/TestErrorHandling.java 15e09031c360cea5f81609129ac3a6d38d68d3ea

>   test/org/apache/pig/parser/TestQueryParserUtils.java 1c217e3cab9c4b5dc51289a883aa696dcd2feeea

>   test/org/apache/pig/test/MiniCluster.java a7532ad750f06ffae5a03024b1658ff77152c902

>   test/org/apache/pig/test/Util.java 788a72fe3ceca08ec61ae425a393b5b0936454f4 
> 
> 
> Diff: https://reviews.apache.org/r/65239/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nandor Kollar
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message