> On July 28, 2017, 1:21 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 4100 (original), 4104 (patched)
> > <https://reviews.apache.org/r/61047/diff/3/?file=1784510#file1784510line4106>
> >
> >     I see that you are trying to create a HMS snapshot and updating the HMS 
> > snapshot on notifications from HMS, even when HMSFollower feature is 
> > disabled.
> >     
> >     Why should we do it?
> 
> Na Li wrote:
>     why do you think it is HMSSnapshot? It is a general function to execute 
> the transaction block. It could be for permission transaction.
> 
> kalyan kumar kalvagadda wrote:
>     You are right, It could be general. But this change doesn't stop 
> create/update of HMS snapshot.

HMSFollower is updated to not persist full snapshot


- Na


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


On July 28, 2017, 6:36 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61047/
> -----------------------------------------------------------
> 
> (Updated July 28, 2017, 6:36 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, 
> Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Currently, Sentry does not start HMSFollower when HDFS sync is disabled. This 
> breaks PERM sync. For example, when a table/database is dropped, its 
> permission should be removed when perm sync is enabled. Without HMSFollower, 
> Sentry does not know when table/database is dropped, and therefore, cannot 
> remove the permission accordingly.
> Sentry needs to make updates to have the follow behavior
> 1) When HDFS sync is disabled + PERM sync is disabled, do not start 
> HMSFollower
> 2) when HDFS sync is enabled or PERM sync is enabled, start HMSFollower 
> (Sentry change).
> 3) when HDFS sync is enabled, we save path change and perm change. nothing 
> more. 
> 4) when perm sync is enabled, we update perm when table/database is created, 
> dropped or altered, nothing more.
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  4cb46ab 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  670bc5e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAdminServlet.java
>  8a8bbd3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  9e8e0e7 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  f631869 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  10d55dc 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
>  5826766 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  82f600b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  a8ebf7c 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreImportExport.java
>  1c3a4f2 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java
>  a8e8a03 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  9b31b3c 
> 
> 
> Diff: https://reviews.apache.org/r/61047/diff/4/
> 
> 
> Testing
> -------
> 
> unit tests
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to