You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org> on 2018/06/18 20:41:33 UTC
Review Request 67636: HIVE-19176 : Add HoS support to progress bar on
Beeline client.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/
-----------------------------------------------------------
Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
Repository: hive-git
Description
-------
This logic is similar to the RenderStrategy used in Tez to print the progress bar.
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 933bda4ad01a6f7878019a7b4c971a0c39068ae2
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
Diff: https://reviews.apache.org/r/67636/diff/1/
Testing
-------
Thanks,
Bharathkrishna Guruvayoor Murali
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
> On June 21, 2018, 7:47 p.m., Sahil Takiar wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
> > Lines 96 (patched)
> > <https://reviews.apache.org/r/67636/diff/1-2/?file=2042207#file2042207line96>
> >
> > why does testOutput contain both formats?
> >
> > also there is a unnecessary pair of "()" here
One is the progress bar and other one is the report string.
Please note that for the test, I am using :
updateFunction = new RenderStrategy.InPlaceUpdateFunction(monitor);
So it will just print the progress bar and report string to the output stream.
Should I add any tests specifically from beeline side to test the progress bar?
> On June 21, 2018, 7:47 p.m., Sahil Takiar wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java
> > Lines 108 (patched)
> > <https://reviews.apache.org/r/67636/diff/2/?file=2044174#file2044174line108>
> >
> > why is this necessary?
The test TestSparkTask#testRemoteSparkCancel was failing, as at some point it calls SparkJobMonitor#updateFunction and SessionState.get().isHiveServerQuery() was getting NPE.
- Bharathkrishna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205203
-----------------------------------------------------------
On June 22, 2018, 8:01 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 22, 2018, 8:01 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205203
-----------------------------------------------------------
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java
Lines 96 (patched)
<https://reviews.apache.org/r/67636/#comment288136>
why does testOutput contain both formats?
also there is a unnecessary pair of "()" here
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 54 (patched)
<https://reviews.apache.org/r/67636/#comment288131>
since this is a constant, can you make it static. FindBugs is complaining about it: "Unread field:field be static? At RenderStrategy.java:[line 47]"
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 152-154 (patched)
<https://reviews.apache.org/r/67636/#comment288133>
use `progressMap.entrySet` for this instead. FindBugs is complaining about it "org.apache.hadoop.hive.ql.exec.spark.status.RenderStrategy$BaseUpdateFunction.isSameAsPreviousProgress(Map, Map) makes inefficient use of keySet iterator instead of entrySet iterator At RenderStrategy.java:of keySet iterator instead of entrySet iterator At RenderStrategy.java:[line 147]"
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java
Lines 108 (patched)
<https://reviews.apache.org/r/67636/#comment288134>
why is this necessary?
- Sahil Takiar
On June 21, 2018, 3:50 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 3:50 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205643
-----------------------------------------------------------
Ship it!
Ship It!
- Sahil Takiar
On June 22, 2018, 8:01 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 22, 2018, 8:01 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/
-----------------------------------------------------------
(Updated June 22, 2018, 8:01 p.m.)
Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
Repository: hive-git
Description
-------
This logic is similar to the RenderStrategy used in Tez to print the progress bar.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
Diff: https://reviews.apache.org/r/67636/diff/3/
Changes: https://reviews.apache.org/r/67636/diff/2-3/
Testing
-------
Thanks,
Bharathkrishna Guruvayoor Murali
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/
-----------------------------------------------------------
(Updated June 21, 2018, 3:50 p.m.)
Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
Changes
-------
Fixed failing test in TestSparkTask.
Added progress bar to the unit test.
Added javadocs.
Repository: hive-git
Description
-------
This logic is similar to the RenderStrategy used in Tez to print the progress bar.
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
Diff: https://reviews.apache.org/r/67636/diff/2/
Changes: https://reviews.apache.org/r/67636/diff/1-2/
Testing
-------
Thanks,
Bharathkrishna Guruvayoor Murali
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
> On June 19, 2018, 8:53 p.m., Sahil Takiar wrote:
> > is there a way to add some unit tests for this?
I added the progress bar to unit test.
Do you mean to write some test from beeline side, or is this good enough?
- Bharathkrishna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205020
-----------------------------------------------------------
On June 21, 2018, 3:50 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 3:50 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e7f5fc0c6a3d527671c354db8ef2c9772aab6dd0
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestSparkTask.java 368fa9f1fa3ccc8b78cc4f9e98acf352cbc1c4c3
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
> On June 19, 2018, 8:53 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/67636/diff/1/?file=2042205#file2042205line43>
> >
> > how much of this code is new coded vs. copied from other classes?
The methods getReport, isSameAsPreviousProgress, printStatus are copied from existing.
- Bharathkrishna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205020
-----------------------------------------------------------
On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 18, 2018, 8:41 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 933bda4ad01a6f7878019a7b4c971a0c39068ae2
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205020
-----------------------------------------------------------
is there a way to add some unit tests for this?
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 37-40 (patched)
<https://reviews.apache.org/r/67636/#comment287897>
can u add some javadocs for this
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 43 (patched)
<https://reviews.apache.org/r/67636/#comment287899>
how much of this code is new coded vs. copied from other classes?
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 209 (patched)
<https://reviews.apache.org/r/67636/#comment287898>
javadocs please
- Sahil Takiar
On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 18, 2018, 8:41 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 933bda4ad01a6f7878019a7b4c971a0c39068ae2
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review204951
-----------------------------------------------------------
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 379 (patched)
<https://reviews.apache.org/r/67636/#comment287785>
Added this condition because in the do-while loop, when this executes after the results are printed, the results get overwritten by progress bar. So if operation is complete, no need to update.
- Bharathkrishna Guruvayoor Murali
On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 18, 2018, 8:41 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 933bda4ad01a6f7878019a7b4c971a0c39068ae2
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>
Re: Review Request 67636: HIVE-19176 : Add HoS support to progress
bar on Beeline client.
Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67636/#review205024
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java
Lines 43 (patched)
<https://reviews.apache.org/r/67636/#comment287903>
The methods getReport, isSameAsPreviousProgress, printStatus are copied from existing.
- Bharathkrishna Guruvayoor Murali
On June 18, 2018, 8:41 p.m., Bharathkrishna Guruvayoor Murali wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67636/
> -----------------------------------------------------------
>
> (Updated June 18, 2018, 8:41 p.m.)
>
>
> Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This logic is similar to the RenderStrategy used in Tez to print the progress bar.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 933bda4ad01a6f7878019a7b4c971a0c39068ae2
> jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java ad8d1a7f1cca3a763bb7c07335998ab7d39d7598
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/LocalSparkJobMonitor.java 2a6c33bfd4824c96e7004cd1ecce48c62c97d685
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java 004b50ba95934280cf302055a46a5d984b421e07
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RenderStrategy.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 3531ac25a9959aacd5766a9a42316890c68a1cd5
> ql/src/test/org/apache/hadoop/hive/ql/exec/spark/status/TestSparkJobMonitor.java e66354f0869738bd3cf0eb831c13fa6af1eda256
> service/src/java/org/apache/hive/service/ServiceUtils.java 226e43244df10c22143b91f92ef312e56739d036
> service/src/java/org/apache/hive/service/cli/SparkProgressMonitorStatusMapper.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 68fe8d8aa143fafbfc611253ce3a12065016a537
>
>
> Diff: https://reviews.apache.org/r/67636/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bharathkrishna Guruvayoor Murali
>
>