You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ning Zhang <nz...@fb.com> on 2011/03/03 09:53:49 UTC
Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/460/
-----------------------------------------------------------
Review request for hive.
Summary
-------
define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
Diffs
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1076459
trunk/conf/hive-default.xml 1076459
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1076459
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java 1076459
Diff: https://reviews.apache.org/r/460/diff
Testing
-------
Thanks,
Ning
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by Ning Zhang <nz...@fb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/460/
-----------------------------------------------------------
(Updated 2011-03-03 23:25:14.655675)
Review request for hive.
Changes
-------
MIS's patch using thread pool.
Summary
-------
define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
Diffs (updated)
-----
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1076715
trunk/conf/hive-default.xml 1076715
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1076715
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java 1076715
Diff: https://reviews.apache.org/r/460/diff
Testing
-------
Thanks,
Ning
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by Ning Zhang <nz...@fb.com>.
It looks good to me. Thanks!
On Mar 3, 2011, at 10:18 PM, ಕರಿಯ wrote:
Hi, Ning
1) You are right on this. But, here the keepAliveTime will not be having much of prominence as the core pool size itself is nThreads and the max pool size is also nThreads. So, even if the threads are idle, nThreads will always remain in the pool that is created to process the tasks. Also, since in this scenario the thread pool is being created for a specific purpose thread pool configuration is fine.
This can be achieved in a more simple manner as below:
ExecutorService exeService = Executors.newFixedThreadPool(nThreads);
I'll use this in the new patch.
Further, If we are going to be use (which we need to in future) a common or a centralized thread pool, then the thread pool configuration needs to be carefully arrived at taking into account the number of cores available at our disposal on a particular machine and depending on profiling results, but this is for later.
2) In the current scenario, we need to call execService.shutDown() in any case, if an exception is thrown or otherwise, as it is a local thread pool and we won't be using it any further. If the thread pool were to be a common/centralized one, we need not have to call shutDown().
Please let me know if this is fine, then I'll upload the patch attached with this file in Jira.
Thanks,
ಕರಿಯ
On Fri, Mar 4, 2011 at 12:44 AM, Ning Zhang <nz...@fb.com>> wrote:
Hi MIS,
Thanks for the contribution! To allow broader audience to review, can you upload your patch to the JIRA and the review board (I can help you with the review board if it doesn't allow you to change the request).
A couple of comments before uploading your patch:
1) the 5 sec keepAliveTime seems low. If the # of threads is more than the # of cores, does it mean the thread will be terminated after 5 secs after it is waiting to get scheduled?
2) do you need to call execService.shutDown() in case of a Throwable is caught?
On Mar 3, 2011, at 10:09 AM, MIS wrote:
Hi, Ning
Just to be clear on what I was suggesting, I have created a patch only for this file.
Please have a look.
Thanks,
MIS.
On Thu, Mar 3, 2011 at 5:50 PM, M IS <mi...@gmail.com>> wrote:
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/460/
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java<https://reviews.apache.org/r/460/diff/1/?file=13550#file13550line82> (Diff revision 1)
public void run(SessionState sess, Set<ReadEntity> inputs,
77
Thread[] threads = new Thread[nThreads];
How about going for a centralized thread pool and submitting the tasks for that pool.
This can have advantages like, we need not have to create threads and we could come to know of the status of the task submitted through the future object. And use this future to to wait till the task is finished. We can re factor the code to make UpdateWorker to implement Runnable instead of extending of Thread.
- M
On March 3rd, 2011, 12:53 a.m., Ning Zhang wrote:
Review request for hive.
By Ning Zhang.
Updated 2011-03-03 00:53:49
Description
define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
Diffs
* trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (1076459)
* trunk/conf/hive-default.xml (1076459)
* trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (1076459)
* trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java (1076459)
View Diff<https://reviews.apache.org/r/460/diff/>
<HIVE-2026_1.patch>
<HIVE-2026_2.patch>
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by ಕರಿಯ <mi...@gmail.com>.
Hi, Ning
1) You are right on this. But, here the keepAliveTime will not be having
much of prominence as the core pool size itself is nThreads and the max pool
size is also nThreads. So, even if the threads are idle, nThreads will
always remain in the pool that is created to process the tasks. Also, since
in this scenario the thread pool is being created for a specific purpose
thread pool configuration is fine.
This can be achieved in a more simple manner as below:
ExecutorService exeService = Executors.newFixedThreadPool(nThreads);
I'll use this in the new patch.
Further, If we are going to be use (which we need to in future) a common or
a centralized thread pool, then the thread pool configuration needs to be
carefully arrived at taking into account the number of cores available at
our disposal on a particular machine and depending on profiling results, but
this is for later.
2) In the current scenario, we need to call execService.shutDown() in any
case, if an exception is thrown or otherwise, as it is a local thread pool
and we won't be using it any further. If the thread pool were to be a
common/centralized one, we need not have to call shutDown().
Please let me know if this is fine, then I'll upload the patch attached with
this file in Jira.
Thanks,
ಕರಿಯ
On Fri, Mar 4, 2011 at 12:44 AM, Ning Zhang <nz...@fb.com> wrote:
> Hi MIS,
>
> Thanks for the contribution! To allow broader audience to review, can you
> upload your patch to the JIRA and the review board (I can help you with the
> review board if it doesn't allow you to change the request).
>
> A couple of comments before uploading your patch:
>
> 1) the 5 sec keepAliveTime seems low. If the # of threads is more than
> the # of cores, does it mean the thread will be terminated after 5 secs
> after it is waiting to get scheduled?
>
> 2) do you need to call execService.shutDown() in case of a Throwable is
> caught?
>
> On Mar 3, 2011, at 10:09 AM, MIS wrote:
>
> Hi, Ning
>
> Just to be clear on what I was suggesting, I have created a patch only for
> this file.
> Please have a look.
>
> Thanks,
> MIS.
>
>
> On Thu, Mar 3, 2011 at 5:50 PM, M IS <mi...@gmail.com> wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/460/
>>
>> trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java<https://reviews.apache.org/r/460/diff/1/?file=13550#file13550line82> (Diff
>> revision 1)
>>
>> public void run(SessionState sess, Set<ReadEntity> inputs,
>>
>> 77
>>
>> Thread[] threads = new Thread[nThreads];
>>
>> How about going for a centralized thread pool and submitting the tasks for that pool.
>> This can have advantages like, we need not have to create threads and we could come to know of the status of the task submitted through the future object. And use this future to to wait till the task is finished. We can re factor the code to make UpdateWorker to implement Runnable instead of extending of Thread.
>>
>>
>> - M
>>
>> On March 3rd, 2011, 12:53 a.m., Ning Zhang wrote:
>> Review request for hive.
>> By Ning Zhang.
>>
>> *Updated 2011-03-03 00:53:49*
>> Description
>>
>> define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
>>
>> Diffs
>>
>> - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
>> (1076459)
>> - trunk/conf/hive-default.xml (1076459)
>> - trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>> (1076459)
>> - trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java
>> (1076459)
>>
>> View Diff <https://reviews.apache.org/r/460/diff/>
>>
>
> <HIVE-2026_1.patch>
>
>
>
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by Ning Zhang <nz...@fb.com>.
Hi MIS,
Thanks for the contribution! To allow broader audience to review, can you upload your patch to the JIRA and the review board (I can help you with the review board if it doesn't allow you to change the request).
A couple of comments before uploading your patch:
1) the 5 sec keepAliveTime seems low. If the # of threads is more than the # of cores, does it mean the thread will be terminated after 5 secs after it is waiting to get scheduled?
2) do you need to call execService.shutDown() in case of a Throwable is caught?
On Mar 3, 2011, at 10:09 AM, MIS wrote:
Hi, Ning
Just to be clear on what I was suggesting, I have created a patch only for this file.
Please have a look.
Thanks,
MIS.
On Thu, Mar 3, 2011 at 5:50 PM, M IS <mi...@gmail.com>> wrote:
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/460/
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java<https://reviews.apache.org/r/460/diff/1/?file=13550#file13550line82> (Diff revision 1)
public void run(SessionState sess, Set<ReadEntity> inputs,
77
Thread[] threads = new Thread[nThreads];
How about going for a centralized thread pool and submitting the tasks for that pool.
This can have advantages like, we need not have to create threads and we could come to know of the status of the task submitted through the future object. And use this future to to wait till the task is finished. We can re factor the code to make UpdateWorker to implement Runnable instead of extending of Thread.
- M
On March 3rd, 2011, 12:53 a.m., Ning Zhang wrote:
Review request for hive.
By Ning Zhang.
Updated 2011-03-03 00:53:49
Description
define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
Diffs
* trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (1076459)
* trunk/conf/hive-default.xml (1076459)
* trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (1076459)
* trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java (1076459)
View Diff<https://reviews.apache.org/r/460/diff/>
<HIVE-2026_1.patch>
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by MIS <mi...@gmail.com>.
Hi, Ning
Just to be clear on what I was suggesting, I have created a patch only for
this file.
Please have a look.
Thanks,
MIS.
On Thu, Mar 3, 2011 at 5:50 PM, M IS <mi...@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/460/
>
> trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java<https://reviews.apache.org/r/460/diff/1/?file=13550#file13550line82> (Diff
> revision 1)
>
> public void run(SessionState sess, Set<ReadEntity> inputs,
>
> 77
>
> Thread[] threads = new Thread[nThreads];
>
> How about going for a centralized thread pool and submitting the tasks for that pool.
> This can have advantages like, we need not have to create threads and we could come to know of the status of the task submitted through the future object. And use this future to to wait till the task is finished. We can re factor the code to make UpdateWorker to implement Runnable instead of extending of Thread.
>
>
> - M
>
> On March 3rd, 2011, 12:53 a.m., Ning Zhang wrote:
> Review request for hive.
> By Ning Zhang.
>
> *Updated 2011-03-03 00:53:49*
> Description
>
> define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
>
> Diffs
>
> - trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
> (1076459)
> - trunk/conf/hive-default.xml (1076459)
> - trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> (1076459)
> - trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java
> (1076459)
>
> View Diff <https://reviews.apache.org/r/460/diff/>
>
Re: Review Request: HIVE-2026. Parallelize UpdateInputAccessTimeHook
Posted by M IS <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/460/#review283
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java
<https://reviews.apache.org/r/460/#comment534>
How about going for a centralized thread pool and submitting the tasks for that pool.
This can have advantages like, we need not have to create threads and we could come to know of the status of the task submitted through the future object. And use this future to to wait till the task is finished. We can re factor the code to make UpdateWorker to implement Runnable instead of extending of Thread.
- M
On 2011-03-03 00:53:49, Ning Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/460/
> -----------------------------------------------------------
>
> (Updated 2011-03-03 00:53:49)
>
>
> Review request for hive.
>
>
> Summary
> -------
>
> define hive.hooks.parallel.degree to control max # of thread to update metastore in parallel.
>
>
> Diffs
> -----
>
> trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1076459
> trunk/conf/hive-default.xml 1076459
> trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 1076459
> trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/UpdateInputAccessTimeHook.java 1076459
>
> Diff: https://reviews.apache.org/r/460/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ning
>
>