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