You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Rohini Palaniswamy <ro...@gmail.com> on 2016/12/02 23:44:53 UTC
Re: Review Request 54056: Drop Hadoop 1.x support in Pig 0.17
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54056/#review157618
-----------------------------------------------------------
Comments:
1) Can you change hadoopversion 23 to 2 everywhere. Would be a good time to cleanup 23 which is now a obsolete version out of the code. Will also be cleaner to use 2 and 3 when hadoop 3 comes in. There will be many build scripts out there using hadoopversion=23. So do add a condition in the beginning to change 23 to 2 for hadoopversion if specified in build.xml
2) Fine with the classes under shims that you have moved out. But would be good to keep the src.shims.dir structure and just the HadoopShims class instead of moving the methods to MapredUtil or FSUtil. We might need it for Hadoop 3 and would be easier then not having to recreate the shims folder structure again. Also there will lesser changes with this patch without MapredUtil/FSUtil changes. The HadoopShims changes where you have removed simpler methods and inlinined them in classes is good.
3) Try to get rid of reflections in code as much as possible which were done for Hadoop 1.x or Hbase 94. For eg: UserGroupInformation methods in hadoop, TableMapReduceUtil in hbase are called via reflection in HBaseStorage
BUILDING.md (line 23)
<https://reviews.apache.org/r/54056/#comment228389>
Remove -Dhadoopversion=23
bin/pig (lines 401 - 402)
<https://reviews.apache.org/r/54056/#comment228427>
You need this else block to run with bundled hadoop jars instead of hadoop installation. It needs to be changed to run with hadoop 2.x. Currently there is only lib/hadoop1-runtime in pig installaion. Will have to add lib/hadoop2-runtime with hadoop 2 jars for it to work. We should also upgrade bundled hadoop from 2.6.0 to 2.7.3 which is the latest one.
build.xml (line 211)
<https://reviews.apache.org/r/54056/#comment228391>
Can this changed to be 1 (with 95 if specified being changed to 1 similar to 23->2)
src/docs/src/documentation/content/xdocs/start.xml (line 37)
<https://reviews.apache.org/r/54056/#comment228428>
Remove 0.23.X
src/org/apache/pig/backend/hadoop/datastorage/FSUtil.java (line 41)
<https://reviews.apache.org/r/54056/#comment228429>
Can keep this in HadoopShims itself. Few changes to method can be done now it is only Hadoop 2
- Remove block with check for Hadoop 0.23
- Remove the reflection and access the method directly
src/org/apache/pig/builtin/HiveUDFBase.java (line 185)
<https://reviews.apache.org/r/54056/#comment228467>
Remove the reflection and access class directly. Same in OrcStorage
src/org/apache/pig/builtin/TextLoader.java
<https://reviews.apache.org/r/54056/#comment228468>
Only && !HadoopShims.isHadoopYARN() should be removed from this condition and not the full condition
src/org/apache/pig/impl/util/JarManager.java (line 68)
<https://reviews.apache.org/r/54056/#comment228469>
Remove GUAVA here as it need not be shipped any more
src/org/apache/pig/impl/util/JarManager.java (lines 210 - 212)
<https://reviews.apache.org/r/54056/#comment228470>
Remove whole block
test/org/apache/pig/test/Util.java (line 650)
<https://reviews.apache.org/r/54056/#comment228471>
Can you rename method to getFSMkDirCommand()
- Rohini Palaniswamy
On Nov. 24, 2016, 9:54 a.m., Adam Szita wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54056/
> -----------------------------------------------------------
>
> (Updated Nov. 24, 2016, 9:54 a.m.)
>
>
> Review request for pig, Daniel Dai and Rohini Palaniswamy.
>
>
> Bugs: PIG-4923
> https://issues.apache.org/jira/browse/PIG-4923
>
>
> Repository: pig-git
>
>
> Description
> -------
>
> Drop Hadoop 1.x support in Pig 0.17
>
>
> Diffs
> -----
>
> BUILDING.md ccee21cc50cb8fa1ceaf70e66fe2feaedf4b2263
> bin/pig 81f142681101024761025822aeba9fae23f0a975
> bin/pig.py 5ff1d37f8e3d2410c2dacfe180ecd77407c275e3
> build.xml 49f887b306835852168529fe9d3a721f804c8b8c
> contrib/piggybank/java/build.xml a647e71932a9a066574751b7a7167f0be033d1c6
> contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/IndexedStorage.java 47c6105eb407c2260523f53a5e73a4ae18698259
> contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestIndexedStorage.java 46385e980bdc41307f5e156109742bf557273676
> contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestPathPartitionHelper.java e914d101090c6063cecda5329a538273965638c2
> ivy.xml bae1a8180d37ce86ff01cfccd6864e277f64f90b
> ivy/libraries.properties 3a819a510fe87721bc086129a4aa10e3f36e31e3
> shims/src/hadoop20/org/apache/pig/backend/hadoop/PigATSClient.java 07c5f4989d7113b00d3472b7295e5729e8b775e1
> shims/src/hadoop20/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapBase.java 8bdfa2cea5575cac9f262dafbfa997c897e92e89
> shims/src/hadoop20/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapReduce.java 50d3b1b50db86ce871db50ce09a735d1a52b7889
> shims/src/hadoop20/org/apache/pig/backend/hadoop/executionengine/shims/HadoopShims.java 515ce660a1c738984ae63bca3ccfd3dd162a2c71
> shims/src/hadoop20/org/apache/pig/backend/hadoop20/PigJobControl.java 07c984101ccad1df97f9afa5896f29dfedc2150f
> shims/src/hadoop23/org/apache/hadoop/mapred/DowngradeHelper.java b3a17722c5417c894e298d5693b4616ecee4df39
> shims/src/hadoop23/org/apache/pig/backend/hadoop/PigATSClient.java e3c6090370cc6e1d6c54b8b82b03d17dd4ed1153
> shims/src/hadoop23/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapBase.java 9538149ab6b51bec812b09fc62fe519b60c9dc91
> shims/src/hadoop23/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapReduce.java 783ae552f070ca79bc550d1f03dae3c183757315
> shims/src/hadoop23/org/apache/pig/backend/hadoop/executionengine/shims/HadoopShims.java 8fbf33fc793986f9b165a61db30baae1be18e0f2
> shims/src/hadoop23/org/apache/pig/backend/hadoop23/PigJobControl.java 6439611ae1b983266a9832bd54e653862bdc9564
> shims/test/hadoop20/org/apache/pig/test/MiniCluster.java 2ceeaad53a15bedd2c8d53452c349fb1fe9cbc9d
> shims/test/hadoop20/org/apache/pig/test/TezMiniCluster.java 505c4727caaf849ff20e301222fa67a20655768d
> shims/test/hadoop23/org/apache/pig/test/MiniCluster.java 17bb8664f5c575ef5237f9ab39748c2f0a7e3e06
> shims/test/hadoop23/org/apache/pig/test/TezMiniCluster.java 792a1bd4e60bdfbc9d22bd7eb24c3cedb215068e
> src/docs/src/documentation/content/xdocs/start.xml 8de0933c41736909a273ec782c96e87798df9701
> src/docs/src/documentation/content/xdocs/tabs.xml f2f34eba132f92b48bc88e5ac80005aa7aeea825
> src/org/apache/pig/backend/hadoop/PigATSClient.java PRE-CREATION
> src/org/apache/pig/backend/hadoop/PigJobControl.java PRE-CREATION
> src/org/apache/pig/backend/hadoop/datastorage/ConfigurationUtil.java 26fc5d59616f1a2d418525b08e2823295dae37f7
> src/org/apache/pig/backend/hadoop/datastorage/FSUtil.java PRE-CREATION
> src/org/apache/pig/backend/hadoop/executionengine/Launcher.java cc85991bd24bae02b5499d74b726ea83b50a9044
> src/org/apache/pig/backend/hadoop/executionengine/fetch/FetchLauncher.java 019fc87c771e7a13eedb6374d2b09431056ac3d1
> src/org/apache/pig/backend/hadoop/executionengine/fetch/FetchPOStoreImpl.java 2a57435e0724c30c7b3f18b31ca6e2506e45d558
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java 4966f57481d03a38e7adb5021890e7517ac13b35
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRCompiler.java 193905ffec003aaf0329dc0733a020ec5fa83174
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReduceLauncher.java 632806fbdb517b87b87971833cffbf2a6d0fcf67
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReducePOStoreImpl.java 9acc97670551751f062aca201ce7cbdb973f9962
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigInputFormat.java e6a3d65c7f603e718310beec8e71dcd2cd01fdf2
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapBase.java PRE-CREATION
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigMapReduce.java PRE-CREATION
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigOutputCommitter.java 83f3fbbef1f538bffd0a9439cdb7bcaf5ffe5f45
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigOutputFormat.java 1acbd362b99b882fecb16322b6fc24650be6f1b0
> src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java fb401b8afadc175e4ff8b90f81714b2c3c074f23
> src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigProcessor.java 9d2ffb3497a89fec74987534cf97b31839d0bdae
> src/org/apache/pig/backend/hadoop/executionengine/util/MapRedUtil.java b485c563e10650d19c912f7214bf4d4cd899da79
> src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 13ab055e37a084a168ee7a29009e13e34d878134
> src/org/apache/pig/builtin/HiveUDFBase.java 1c87fdb124cdbe105d2116bb6d1a6d730feaa79e
> src/org/apache/pig/builtin/OrcStorage.java 5e05dd2f8709a6414b04f9cb459eaca435bed762
> src/org/apache/pig/builtin/PigStorage.java 730946f779569d45564c414273fc38b2b930e753
> src/org/apache/pig/builtin/TextLoader.java 8083b6391eef2398555a2bc2f86ae4ccf26f8ed7
> src/org/apache/pig/impl/io/PigFile.java d46e10b401322c3abf81563825b54b8561adf3ff
> src/org/apache/pig/impl/io/ReadToEndLoader.java 21c42274a02fb055d13951f22d11115cb100d2b7
> src/org/apache/pig/impl/util/JarManager.java d62923497fd085becd23e98a1631456a7eeac58b
> src/org/apache/pig/impl/util/UriUtil.java 0d79490a246bfa4a6ead91224999b99cfa18243c
> src/org/apache/pig/impl/util/Utils.java 0f037df9779964239fe4507b69d3c9842be0eb32
> src/org/apache/pig/parser/QueryParserUtils.java 908a7b1f4f0daa8797784b35373db013678363f7
> src/org/apache/pig/parser/RegisterResolver.java 8e06bd18aa1840025fa7d4e81974c60ddf4522b7
> src/org/apache/pig/tools/pigstats/PigStatsUtil.java 542cc2ebc0f40c4114566af46b28ae78c7a0efb1
> src/org/apache/pig/tools/pigstats/mapreduce/MRJobStats.java c8b9128200389882048ad5a05f970ec549a2de8a
> src/org/apache/pig/tools/pigstats/mapreduce/MRPigStatsUtil.java 4b5927511d1c0c155a6bbaf9cc5eed350ab0f9a3
> test/e2e/pig/build.xml 2c197adcb0ba12ff611981b1e4e8511c2b95d303
> test/excluded-tests-20 f453aa84c2d8af551da425478124b8f9e8f99fab
> test/org/apache/pig/TestLoadStoreFuncLifeCycle.java f944476c501ed8e7868808983015c87649475abb
> test/org/apache/pig/data/TestSchemaTuple.java 14117482fc5326d3724c92a4e403e774b83100de
> test/org/apache/pig/parser/TestQueryParserUtils.java a6fb3919d28cc2df98a86c8d3d5a1beabf7b6063
> test/org/apache/pig/test/MiniCluster.java PRE-CREATION
> test/org/apache/pig/test/TestBZip.java 5a51d206a58418ae5a55c1949a4f08e1557fca23
> test/org/apache/pig/test/TestJobControlCompiler.java 37d6542c7335f2fd3e8111e558da9f535e99b485
> test/org/apache/pig/test/TestLoaderStorerShipCacheFiles.java 5e273ab5dda75f8dd92f3d40cb2582e21c5972d5
> test/org/apache/pig/test/TestPigRunner.java 13ed4688c3fa26e024f16fa40bcc68fc97a83340
> test/org/apache/pig/test/TestTmpFileCompression.java 29ae03173352ddbb0b4af11905bf0fd92f7a9dd8
> test/org/apache/pig/test/TezMiniCluster.java PRE-CREATION
> test/org/apache/pig/test/Util.java 67af8e232210e977b4b999cd2de851425c1aebd2
> test/perf/pigmix/bin/generate_data.sh cc216d6f2e9a78975a77e85e9abffe5786bdd9e8
> test/perf/pigmix/build.xml 8fe2d1550c8f90e6402152eb1148d1957db6485c
>
> Diff: https://reviews.apache.org/r/54056/diff/
>
>
> Testing
> -------
>
> Ran unit tests, made sure e2e and pigmix tests run
>
>
> Thanks,
>
> Adam Szita
>
>