You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Navis Ryu <na...@nexr.com> on 2013/12/30 07:57:57 UTC

Re: Review Request 15873: Query cancel should stop running MR tasks

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

(Updated Dec. 30, 2013, 6:57 a.m.)


Review request for hive.


Changes
-------

Fixed bug in parallel execution


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


Repository: hive-git


Description
-------

Currently, query canceling does not stop running MR job immediately.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 

Diff: https://reviews.apache.org/r/15873/diff/


Testing
-------


Thanks,

Navis Ryu


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.

> On Feb. 25, 2014, 4:10 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 51
> > <https://reviews.apache.org/r/15873/diff/2/?file=404159#file404159line51>
> >
> >     We should make this volatile, another thread calling shutdown might otherwise it is not clear when the main thread will know that shutdown has been called.
> >     
> >     The calls to the running concurrent queue can establish happens-before relation ship in some cases, but it is not clear that it will establish it between the shutdown and poll.
> >     "All of these collections help avoid Memory Consistency Errors by defining a happens-before relationship between an operation that adds an object to the collection with subsequent operations that access or remove that object." http://docs.oracle.com/javase/tutorial/essential/concurrency/collections.html
> >

It's protected by synchronized keyword. DriverContext contains queues and statuses which should be protected from concurrent access.


- Navis


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


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review35325
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment65785>

    We should make this volatile, another thread calling shutdown might otherwise it is not clear when the main thread will know that shutdown has been called.
    
    The calls to the running concurrent queue can establish happens-before relation ship in some cases, but it is not clear that it will establish it between the shutdown and poll.
    "All of these collections help avoid Memory Consistency Errors by defining a happens-before relationship between an operation that adds an object to the collection with subsequent operations that access or remove that object." http://docs.oracle.com/javase/tutorial/essential/concurrency/collections.html
    


- Thejas Nair


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.

> On Feb. 27, 2014, 11:08 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 110
> > <https://reviews.apache.org/r/15873/diff/3/?file=478815#file478815line110>
> >
> >     When pollFinished is running, this shutdown() function will not be able to make progress. Which means that the query cancellation will happen only after a task (could be an MR task) is complete.
> >     
> >     It seems synchronizing around shutdown should be sufficient, either by making it volatile or having synchronized methods around it.
> >     
> >     Since thread safe concurrent collection classes are being used here, I don't see other concurrency issues that would make it necessary to make all these functions synchronized. 
> >     
> >     
> >

It just only polls status of running tasks and goes into wait state quite quickly, so it would not hinder shutdown process. Furthermore, two threads, polling and shutdown, has a race condition on both collections, runnable and running, so those should be guarded by shared something.


- Navis


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


On Feb. 27, 2014, 7:06 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 7:06 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6705ec4 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.

> On Feb. 27, 2014, 11:08 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 110
> > <https://reviews.apache.org/r/15873/diff/3/?file=478815#file478815line110>
> >
> >     When pollFinished is running, this shutdown() function will not be able to make progress. Which means that the query cancellation will happen only after a task (could be an MR task) is complete.
> >     
> >     It seems synchronizing around shutdown should be sufficient, either by making it volatile or having synchronized methods around it.
> >     
> >     Since thread safe concurrent collection classes are being used here, I don't see other concurrency issues that would make it necessary to make all these functions synchronized. 
> >     
> >     
> >
> 
> Navis Ryu wrote:
>     It just only polls status of running tasks and goes into wait state quite quickly, so it would not hinder shutdown process. Furthermore, two threads, polling and shutdown, has a race condition on both collections, runnable and running, so those should be guarded by shared something.

Yes, it will go into the wait state quickly. But I haven't understood how the wait helps here. There is no notify in this code, so the wait will always wait for 2 seconds. It will be no different from a sleep(2000) .
So it looks like the polling outside loop will continue until all the currently running jobs are complete.


- Thejas


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


On March 4, 2014, 8:02 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 8:02 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 332cadb 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.

