> On April 14, 2016, 7:26 a.m., Szehon Ho wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java, line 203
> > <https://reviews.apache.org/r/45733/diff/2/?file=1327502#file1327502line203>
> >
> >     Is this needed?

Without this, the function always return `false`. Might as well remove the 
variable and the return type.


> On April 14, 2016, 7:26 a.m., Szehon Ho wrote:
> > itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestQueryDisplay.java,
> >  line 155
> > <https://reviews.apache.org/r/45733/diff/2/?file=1327498#file1327498line155>
> >
> >     Was a task removed from the display?

No, but there is some change in when the tasks are updated in the query 
display. Each task updates the task display whenever there's a status change. 
The query display needs to be registered with the task. Earlier they were 
getting set through the `QueryPlan` constructer, and a lot of tasks were 
getting missed in the query display. Now they are registered by the driver in 
the execute function. Because of this, all the important tasks have query 
displays, and some not-so important tasks are left out, which is fine. 

Secondly, this was already 1 before my last change. Now it's again 1, so 
shouldn't be an issue.


On April 14, 2016, 7:26 a.m., Rajat Khandelwal wrote:
> > Looks good mostly.  Just some questions and a nit below.
> > 
> > Also it doesn't work in all cases right?  Like for example if its a 
> > MapRedLocalTask that runs out of process, this will not show the progress.

I'm seeing some MapRedLocal tasks in the output, haven't checked whether they 
were running out of process.


- Rajat


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


On April 6, 2016, 3:27 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45733/
> -----------------------------------------------------------
> 
> (Updated April 6, 2016, 3:27 p.m.)
> 
> 
> Review request for hive, Amareshwari Sriramadasu and Szehon Ho.
> 
> 
> Bugs: HIVE-13421
>     https://issues.apache.org/jira/browse/HIVE-13421
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestQueryDisplay.java
>  418f71eb87cdd519677b2f5a59c67099f704ec80 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 
> 7276e31ac2ec221c803b86f36d9cfcc4b2811e8c 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryDisplay.java 
> d582bc063fc150002a01d63451ae6632fca29ac1 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryPlan.java 
> ef0923d555ba662b4ed30ef45a3d72760cdfad52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 
> c96c8135a344049e57167559c4d760b876a42ca5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 
> 6c677f5bbae024b503594238e59f9fbf6ba283cf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java 
> d164859219896d88c42a69e56f621cb08012f633 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HadoopJobExecHelper.java 
> 1b296b9986907d983a754f9957f2cbe4f7583ae5 
>   service/src/test/org/apache/hive/service/cli/CLIServiceTest.java 
> 698b13d66f100618aab3c3ee2cbf3c3df8477afe 
> 
> Diff: https://reviews.apache.org/r/45733/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>

Reply via email to