> 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?
why do you think it is HMSSnapshot? It is a general function to execute the transaction block. It could be for permission transaction. > On July 28, 2017, 1:21 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 120 (patched) > > <https://reviews.apache.org/r/61047/diff/3/?file=1784512#file1784512line120> > > > > Why can't it be stack variable for shouldEnableHMSFollower? It is not > > used any where. Make it final prevent its value changing. It is used by shouldEnableHMSFollower(), which is used in two places. If it is stack variable, need to get it twice from configuration. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61047/#review181660 ----------------------------------------------------------- On July 27, 2017, 3:57 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61047/ > ----------------------------------------------------------- > > (Updated July 27, 2017, 3:57 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/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 > > > Diff: https://reviews.apache.org/r/61047/diff/3/ > > > Testing > ------- > > unit tests > > > Thanks, > > Na Li > >
