> On Jan. 5, 2017, 8:34 p.m., Vamsee Yarlagadda wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java, > > line 504 > > <https://reviews.apache.org/r/54947/diff/1/?file=1590559#file1590559line504> > > > > Just wondering, it makes perfect sense to call pm.deletePersistent for > > the privileges that have empty roles but for the else clause, we seem to > > call pm.makePersistent. Is this really required? - The updated privilege is > > already saved right? If it is not, where are they saving the privilege data > > into the database before this change? > > kalyan kumar kalvagadda wrote: > Logic is writtern in makePersistent function of MSentryGMPrivilege which > is called when a privilege is added/deleted. > > I'm replacing "pm.makePersistent(MSentryGMPrivilege)" function call with > the above function so it has to handle both the cases.
Thanks for clarifying. It makes sense after your explanation. - Vamsee ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54947/#review160626 ----------------------------------------------------------- On Jan. 5, 2017, 4:20 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54947/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2017, 4:20 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, > and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > SENTRY-1556 Simplify privilege cleaning > > I made changes for the first approach by removing privileges moment they are > not associated to any role. > I have identified below scenarios which this will happen > When a role is deleted, we can see if the associated privileges are > associated to any other roles. All the privileges that are not associated to > any roles can be deleted from storage > When a privilege is revoked for a role, we can remove the privilege from > storage if it is not associated to any role > > Note: Once this approached is reviewed and accepted, we need not call > PrivCleaner periodically for cleanup > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java > 12245ec6d1b28863e4c1fd7b265e60204732f0e6 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java > 55b61ac07c6865402ffe4d0ff1690afb435deb95 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 4c3af7992c90ba6ce33ff38ca6c5a3eb3492dd8b > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryRole.java > 0484eaade5853d13bc6671c6e0648c14f76116c0 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryPrivilege.java > PRE-CREATION > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestSentryRole.java > 29134fec58e479e79aefb6cf96c6261698d64c08 > > Diff: https://reviews.apache.org/r/54947/diff/ > > > Testing > ------- > > Added unit tests the scenarios mentioned in description. > Also tested the same with Sentry thrift client. > > > Thanks, > > kalyan kumar kalvagadda > >
