> On March 6, 2017, 9:38 p.m., Lei Xu wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > > Lines 72 (patched) > > <https://reviews.apache.org/r/55706/diff/4/?file=1654008#file1654008line72> > > > > It seems that `tPermUpdate` is using `curSeqNum`. > > What prevent us to use `curSeqNum` here for `permissionsUpdate`? At > > lease in this patch, these two seq number are not the same, IIUC?
Yeah, there are two seq number here: curSeqNum and seqNum. curSeqNum is read from database and seqNum is passed down. Because in this patch we have not removed the logic of depending on the in-memory cache as the synchronization mechanism to NN yet, so we still give the old seqNum here. As TODO points out we will change it use curSeqNum once we removed the in-memory cache. > On March 6, 2017, 9:38 p.m., Lei Xu wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2369 (patched) > > <https://reviews.apache.org/r/55706/diff/4/?file=1654016#file1654016line2379> > > > > Will `query.execute()` return `null`? > > > > > > http://www.datanucleus.org/javadocs/javax.jdo/3.2/javax/jdo/Query.html#execute-- See the comment below. - Hao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55706/#review168025 ----------------------------------------------------------- On March 2, 2017, 9:43 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55706/ > ----------------------------------------------------------- > > (Updated March 2, 2017, 9:43 p.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Read full permission and path snapshot from SentryDB and make the update > available for NN plugin upon requests. > In detail: > 1. Added Path/PermissionImage classes to represent corresponding > Path/Permission snapshot read from DB. > 2. Refactor full snapshot retriever APIs in SentryStore to become a single > transaction to ensure snapshot consistency. > 3. Added Path/PermissionImageRetriever classes to retrieve > Path/PermissionImage from DB and convert to corresponding > Path/PermissionUpdate, which later would be consumed by NN plugin. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 992c8b731810297be847e3b802698332aefe8f8f > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > PRE-CREATION > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHdfsMetricsUtil.java > e68c7080cec007e3cdf2c89efb9701eef9cca0a0 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > f68f690436301c79e93b0742996dda25a750c0c3 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java > 6d5c607273bb08597780b655d7b59cd41f0844bb > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateablePermissions.java > fe2baa6a446874185e8344bb16d76d803826d1f3 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestUpdateForwarder.java > 0f0d0a743d5a5f9f1678ac7c5217b9a27537e85b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > c1186ba405a05be70336e9169a1454208df016ca > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 91f15c0f3999a25a20dfb45f2f28abbda54366f4 > > > Diff: https://reviews.apache.org/r/55706/diff/4/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
