You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Peter Vary <pv...@cloudera.com> on 2017/10/06 15:37:19 UTC
Review Request 62810: HIVE-17300 WebUI query plan graphs
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62810/
-----------------------------------------------------------
Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
Bugs: HIVE-17300
https://issues.apache.org/jira/browse/HIVE-17300
Repository: hive-git
Description
-------
Moving the review here, since could not change Karen's original one
Diffs
-----
common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d2afc2c
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1943c6d
ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514
ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java bf6cb91
ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 3c07197
ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1
service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e
service/src/resources/hive-webapps/static/css/query-plan-graph.css PRE-CREATION
service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION
service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION
Diff: https://reviews.apache.org/r/62810/diff/1/
Testing
-------
Thanks,
Peter Vary
Re: Review Request 62810: HIVE-17300 WebUI query plan graphs
Posted by Barna Zsombor Klara <kz...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62810/#review189575
-----------------------------------------------------------
Thank you for the patch Peter (and Karen). I have 3 minor comments if you fixed those, then we can ship it.
common/src/java/org/apache/hadoop/hive/common/LogUtils.java
Lines 239-240 (patched)
<https://reviews.apache.org/r/62810/#comment266728>
Can you please check that this cast is always correct? An if with an instance of check should be enough.
ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java
Lines 159 (patched)
<https://reviews.apache.org/r/62810/#comment266730>
I would prefer an iterator with a type parameter. This way we can avoid the explicit cast 2 lines below.
ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java
Lines 162 (patched)
<https://reviews.apache.org/r/62810/#comment266731>
Same as before, please use a typed iterator if possible.
- Barna Zsombor Klara
On Oct. 6, 2017, 3:37 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62810/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2017, 3:37 p.m.)
>
>
> Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
>
>
> Bugs: HIVE-17300
> https://issues.apache.org/jira/browse/HIVE-17300
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Moving the review here, since could not change Karen's original one
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d2afc2c
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1943c6d
> ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514
> ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java bf6cb91
> ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 3c07197
> ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1
> service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e
> service/src/resources/hive-webapps/static/css/query-plan-graph.css PRE-CREATION
> service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION
> service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62810/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 62810: HIVE-17300 WebUI query plan graphs
Posted by Barna Zsombor Klara <kz...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62810/#review189606
-----------------------------------------------------------
Ship it!
Ship It!
- Barna Zsombor Klara
On Oct. 30, 2017, 4:23 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62810/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2017, 4:23 p.m.)
>
>
> Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
>
>
> Bugs: HIVE-17300
> https://issues.apache.org/jira/browse/HIVE-17300
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Moving the review here, since could not change Karen's original one
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6631a6e
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6c6ad92
> ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514
> ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java 132bec6
> ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 2d2eafd
> ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1
> service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e
> service/src/resources/hive-webapps/static/css/query-plan-graph.css PRE-CREATION
> service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION
> service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62810/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 62810: HIVE-17300 WebUI query plan graphs
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62810/
-----------------------------------------------------------
(Updated Oct. 30, 2017, 4:23 p.m.)
Review request for hive, Karen Coppage, Xuefu Zhang, and Xuefu Zhang.
Changes
-------
Addressing Zsombor's comments
Bugs: HIVE-17300
https://issues.apache.org/jira/browse/HIVE-17300
Repository: hive-git
Description
-------
Moving the review here, since could not change Karen's original one
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/common/LogUtils.java 0a3e0c7
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6631a6e
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6c6ad92
ql/src/java/org/apache/hadoop/hive/ql/MapRedStats.java 4b60514
ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java 132bec6
ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 2d2eafd
ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapRedTask.java 41a1ef1
service/src/jamon/org/apache/hive/tmpl/QueryProfileTmpl.jamon ff7476e
service/src/resources/hive-webapps/static/css/query-plan-graph.css PRE-CREATION
service/src/resources/hive-webapps/static/js/query-plan-graph.js PRE-CREATION
service/src/resources/hive-webapps/static/js/vis.min.js PRE-CREATION
Diff: https://reviews.apache.org/r/62810/diff/2/
Changes: https://reviews.apache.org/r/62810/diff/1-2/
Testing
-------
Thanks,
Peter Vary