> On Feb. 23, 2017, 4:43 p.m., Alexander Kolbasov wrote: > > I would suggest a simple approach: > > > > 1) Remove all knowledge about the priv cleaner from SentryStore itself. > > 2) Priv cleaner needs SentryStore for DB operations, so we should create > > one in SentryService - it will be shared between the cleaner and > > HMSFollower and later we can remove one from the service register() method > > as well. > > 3) Start the periodic process in SentryService itself. That's where we > > start other things like HMSFollower. > > > > I think this will make things quite simple and minimize the number of files > > that are touched by this change.
I updated the patch to move `SentryStore` creation to `SentryService`. However, the major changes, in terms of the number of files, are the `ProcessorFactory#register()`, which is required to pass the sentry store to the thrift processor. I also changed HMSFollower to use the sentry store passed in. However, it makes testing more difficult. I feel that the state of the two delta tables should be private states of `SentryStore`. The code itself is very simple now. Do you think a test is needed? - Lei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56952/#review166622 ----------------------------------------------------------- On Feb. 24, 2017, 2:47 p.m., Lei Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56952/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2017, 2:47 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-1637 > https://issues.apache.org/jira/browse/SENTRY-1637 > > > Repository: sentry > > > Description > ------- > > * Add configuration keys: SENTRY_STORE_DELTA_REMOVAL_PERIOD_SECONDS to enable > the background clean thread. > * Use ```ScheduledExecutorSerivce``` to periodically involke > ```SentryStore.purgeDeltaChanges()``` > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessorFactory.java > db55b5aa33cebbef235cceca6ccda48603da2a26 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorFactory.java > 1cce1fc4b9157c76c68b35a292263c5e96270bd2 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 7fc3ca8bc2444f6cfcbcbabaebf0d3e18ef3d209 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java > 691c1fb81cd50b6bf671576b3bba69aff291c008 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > c91051db70a5f606980fb29f780fbc199945e4f3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java > a3bb6ab19de35d3018f1e9a938936b22d5aecb48 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > e6021f182e26f7b15773d64d84263c6da586d7a9 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 806d03e81a3660a30c6513efbddd2a1610359fc1 > > Diff: https://reviews.apache.org/r/56952/diff/ > > > Testing > ------- > > mvn test -Dtest=TestSentryStore > > > Thanks, > > Lei Xu > >