> On Feb. 27, 2014, 11:08 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 110
> > <https://reviews.apache.org/r/15873/diff/3/?file=478815#file478815line110>
> >
> >     When pollFinished is running, this shutdown() function will not be able to make progress. Which means that the query cancellation will happen only after a task (could be an MR task) is complete.
> >     
> >     It seems synchronizing around shutdown should be sufficient, either by making it volatile or having synchronized methods around it.
> >     
> >     Since thread safe concurrent collection classes are being used here, I don't see other concurrency issues that would make it necessary to make all these functions synchronized. 
> >     
> >     
> >
> 
> Navis Ryu wrote:
>     It just only polls status of running tasks and goes into wait state quite quickly, so it would not hinder shutdown process. Furthermore, two threads, polling and shutdown, has a race condition on both collections, runnable and running, so those should be guarded by shared something.
> 
> Thejas Nair wrote:
>     Yes, it will go into the wait state quickly. But I haven't understood how the wait helps here. There is no notify in this code, so the wait will always wait for 2 seconds. It will be no different from a sleep(2000) .
>     So it looks like the polling outside loop will continue until all the currently running jobs are complete.
>
> 
> Navis Ryu wrote:
>     In javadoc, Object.wait()
>     
>     The current thread must own this object's monitor. The thread 
>     releases ownership of this monitor and waits until another thread 
>     notifies threads waiting on this object's monitor
>     
>     In wait state, any other thread can take the monitor (in sleep, it's not possible). So shutdown thread does not need to wait for 2 seconds. Polling thread might notice 2 seconds after shutdown as you said because it's not notified. But I think it's not a big deal. Isn't it?

Thanks for the explanation!


- Thejas


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


On March 4, 2014, 8:02 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 8:02 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 332cadb 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.

> On Feb. 27, 2014, 11:08 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 110
> > <https://reviews.apache.org/r/15873/diff/3/?file=478815#file478815line110>
> >
> >     When pollFinished is running, this shutdown() function will not be able to make progress. Which means that the query cancellation will happen only after a task (could be an MR task) is complete.
> >     
> >     It seems synchronizing around shutdown should be sufficient, either by making it volatile or having synchronized methods around it.
> >     
> >     Since thread safe concurrent collection classes are being used here, I don't see other concurrency issues that would make it necessary to make all these functions synchronized. 
> >     
> >     
> >
> 
> Navis Ryu wrote:
>     It just only polls status of running tasks and goes into wait state quite quickly, so it would not hinder shutdown process. Furthermore, two threads, polling and shutdown, has a race condition on both collections, runnable and running, so those should be guarded by shared something.
> 
> Thejas Nair wrote:
>     Yes, it will go into the wait state quickly. But I haven't understood how the wait helps here. There is no notify in this code, so the wait will always wait for 2 seconds. It will be no different from a sleep(2000) .
>     So it looks like the polling outside loop will continue until all the currently running jobs are complete.
>

In javadoc, Object.wait()

The current thread must own this object's monitor. The thread 
releases ownership of this monitor and waits until another thread 
notifies threads waiting on this object's monitor

In wait state, any other thread can take the monitor (in sleep, it's not possible). So shutdown thread does not need to wait for 2 seconds. Polling thread might notice 2 seconds after shutdown as you said because it's not notified. But I think it's not a big deal. Isn't it? 


- Navis


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


On March 4, 2014, 8:02 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 8:02 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 332cadb 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review35625
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment66425>

    should check shutdown thrown a HiveException that results in a message like  "FAILED: Operation cancelled" ?
    It seems very possible that launching() is called after cancellation. I think better to give the more informative error message.
    



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment66438>

    When pollFinished is running, this shutdown() function will not be able to make progress. Which means that the query cancellation will happen only after a task (could be an MR task) is complete.
    
    It seems synchronizing around shutdown should be sufficient, either by making it volatile or having synchronized methods around it.
    
    Since thread safe concurrent collection classes are being used here, I don't see other concurrency issues that would make it necessary to make all these functions synchronized. 
    
    
    



ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java
<https://reviews.apache.org/r/15873/#comment66260>

    This second addToRunnable(tsk) is redundant.
    


- Thejas Nair


On Feb. 27, 2014, 7:06 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 7:06 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6705ec4 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review36359
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java
<https://reviews.apache.org/r/15873/#comment67310>

    This second addToRunnable(tsk) is redundant.
    


- Thejas Nair


On March 4, 2014, 8:02 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 8:02 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 332cadb 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review36492
-----------------------------------------------------------

Ship it!


Ship It!

- Thejas Nair


On March 7, 2014, 1:17 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 1:17 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 318e21a 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/
-----------------------------------------------------------

(Updated March 7, 2014, 1:17 a.m.)


Review request for hive.


Changes
-------

Rebased to trunk


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


Repository: hive-git


Description
-------

Currently, query canceling does not stop running MR job immediately.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 318e21a 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 

Diff: https://reviews.apache.org/r/15873/diff/


Testing
-------


Thanks,

Navis Ryu


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/
-----------------------------------------------------------

(Updated March 4, 2014, 8:02 a.m.)


Review request for hive.


Changes
-------

Addressed two comments, except one on synchronization.


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


Repository: hive-git


Description
-------

Currently, query canceling does not stop running MR job immediately.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 332cadb 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 

Diff: https://reviews.apache.org/r/15873/diff/


Testing
-------


Thanks,

Navis Ryu


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/
-----------------------------------------------------------

(Updated Feb. 27, 2014, 7:06 a.m.)


Review request for hive.


Changes
-------

Rebased to trunk


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


Repository: hive-git


Description
-------

Currently, query canceling does not stop running MR job immediately.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6705ec4 
  ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java c51a9c8 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ConditionalTask.java 854cd52 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 

Diff: https://reviews.apache.org/r/15873/diff/


Testing
-------


Thanks,

Navis Ryu


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.

> On Feb. 25, 2014, 4:25 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java, line 44
> > <https://reviews.apache.org/r/15873/diff/2/?file=404159#file404159line44>
> >
> >     Thanks for doing this. Its much cleaner with this change!

Thanks. 


- Navis


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


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review35371
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java
<https://reviews.apache.org/r/15873/#comment65853>

    Thanks for doing this. Its much cleaner with this change!


- Thejas Nair


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Navis Ryu <na...@nexr.com>.

> On Jan. 2, 2014, 5:37 p.m., Sergey Shelukhin wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 1347
> > <https://reviews.apache.org/r/15873/diff/2/?file=404158#file404158line1347>
> >
> >     wouldn't this be true in case of many error paths above? 
> >     Where driverCxt.shutdown(); is called

The executor thread throws exception very after calling Driver.shutdown(). This code should be walked only when shutdown is called by other thread.


- Navis


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


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 15873: Query cancel should stop running MR tasks

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15873/#review31039
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/15873/#comment59384>

    wouldn't this be true in case of many error paths above? 
    Where driverCxt.shutdown(); is called


- Sergey Shelukhin


On Dec. 30, 2013, 6:57 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15873/
> -----------------------------------------------------------
> 
> (Updated Dec. 30, 2013, 6:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-5901
>     https://issues.apache.org/jira/browse/HIVE-5901
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Currently, query canceling does not stop running MR job immediately.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 62fc150 
>   ql/src/java/org/apache/hadoop/hive/ql/DriverContext.java 1c84523 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskRunner.java ead7b59 
> 
> Diff: https://reviews.apache.org/r/15873/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>