You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2015/09/26 03:36:31 UTC
Review Request 38783: HIVE-11969 start Tez session in background when
starting CLI
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/
-----------------------------------------------------------
Review request for hive, Gunther Hagleitner and Siddharth Seth.
Repository: hive-git
Description
-------
See JIRA
Diffs
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 014941e
Diff: https://reviews.apache.org/r/38783/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Sergey Shelukhin <se...@hortonworks.com>.
> On Sept. 30, 2015, 10:25 p.m., Siddharth Seth wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java, line 154
> > <https://reviews.apache.org/r/38783/diff/1/?file=1085096#file1085096line154>
> >
> > Minor: Second parameter needed? Is always null
Kept it for consistency with open(), to which it is passed in some places
- Sergey
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/#review101167
-----------------------------------------------------------
On Sept. 26, 2015, 1:36 a.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38783/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2015, 1:36 a.m.)
>
>
> Review request for hive, Gunther Hagleitner and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> See JIRA
>
>
> Diffs
> -----
>
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 014941e
>
> Diff: https://reviews.apache.org/r/38783/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/#review101167
-----------------------------------------------------------
Mostly minor comments on the patch itself.
Some more suggestions:
- Is a configuration flag required to go back to the original behaviour in case of issues - i.e. defaults to async via the CLI, but an option exists to go back to the current model - where everything happens in the same state. I'm a little wary of ThreadLocals used in various places causing issues.
- Change Q tests to use this mechanism as well. They open their own session if I'm not mistaken.
- In getSesssion() - if the future has not completed, does it make sense to log a line every 10 seconds or so to indicate that a query is not running due to inadequate cluster resources / waiting for the AM.
- Would statements like show databases; / describe table hit this code path. I don't think getSession would be invoked (that's only from TezTask when a DAG is created)
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 151)
<https://reviews.apache.org/r/38783/#comment158511>
Minor: Second parameter needed? Is always null
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 274)
<https://reviews.apache.org/r/38783/#comment158512>
Typo: initilize
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 353)
<https://reviews.apache.org/r/38783/#comment158513>
Does sessionFuture need to be set to null as well. Looks like a TezSessionState object can be re-used with different AMs / Applications.
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 361)
<https://reviews.apache.org/r/38783/#comment158514>
Minor: parameter name can cause confusion. Isn't always the asyncSession
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java (line 380)
<https://reviews.apache.org/r/38783/#comment158517>
I believe the behaviour will be similar to what it is rightnow for invocation of open(). i.e. a RuntimeException will end up being thrown in case of most errors.
Wondering if a SessionNotRunning exception from waitTillReady() needs to be handled differently in the async model. Separate jira anyway.
- Siddharth Seth
On Sept. 26, 2015, 1:36 a.m., Sergey Shelukhin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38783/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2015, 1:36 a.m.)
>
>
> Review request for hive, Gunther Hagleitner and Siddharth Seth.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> See JIRA
>
>
> Diffs
> -----
>
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java d62fd5c
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 014941e
>
> Diff: https://reviews.apache.org/r/38783/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sergey Shelukhin
>
>
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/
-----------------------------------------------------------
(Updated Oct. 5, 2015, 9:16 p.m.)
Review request for hive, Gunther Hagleitner and Siddharth Seth.
Repository: hive-git
Description
-------
See JIRA
Diffs (updated)
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 4b52578
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 54a529e
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java 2d740ed
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java dc8c336
ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestTezTask.java d004a27
Diff: https://reviews.apache.org/r/38783/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/
-----------------------------------------------------------
(Updated Oct. 2, 2015, 9:10 p.m.)
Review request for hive, Gunther Hagleitner and Siddharth Seth.
Repository: hive-git
Description
-------
See JIRA
Diffs (updated)
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 4b52578
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 77ca613
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java 2d740ed
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java dc8c336
ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestTezTask.java d004a27
Diff: https://reviews.apache.org/r/38783/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/
-----------------------------------------------------------
(Updated Oct. 1, 2015, 9:04 p.m.)
Review request for hive, Gunther Hagleitner and Siddharth Seth.
Repository: hive-git
Description
-------
See JIRA
Diffs (updated)
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 4b52578
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java dc8c336
Diff: https://reviews.apache.org/r/38783/diff/
Testing
-------
Thanks,
Sergey Shelukhin
Re: Review Request 38783: HIVE-11969 start Tez session in background
when starting CLI
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38783/
-----------------------------------------------------------
(Updated Oct. 1, 2015, 9:03 p.m.)
Review request for hive, Gunther Hagleitner and Siddharth Seth.
Repository: hive-git
Description
-------
See JIRA
Diffs (updated)
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 4b52578
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 568ebbe
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java dc8c336
Diff: https://reviews.apache.org/r/38783/diff/
Testing
-------
Thanks,
Sergey Shelukhin