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/05/23 05:32:24 UTC

Review Request 67263: HIVE-19602

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/
-----------------------------------------------------------

Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Bugs: HIVE-19602
    https://issues.apache.org/jira/browse/HIVE-19602


Repository: hive-git


Description
-------

Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/1/


Testing
-------


Thanks,

Bharathkrishna Guruvayoor Murali


Re: Review Request 67263: HIVE-19602

Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203993
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
Line 340 (original), 234 (patched)
<https://reviews.apache.org/r/67263/#comment286394>

    add some javadocs for this class



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 12 (patched)
<https://reviews.apache.org/r/67263/#comment286393>

    can you add some javadocs for this class



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 18 (patched)
<https://reviews.apache.org/r/67263/#comment286392>

    this can be package private right?


- Sahil Takiar


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

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/67263/#review203640
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 27 (patched)
<https://reviews.apache.org/r/67263/#comment285929>

    Using an empty String as last argument, because the standard InplaceUpdate header expects one more argument. Same in rows() method too.
    
    Should I instead pass the headerFormat and rowFormat also as a parameter, by adding methods in ProgressMonitor interface? Then I will need to update Tez code, thrift code to have response in this form. Can avoid that if this is acceptable.



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 77 (patched)
<https://reviews.apache.org/r/67263/#comment285930>

    These states are not used as of now, but I guess it will be needed when we add progress bar in beeline.
    (with states similar to TezProgressMonitorStatusMapper).
    Are these executionStatuses good enough.



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 102 (patched)
<https://reviews.apache.org/r/67263/#comment285928>

    Using just an int counter here, instead of adding completed stages to a HashSet as done in existing implementation, because we just need the count.


- Bharathkrishna Guruvayoor Murali


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 70 (original), 67 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line72>
> >
> >     is this still used?

Will remove this if we do not need to pass the headers , footers etc.. to ProgressMonitor.
(ie. if the progress bar format shown in below comment is acceptable).


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> >     whats the point of this class?

I used this class to follow same pattern as in tez. I will add the logic similar to RenderStrategy used in tez while adding beeline progress bar, so this should be useful.


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
> > Lines 27 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027541#file2027541line27>
> >
> >     whats the impact of having an extra argument? does the formatting change at all?

It changes a bit,like this:

----------------------------------------------------------------------------------------------
          STAGES   ATTEMPT        STATUS  TOTAL  COMPLETED  RUNNING  PENDING  FAILED
----------------------------------------------------------------------------------------------
Stage-1 ........         0      FINISHED      1          1        0        0       0
----------------------------------------------------------------------------------------------
STAGES: 01/01    [==========================>>] 100%  ELAPSED TIME: 1.01 s
----------------------------------------------------------------------------------------------

Notice the bit of extra space at the end. But other than that, it looks pretty much same.


- Bharathkrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > are there any logic changes, or is most of the code just copied into the new class?

Most of the code is copied to new class. Mostly I have the doubts mentioned in above comments.


- Bharathkrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 70 (original), 67 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line72>
> >
> >     is this still used?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     Will remove this if we do not need to pass the headers , footers etc.. to ProgressMonitor.
>     (ie. if the progress bar format shown in below comment is acceptable).
> 
> Sahil Takiar wrote:
>     So what changes about the progress bar format if you do pass the headers and footers?

Nothing changes, except the bit of dashed lines at the end (----), which we are considering fine for now as mentioned in below comment.
So removing this variable definition.


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> >     whats the point of this class?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     I used this class to follow same pattern as in tez. I will add the logic similar to RenderStrategy used in tez while adding beeline progress bar, so this should be useful.
> 
> Sahil Takiar wrote:
>     Ok, can this be a private class? Is it used outside `SparkJobMonitor`?

Not used outside SparkJobMonitor. Making private


- Bharathkrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------


On May 29, 2018, 10:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 29, 2018, 10:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Sahil Takiar <ta...@gmail.com>.

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 70 (original), 67 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line72>
> >
> >     is this still used?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     Will remove this if we do not need to pass the headers , footers etc.. to ProgressMonitor.
>     (ie. if the progress bar format shown in below comment is acceptable).

So what changes about the progress bar format if you do pass the headers and footers?


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> >     whats the point of this class?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     I used this class to follow same pattern as in tez. I will add the logic similar to RenderStrategy used in tez while adding beeline progress bar, so this should be useful.

Ok, can this be a private class? Is it used outside `SparkJobMonitor`?


> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
> > Lines 27 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027541#file2027541line27>
> >
> >     whats the impact of having an extra argument? does the formatting change at all?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     It changes a bit,like this:
>     
>     ----------------------------------------------------------------------------------------------
>               STAGES   ATTEMPT        STATUS  TOTAL  COMPLETED  RUNNING  PENDING  FAILED
>     ----------------------------------------------------------------------------------------------
>     Stage-1 ........         0      FINISHED      1          1        0        0       0
>     ----------------------------------------------------------------------------------------------
>     STAGES: 01/01    [==========================>>] 100%  ELAPSED TIME: 1.01 s
>     ----------------------------------------------------------------------------------------------
>     
>     Notice the bit of extra space at the end. But other than that, it looks pretty much same.

