You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arun Suresh <ar...@gmail.com> on 2015/04/13 17:50:35 UTC
Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/
-----------------------------------------------------------
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
Repository: sentry
Description
-------
SENTRY-696 : Improve MetastorePlugin cache initialization latency
This path introduces the following changes :
1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
2) The Cache initialization itself will now be performed in parallel
- It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
Diffs
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
Diff: https://reviews.apache.org/r/33134/diff/
Testing
-------
Thanks,
Arun Suresh
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/#review79915
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129508>
Nit: Rename queueFlushConplete to queueFlushComplete
- Prasad Mujumdar
On April 13, 2015, 4:10 p.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33134/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:10 p.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-696 : Improve MetastorePlugin cache initialization latency
>
> This path introduces the following changes :
> 1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
> 2) The Cache initialization itself will now be performed in parallel
> - It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
>
> Diff: https://reviews.apache.org/r/33134/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/#review79906
-----------------------------------------------------------
Nice work!
Looks mostly fine. A couple of possible concurrency related comments below.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
<https://reviews.apache.org/r/33134/#comment129500>
I guess addToAddPaths() needs to be made thread safe ?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
<https://reviews.apache.org/r/33134/#comment129501>
Result list itself is being changed from the dbTask. Should it be a thread safe queue that's polled for remaining tasks ?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129502>
The finally block is always setting syncSent to true. Should it be unset in this case ?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129503>
Perhaps print a warning that the HDFS ACLs won't be in sync till the sync is completed ?
- Prasad Mujumdar
On April 13, 2015, 4:10 p.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33134/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:10 p.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-696 : Improve MetastorePlugin cache initialization latency
>
> This path introduces the following changes :
> 1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
> 2) The Cache initialization itself will now be performed in parallel
> - It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
>
> Diff: https://reviews.apache.org/r/33134/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/#review79930
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
<https://reviews.apache.org/r/33134/#comment129561>
Yup... will do
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
<https://reviews.apache.org/r/33134/#comment129562>
agreed..
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129563>
yup... has to be moved out of the try block..
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129565>
done
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java
<https://reviews.apache.org/r/33134/#comment129568>
will do
- Arun Suresh
On April 13, 2015, 4:10 p.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33134/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 4:10 p.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-696 : Improve MetastorePlugin cache initialization latency
>
> This path introduces the following changes :
> 1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
> 2) The Cache initialization itself will now be performed in parallel
> - It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
>
> Diff: https://reviews.apache.org/r/33134/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Yibing Shi <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/#review79913
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java
<https://reviews.apache.org/r/33134/#comment129506>
Why do we use the DB location if the table deosn't have a location? Method get_all_tables also returns views, which don't have any location at all. Should we just ignore those tables that don't have any storage/location?
- Yibing Shi
On April 13, 2015, 7:57 p.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33134/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 7:57 p.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-696 : Improve MetastorePlugin cache initialization latency
>
> This path introduces the following changes :
> 1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
> 2) The Cache initialization itself will now be performed in parallel
> - It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
> sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml f35baf4
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
>
> Diff: https://reviews.apache.org/r/33134/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/#review80570
-----------------------------------------------------------
Ship it!
- Prasad Mujumdar
On April 14, 2015, 8:13 a.m., Arun Suresh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33134/
> -----------------------------------------------------------
>
> (Updated April 14, 2015, 8:13 a.m.)
>
>
> Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> SENTRY-696 : Improve MetastorePlugin cache initialization latency
>
> This path introduces the following changes :
> 1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
> 2) The Cache initialization itself will now be performed in parallel
> - It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
> sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml f35baf4
> sentry-hdfs/sentry-hdfs-service/pom.xml 4d65edf
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33134/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arun Suresh
>
>
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/
-----------------------------------------------------------
(Updated April 14, 2015, 8:13 a.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
Changes
-------
* Minor refactoring
* Incorporating review feedback
Repository: sentry
Description
-------
SENTRY-696 : Improve MetastorePlugin cache initialization latency
This path introduces the following changes :
1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
2) The Cache initialization itself will now be performed in parallel
- It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml f35baf4
sentry-hdfs/sentry-hdfs-service/pom.xml 4d65edf
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java PRE-CREATION
Diff: https://reviews.apache.org/r/33134/diff/
Testing
-------
Thanks,
Arun Suresh
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/
-----------------------------------------------------------
(Updated April 14, 2015, 1:19 a.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
Changes
-------
* Added TestCases
* Minor bugfixes
Repository: sentry
Description
-------
SENTRY-696 : Improve MetastorePlugin cache initialization latency
This path introduces the following changes :
1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
2) The Cache initialization itself will now be performed in parallel
- It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml f35baf4
sentry-hdfs/sentry-hdfs-service/pom.xml 4d65edf
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestMetastoreCacheInitializer.java PRE-CREATION
Diff: https://reviews.apache.org/r/33134/diff/
Testing
-------
Thanks,
Arun Suresh
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/
-----------------------------------------------------------
(Updated April 13, 2015, 7:57 p.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
Changes
-------
* Incorporated Prasad's review feedback
* Increased parallelism in CacheInitializer by spreading the partition requests across multiple calls
* Allows users to configure the max tables and max partitions per call
Repository: sentry
Description
-------
SENTRY-696 : Improve MetastorePlugin cache initialization latency
This path introduces the following changes :
1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
2) The Cache initialization itself will now be performed in parallel
- It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
sentry-hdfs/sentry-hdfs-namenode-plugin/pom.xml f35baf4
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
Diff: https://reviews.apache.org/r/33134/diff/
Testing
-------
Thanks,
Arun Suresh
Re: Review Request 33134: SENTRY-696 : Improve MetastorePlugin cache
initialization latency
Posted by Arun Suresh <ar...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33134/
-----------------------------------------------------------
(Updated April 13, 2015, 4:10 p.m.)
Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Yibing Shi.
Changes
-------
minor fixes
Repository: sentry
Description
-------
SENTRY-696 : Improve MetastorePlugin cache initialization latency
This path introduces the following changes :
1) Exposes a config variable to allow cache initialization to happen asynchronously (by default false)
2) The Cache initialization itself will now be performed in parallel
- It introduces a separate thread pool and attempts to extract DB/Table/Partition information as separate threadpool tasks
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ServiceConstants.java 489d165
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java PRE-CREATION
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java 7106e74
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java ee5e0f9
Diff: https://reviews.apache.org/r/33134/diff/
Testing
-------
Thanks,
Arun Suresh