> On Nov. 13, 2014, 10:55 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 1573 > > <https://reviews.apache.org/r/27846/diff/2/?file=762600#file762600line1573> > > > > Make this configurable? Just thinking, does it make sense to have this > > parameter be in time units? So that it is deterministic how often this is > > called? > > Mike Yoder wrote: > I first thought about time units - but the thing with that is we could be > running that query over and over again without anything for it to do. We > know when privs are potentially being orphaned, and when we think there might > be some number of them we can clean them up. The only goal is to prevent > "too many" of them lying around in the db. If, say, 9 of them hang out for a > long time, that's really no harm done. > > As for making it configurable... I think that's probably (hopefully?) not > a useful knob for a user to turn; how exactly would we advise them to tune > it, and under what circumstances? We'd also need to test a variety of > different settings of this config value. > > Prior to Cloudera, I focused on *removing* unnecessary tunables from > customer-facing interfaces. :-) Hadoop is 180 degrees opposite of that > thinking, I'll grant you. > > Nonetheless, you've got more experience than I in this area and if you > want me to, I'll put it in. It might in theory be useful to have around if > there's a bug in the thread and the config could also shut off the thread. > Let me know.
Yeah, I see that. Many times making some things configurable can do more bad than good :-). I am only worried that value 10 might be too low, that this is fired too often and could cause performance regression. As far as I understand, having orphaned privileges is bad for following reasons: 1. Slight performance hit in sql queries going on SENTRY_DB_PRIVILEGE, this might not be too bad unless our ratio of orphans is pretty high? 2. Increased unnecessary memory foot print for services which cache this data (Impala and Namenode). I am not entirely sure if they handle orphans actually. So if you think of it, it is not too bad. But if the user is granting/revoking bulk privileges, this clean up might be called multiple times successively which can clog the sentry service? May be threshold around 50/100 would be safe? - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27846/#review61323 ----------------------------------------------------------- On Nov. 14, 2014, 7:11 p.m., Mike Yoder wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27846/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2014, 7:11 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-140 > https://issues.apache.org/jira/browse/SENTRY-140 > > > Repository: sentry > > > Description > ------- > > Due to the M-to-N relationship of roles to privileges, when a role is removed > from a privilege (or vice versa) that privilege is not deleted from the > database. > > This fix introduces a thread that periodically (currently, once every 10 > times a role and a privilege are disassociated) runs a SQL query that > searches for all orphaned privileges and deletes them. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f6699d2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 2e829d6 > > Diff: https://reviews.apache.org/r/27846/diff/ > > > Testing > ------- > > Wrote a test case; local testing. > > > Thanks, > > Mike Yoder > >
