----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24627/#review51416 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/MapOperator.java <https://reviews.apache.org/r/24627/#comment89725> why do you need a map operator at all then? Can't you just write a net new processor that doesn't init map op? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java <https://reviews.apache.org/r/24627/#comment89727> conf setup should happen in initVertexConf. ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java <https://reviews.apache.org/r/24627/#comment89728> if i read this right the only diff is the process or. can you use a var for this and keep a single call to "new Vertex"? String procClassName; if ... { procClassName = ... } ... new Vertext(...procClassName) if you move all the conf setup into the initVertexConf method this should be more clear. ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java <https://reviews.apache.org/r/24627/#comment89726> indentation seems broken ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java <https://reviews.apache.org/r/24627/#comment89729> that means you're setting a path as the alias? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MergeFileMapRecordProcessor.java <https://reviews.apache.org/r/24627/#comment89730> I'm assuming that Merge* and ORCMerge* contain a lot of copied code? (from the MR path). If that's the case can you factor that out? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezProcessor.java <https://reviews.apache.org/r/24627/#comment89731> this should be a different class. not every processor will need these things. ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java <https://reviews.apache.org/r/24627/#comment89733> don't call it jobClose if it only applies to merge work. ql/src/java/org/apache/hadoop/hive/ql/io/CombineHiveInputFormat.java <https://reviews.apache.org/r/24627/#comment89736> you seem to be fighting that merge work is only partly a map work. why not create a dummy op? that way everything is the same. you could even create a real op and move your merge logic into it. - Gunther Hagleitner On Aug. 15, 2014, 5:27 a.m., Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24627/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2014, 5:27 a.m.) > > > Review request for hive and Gunther Hagleitner. > > > Bugs: HIVE-7704 > https://issues.apache.org/jira/browse/HIVE-7704 > > > Repository: hive-git > > > Description > ------- > > Currently tez falls back to MR task for merge file task. It will beneficial > to convert the merge file tasks to tez task to make use of the performance > gains from tez. > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties b801678 > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java cd017d8 > ql/src/java/org/apache/hadoop/hive/ql/exec/MapOperator.java d5de58e > ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java a2975cb > ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 3d74459 > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1d6a93a > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecMapper.java 4e0fd79 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java e116426 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MapRecordProcessor.java > 8513e33 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MapTezProcessor.java 31f3bcd > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MergeFileMapRecordProcessor.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/MergeFileTezProcessor.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/OrcMergeFileMapRecordProcessor.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/OrcMergeFileTezProcessor.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RCFileMergeFileMapRecordProcessor.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/RecordProcessor.java 1577827 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezProcessor.java c2ba782 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java 951e918 > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/tools/RCFileMergeFileTezProcessor.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/CombineHiveInputFormat.java > bf44548 > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileInputFormat.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileMapper.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileOutputFormat.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileTask.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeFileWork.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeInputFormat.java > 4651920 > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeMapper.java beb4f7d > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeOutputFormat.java > a3ce699 > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeTask.java c437dd0 > ql/src/java/org/apache/hadoop/hive/ql/io/merge/MergeWork.java 9efee3c > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcFileMergeMapper.java > b36152a > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcFileStripeMergeInputFormat.java > a6c92fb > ql/src/java/org/apache/hadoop/hive/ql/io/orc/Writer.java c391b0e > ql/src/java/org/apache/hadoop/hive/ql/io/orc/WriterImpl.java 76b4d03 > > ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/RCFileBlockMergeInputFormat.java > 6809c79 > > ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/RCFileMergeMapper.java > dee6b1c > ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java 7129ed8 > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezProcContext.java 8513f99 > ql/src/java/org/apache/hadoop/hive/ql/parse/TezCompiler.java d58c59d > ql/src/test/queries/clientpositive/orc_merge5.q PRE-CREATION > ql/src/test/queries/clientpositive/orc_merge6.q PRE-CREATION > ql/src/test/queries/clientpositive/orc_merge7.q PRE-CREATION > ql/src/test/results/clientpositive/infer_bucket_sort_dyn_part.q.out ea37c36 > ql/src/test/results/clientpositive/list_bucket_dml_10.q.out e9367ac > ql/src/test/results/clientpositive/list_bucket_dml_4.q.out 99496d5 > ql/src/test/results/clientpositive/list_bucket_dml_6.q.out d5deadb > ql/src/test/results/clientpositive/list_bucket_dml_7.q.out 4aea4db > ql/src/test/results/clientpositive/list_bucket_dml_9.q.out f94a3cc > ql/src/test/results/clientpositive/merge_dynamic_partition4.q.out 0899648 > ql/src/test/results/clientpositive/merge_dynamic_partition5.q.out 0653469 > ql/src/test/results/clientpositive/orc_createas1.q.out a104480 > ql/src/test/results/clientpositive/orc_merge3.q.out 258f538 > ql/src/test/results/clientpositive/orc_merge5.q.out PRE-CREATION > ql/src/test/results/clientpositive/orc_merge6.q.out PRE-CREATION > ql/src/test/results/clientpositive/orc_merge7.q.out PRE-CREATION > ql/src/test/results/clientpositive/rcfile_createas1.q.out c8d65c9 > ql/src/test/results/clientpositive/rcfile_merge1.q.out ac6a2bd > ql/src/test/results/clientpositive/rcfile_merge2.q.out d8a61f3 > ql/src/test/results/clientpositive/rcfile_merge3.q.out 5c717b5 > ql/src/test/results/clientpositive/tez/orc_merge5.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/orc_merge6.q.out PRE-CREATION > ql/src/test/results/clientpositive/tez/orc_merge7.q.out PRE-CREATION > ql/src/test/results/clientpositive/union_remove_10.q.out 74b9e68 > ql/src/test/results/clientpositive/union_remove_11.q.out e0bd498 > ql/src/test/results/clientpositive/union_remove_12.q.out ada4883 > ql/src/test/results/clientpositive/union_remove_13.q.out a160843 > ql/src/test/results/clientpositive/union_remove_14.q.out cbb7ae8 > ql/src/test/results/clientpositive/union_remove_16.q.out 721caa0 > ql/src/test/results/clientpositive/union_remove_9.q.out 5a9c5a2 > > Diff: https://reviews.apache.org/r/24627/diff/ > > > Testing > ------- > > > Thanks, > > Prasanth_J > >