Ok, thats probably fine for now.


- Sahil


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Bharathkrishna Guruvayoor Murali via Review Board <no...@reviews.apache.org>.

> On May 25, 2018, 4:43 p.m., Sahil Takiar wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
> > Line 304 (original), 231 (patched)
> > <https://reviews.apache.org/r/67263/diff/1/?file=2027540#file2027540line306>
> >
> >     whats the point of this class?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     I used this class to follow same pattern as in tez. I will add the logic similar to RenderStrategy used in tez while adding beeline progress bar, so this should be useful.
> 
> Sahil Takiar wrote:
>     Ok, can this be a private class? Is it used outside `SparkJobMonitor`?
> 
> Bharathkrishna Guruvayoor Murali wrote:
>     Not used outside SparkJobMonitor. Making private

I actually removed this class, because I was using the static class in a "non-static" kind of way. Just donot want to have code with wrong pattern even though it will bee changed soon.
I will add it when it is actually required, ie. later when I need to introduce progress bar in beeline.


- Bharathkrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------


On June 1, 2018, 4:58 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated June 1, 2018, 4:58 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review203891
-----------------------------------------------------------



are there any logic changes, or is most of the code just copied into the new class?


ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
Line 70 (original), 67 (patched)
<https://reviews.apache.org/r/67263/#comment286249>

    is this still used?



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
Line 304 (original), 231 (patched)
<https://reviews.apache.org/r/67263/#comment286251>

    whats the point of this class?



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 27 (patched)
<https://reviews.apache.org/r/67263/#comment286252>

    whats the impact of having an extra argument? does the formatting change at all?



ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java
Lines 77 (patched)
<https://reviews.apache.org/r/67263/#comment286254>

    the current states LGTM


- Sahil Takiar


On May 23, 2018, 5:32 a.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 5:32 a.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review204126
-----------------------------------------------------------


Fix it, then Ship it!




minor comment otherwise LGTM


ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java
Line 238 (original), 245 (patched)
<https://reviews.apache.org/r/67263/#comment286518>

    can be private


- Sahil Takiar


On May 29, 2018, 10:53 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated May 29, 2018, 10:53 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67263/#review204332
-----------------------------------------------------------


Ship it!




Ship It!

- Sahil Takiar


On June 4, 2018, 6:34 p.m., Bharathkrishna Guruvayoor Murali wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67263/
> -----------------------------------------------------------
> 
> (Updated June 4, 2018, 6:34 p.m.)
> 
> 
> Review request for hive, Sahil Takiar and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-19602
>     https://issues.apache.org/jira/browse/HIVE-19602
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67263/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bharathkrishna Guruvayoor Murali
> 
>


Re: Review Request 67263: HIVE-19602

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/67263/
-----------------------------------------------------------

(Updated June 7, 2018, 10:43 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
-------

Making changes to be in sync with HIVE-19508 so that it does not cause merge conflicts with master


Bugs: HIVE-19602
    https://issues.apache.org/jira/browse/HIVE-19602


Repository: hive-git


Description
-------

Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java e78b1cd6637c46070378c25a372916817fe99a59 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/5/

Changes: https://reviews.apache.org/r/67263/diff/4-5/


Testing
-------


Thanks,

Bharathkrishna Guruvayoor Murali


Re: Review Request 67263: HIVE-19602

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/67263/
-----------------------------------------------------------

(Updated June 4, 2018, 6:34 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
-------

Correcting issues reported by checkstyle and findbugs checks.


Bugs: HIVE-19602
    https://issues.apache.org/jira/browse/HIVE-19602


Repository: hive-git


Description
-------

Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/4/

Changes: https://reviews.apache.org/r/67263/diff/3-4/


Testing
-------


Thanks,

Bharathkrishna Guruvayoor Murali


Re: Review Request 67263: HIVE-19602

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/67263/
-----------------------------------------------------------

(Updated June 1, 2018, 4:58 a.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Bugs: HIVE-19602
    https://issues.apache.org/jira/browse/HIVE-19602


Repository: hive-git


Description
-------

Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/3/

Changes: https://reviews.apache.org/r/67263/diff/2-3/


Testing
-------


Thanks,

Bharathkrishna Guruvayoor Murali


Re: Review Request 67263: HIVE-19602

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/67263/
-----------------------------------------------------------

(Updated May 29, 2018, 10:53 p.m.)


Review request for hive, Sahil Takiar and Vihang Karajgaonkar.


Changes
-------

Making changes wrt review comments


Bugs: HIVE-19602
    https://issues.apache.org/jira/browse/HIVE-19602


Repository: hive-git


Description
-------

Refactor inplace progress code in Hive-on-spark progress monitor to use ProgressMonitor instance


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java 7afd8864075aa0d9708274eea8839c662324c732 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkProgressMonitor.java PRE-CREATION 


Diff: https://reviews.apache.org/r/67263/diff/2/

Changes: https://reviews.apache.org/r/67263/diff/1-2/


Testing
-------


Thanks,

Bharathkrishna Guruvayoor Murali