----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65422/#review196950 -----------------------------------------------------------
First pass review. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 3689 (patched) <https://reviews.apache.org/r/65422/#comment276939> Default should be reoptimize. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 3691 (patched) <https://reviews.apache.org/r/65422/#comment276940> Instead of config this should be explain modifier. WE already have explain rewrite select .. We similarly can add explain reoptimize select ... common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 5066 (patched) <https://reviews.apache.org/r/65422/#comment276964> Instead of iterating over _this_ which can be very large, more efficient is to iterate on other list. ql/src/java/org/apache/hadoop/hive/ql/AbstractReExecDriver.java Lines 36 (patched) <https://reviews.apache.org/r/65422/#comment276962> Comments on what this driver does? ql/src/java/org/apache/hadoop/hive/ql/AbstractReExecDriver.java Lines 127 (patched) <https://reviews.apache.org/r/65422/#comment276958> Currently its only reexcuted once. Alternatively, we can keep re-running it if it fails again. e.g. in case of OOM, its possible that there are many joins which are mis-planed, but we get stats only for first join. To avoid, very large number of retrials we can limit to some max attempts. ql/src/java/org/apache/hadoop/hive/ql/DriverFactory.java Lines 21 (patched) <https://reviews.apache.org/r/65422/#comment276959> Incorrect import ? ql/src/java/org/apache/hadoop/hive/ql/ReExecOverlayDriver.java Lines 25 (patched) <https://reviews.apache.org/r/65422/#comment276961> Add comments on what this Driver does. ql/src/java/org/apache/hadoop/hive/ql/ReOptimizeDriver.java Lines 29 (patched) <https://reviews.apache.org/r/65422/#comment276963> java class docs. ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java Lines 4748-4749 (patched) <https://reviews.apache.org/r/65422/#comment276968> To avoid this we should switch to find equivalence of 2 operators based on their signature. e.g., Operator::logicalEquals(). ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java Lines 2364 (patched) <https://reviews.apache.org/r/65422/#comment276965> This should extend to all operators. ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java Lines 33 (patched) <https://reviews.apache.org/r/65422/#comment276966> javadoc ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/refs/OperatorRef.java Lines 32 (patched) <https://reviews.apache.org/r/65422/#comment276969> Operator::logicalEquals() ? ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/refs/OperatorRef.java Lines 50 (patched) <https://reviews.apache.org/r/65422/#comment276967> Instead of relying on ids, better is to use (and extend) logic in SharedWorkOptimizer::compareOperator() ? ql/src/java/org/apache/hadoop/hive/ql/stats/OperatorStatsReaderHook.java Lines 56 (patched) <https://reviews.apache.org/r/65422/#comment276970> LOG.debug - Ashutosh Chauhan On Jan. 30, 2018, 6:13 p.m., Zoltan Haindrich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65422/ > ----------------------------------------------------------- > > (Updated Jan. 30, 2018, 6:13 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Repository: hive-git > > > Description > ------- > > preview > > > Diffs > ----- > > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java a78e0c63d7 > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b7d3e99e1a > hcatalog/core/src/main/java/org/apache/hive/hcatalog/cli/HCatCli.java > ad31287879 > hcatalog/core/src/main/java/org/apache/hive/hcatalog/cli/HCatDriver.java > 533f0bcd6f > itests/src/test/resources/testconfiguration.properties d86ff58840 > ql/src/java/org/apache/hadoop/hive/ql/AbstractReExecDriver.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/Context.java 820fbf0f58 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 74595b00f9 > ql/src/java/org/apache/hadoop/hive/ql/DriverFactory.java 49d2bf5f33 > ql/src/java/org/apache/hadoop/hive/ql/IDriver.java 6280be0b08 > ql/src/java/org/apache/hadoop/hive/ql/ReExecOverlayDriver.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/ReOptimizeDriver.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 76e85636d1 > ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java 199b181290 > ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java > 395a5f450f > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/reducesink/VectorReduceSinkCommonOperator.java > 8dd7cfe58c > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/reducesink/VectorReduceSinkEmptyKeyOperator.java > 134fc0ff0b > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/reducesink/VectorReduceSinkObjectHashOperator.java > 1eb72ce4d9 > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/reducesink/VectorReduceSinkUniformHashOperator.java > 384bd74686 > ql/src/java/org/apache/hadoop/hive/ql/hooks/PrivateHookContext.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java > 190771ea6b > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java > cbadfa4f07 > ql/src/java/org/apache/hadoop/hive/ql/plan/Statistics.java 0057f0c2c6 > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/EmptyStatsSource.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/GroupTransformer.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapperProcess.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/RuntimeStatsSource.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/SimpleRuntimeStatsSource.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/StatsSource.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/refs/OperatorRef.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java > dcf8d31eaf > ql/src/java/org/apache/hadoop/hive/ql/stats/OperatorStats.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/stats/OperatorStatsReaderHook.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAssertTrueOOM.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java > d56002d192 > ql/src/test/org/apache/hadoop/hive/ql/plan/mapping/TestCounterMapping.java > PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/plan/mapping/TestReOptimization.java > PRE-CREATION > ql/src/test/queries/clientpositive/retry_failure.q PRE-CREATION > ql/src/test/queries/clientpositive/retry_failure_oom.q PRE-CREATION > ql/src/test/queries/clientpositive/retry_failure_stat_changes.q > PRE-CREATION > ql/src/test/results/clientpositive/llap/retry_failure.q.out PRE-CREATION > ql/src/test/results/clientpositive/llap/retry_failure_oom.q.out > PRE-CREATION > ql/src/test/results/clientpositive/llap/retry_failure_stat_changes.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65422/diff/1/ > > > Testing > ------- > > > Thanks, > > Zoltan Haindrich > >
