> On June 11, 2018, 2:57 p.m., Sergio Pena wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > > Lines 70-74 (patched) > > <https://reviews.apache.org/r/67506/diff/1/?file=2036472#file2036472line70> > > > > I don't feel necessary to do this loop to translate OWNER privileges > > prior to sending them to HDFS. HDFS can know what to do with an OWNER > > privilege the same it does with the ALL privilege in the > > UpdateableAuthzPermissions used in the binding side (that's one single line > > of code). > > > > I thought about what you said what if in the future we add a > > configuration for what OWNER will mean. If we do that, which I don't think > > will happen because we're behaving like other DBs where OWNER = ALL, but if > > we do, then this change in HDFS will not be affected because but that time > > Sentry will not send the OWNER anymore, but the new privilege used in the > > configuration. > > > > I prefer to translate the OWNER privilege in the NN side instead of > > adding another loop that walks through all the privileges send to the NN. > > This is adding an extra overhead on the updates for the NN.
I had to add a loop just to be sure. Ideally there will be only one permission change in permsUpdate. It is be one entry either in add privilege/delete privileges. Except for rename, there will be one entry in each. There is no overhead because of this logic, when a snapshot is taken the translation in in-line with the snapshot creation. For delta updates, each entry when it is extracted from database and de-serialized, translation is done. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67506/#review204543 ----------------------------------------------------------- On June 8, 2018, 12:26 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67506/ > ----------------------------------------------------------- > > (Updated June 8, 2018, 12:26 p.m.) > > > Review request for sentry, Na Li and Sergio Pena. > > > Bugs: SENTRY-2260 > https://issues.apache.org/jira/browse/SENTRY-2260 > > > Repository: sentry > > > Description > ------- > > When owner privileges are implicitly granted/revoked, sentry ACL's in > Namenode plug-in should be updated accordingly. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java > 71ef5f9251f182825177c00a8456ca3166b2d095 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermDeltaRetriever.java > 6974d37aac9cdcbc21edfe66b1794dd1df6597fb > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > 10d52b43729426c53c0168fc0b7d0cdf0e307b57 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDeltaRetriever.java > 60696cccc0d679dfa6b3e6c06cb5120942facc14 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > d2d5391182d0da395562ad23f742d704d9a0ceb5 > > > Diff: https://reviews.apache.org/r/67506/diff/1/ > > > Testing > ------- > > Added new unit tests to verify the functionality. > > > Thanks, > > kalyan kumar kalvagadda > >
