> 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. > > kalyan kumar kalvagadda wrote: > 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.
Since Name Node does not need owner privilege in any way, there is no benefit to expose it to sentry client at Name Node. We can just translate it at sentry server. The performance impact should be small. - Na ----------------------------------------------------------- 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 > >
