zabetak commented on a change in pull request #2865:
URL: https://github.com/apache/hive/pull/2865#discussion_r769579986
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -673,8 +672,7 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx)
throws SemanticExcept
}
this.ctx.setCboInfo(cboMsg);
- // Determine if we should re-throw the exception OR if we try to
mark plan as reAnalyzeAST to retry
- // planning as non-CBO.
+ // Determine if we should re-throw the exception OR if we try to
mark the query to retry as non-CBO.
Review comment:
I am wondering if it would be better to just throw here and let the
reexecution plugin deal with what do to afterwards.
##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -5561,7 +5563,8 @@ private static void populateLlapDaemonVarsSet(Set<String>
llapDaemonVarsSetLocal
"Size of the runtime statistics cache. Unit is: OperatorStat entry; a
query plan consist ~100."),
HIVE_QUERY_PLANMAPPER_LINK_RELNODES("hive.query.planmapper.link.relnodes",
true,
"Whether to link Calcite nodes to runtime statistics."),
-
+ HIVE_QUERY_MAX_RECOMPILATION_COUNT("hive.query.recompilation.max.count", 1,
+ "Maximum number of re-compilations for a single query."),
Review comment:
Why do we need the number of recompilations to be configurable?
##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -5536,10 +5536,12 @@ private static void
populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
HIVE_QUERY_REEXECUTION_ENABLED("hive.query.reexecution.enabled", true,
"Enable query reexecutions"),
- HIVE_QUERY_REEXECUTION_STRATEGIES("hive.query.reexecution.strategies",
"overlay,reoptimize,reexecute_lost_am,dagsubmit",
+ HIVE_QUERY_REEXECUTION_STRATEGIES("hive.query.reexecution.strategies",
+ "overlay,reoptimize,reexecute_lost_am,dagsubmit,recompile_without_cbo",
"comma separated list of plugin can be used:\n"
+ " overlay: hiveconf subtree 'reexec.overlay' is used as an
overlay in case of an execution errors out\n"
+ " reoptimize: collects operator statistics during execution and
recompile the query after a failure\n"
+ + " recompile_without_cbo: recompiles query after a CBO failure\n"
+ " reexecute_lost_am: reexecutes query if it failed due to tez
am node gets decommissioned"),
Review comment:
I didn't go through the whole PR but I get the impression that we
could/should combine the `hive.query.reexecution.strategies` and
`hive.cbo.fallback.strategy` configurations somehow. Having both does not seem
necessary and raises some questions about the expected behavior.
Consider for instance the following:
```
set hive.cbo.fallback.strategy = always;
-- Note that recompile_without_cbo is missing
set hive.query.reexecution.strategies =
overlay,reoptimize,reexecute_lost_am,dagsubmit;
```
Reading the current configuration I am not sure what should happen when the
CBO fails.
The `hive.cbo.fallback.strategy` has not been released yet so we are free to
drop it, or modify it to be consistent with `hive.query.reexecution.strategies`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
