> On 2011-03-14 03:13:45, Siying Dong wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1600
> > <https://reviews.apache.org/r/491/diff/1/?file=13888#file13888line1600>
> >
> >     Do we need it if we do executor.shutdown()?

If we use the futures for the tasks submitted we will have better control over 
each of the task.
The end result that is obtained is still the same, where we await till all the 
submitted tasks get over.


> On 2011-03-14 03:13:45, Siying Dong wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1609
> > <https://reviews.apache.org/r/491/diff/1/?file=13888#file13888line1609>
> >
> >     Here is the desciption of shutdown():
> >     
> >     "void shutdown()Initiates an orderly shutdown in which previously 
> > submitted tasks are executed, but no new tasks will be accepted. Invocation 
> > has no additional effect if already shut down."
> >     
> >     "An ExecutorService can be shut down, which will cause it to reject new 
> > tasks. Two different methods are provided for shutting down an 
> > ExecutorService. The shutdown() method will allow previously submitted 
> > tasks to execute before terminating, while the shutdownNow() method 
> > prevents waiting tasks from starting and attempts to stop currently 
> > executing tasks."
> >     
> >     All submitted jobs should guarantee to finish.

There is a mistake in my comment, which was due to my understanding that tasks 
submitted but not yet started will not run if executor service is shutdown. 
Thanks for rectifying the same.


> On 2011-03-14 03:13:45, Siying Dong wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1563
> > <https://reviews.apache.org/r/491/diff/1/?file=13888#file13888line1563>
> >
> >     Only when we make it final, different threads can access this value 
> > exclusively. I'm not sure the best way to do that. Do you have better 
> > suggestion?

>>Only when we make it final, different threads can access this value 
>>exclusively.
I beg to differ. Here the variables had to be made final so as to make them 
accessible from the anonymous inner class implementation of the Runnable.
A better way would have been to create a class that implements Runnable and 
provide a parameterized constructor that accepts the values that are being used 
in the implementation.


> On 2011-03-14 03:13:45, Siying Dong wrote:
> > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 1554
> > <https://reviews.apache.org/r/491/diff/1/?file=13888#file13888line1554>
> >
> >     What's the benefit of it?

See, in the for loop from lines 1596-1599, we check if the executor is null or 
not. We can avoid this check and instead directly submit the task to the 
executor service, if we choose to initialize the executor service with a single 
thread pool executor in the event the condition at line 1554 i.e., if 
(pathNeedProcess.size() > 1) fails. 
Lets consider the case where we had to go with single thread pool executor. 
This will vary from the current implementation in that the we choose to run the 
tasks in a different thread, but all those tasks will run sequentially in a 
single thread, so the functionality should not be affected.


- M


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


On 2011-03-11 15:03:34, Carl Steinbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/491/
> -----------------------------------------------------------
> 
> (Updated 2011-03-11 15:03:34)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Review request for HIVE-2051.
> 
> 
> This addresses bug HIVE-2051.
>     https://issues.apache.org/jira/browse/HIVE-2051
> 
> 
> Diffs
> -----
> 
>   trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1080771 
> 
> Diff: https://reviews.apache.org/r/491/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Carl
> 
>

Reply via email to