> On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java > > Lines 32 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623194#file1623194line32> > > > > What is the semantics of this call in terms of seqNum?
This is only for code backward compatibility. In SENTRY-1613 will remove seqNum as the seqNum is coming from DB. > On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > > Lines 157 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623195#file1623195line158> > > > > These seem to be very generic - can we have them in some common Sentry > > code? I have moved those code to SENTRY-1612 where would be more appropriate to include. > On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > > Lines 38 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623196#file1623196line38> > > > > With the current design - do we support retrieving full image with any > > seqNum? Why do we need to pass it to the retriever? Aren't we going to > > retrieve the latest snapshot? As mentioned above. This is only for code backward compatibility. In SENTRY-1613 will remove seqNum as the seqNum is coming from DB. > On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > > Lines 46 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623196#file1623196line46> > > > > This isn't used anywhere - but may be it should be used on line 51? You can check the TODO in line 49, it will be used in SENTRY-1613. > On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java > > Lines 51 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623196#file1623196line51> > > > > should it be seqNum or curSeqNum? This is mentioned in TODO, so I guess > > this is the plan? Yeah, it is the plan to use curSeqNum in SENTRY-1613 > On Feb. 7, 2017, 8:04 a.m., Alexander Kolbasov wrote: > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java > > Lines 56 (patched) > > <https://reviews.apache.org/r/55706/diff/2/?file=1623197#file1623197line56> > > > > Why do we construct it with curSeqNum but later in line 75 change it to > > seqNum? As mentioned above it is for code backward compatibilty. Will compelete remove seqNum in SENTRY-1613. - Hao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55706/#review164467 ----------------------------------------------------------- On Feb. 3, 2017, 7:53 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55706/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2017, 7:53 a.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 > ffb07560d7c40fb14cef77950012c2c81fa3fd28 > > 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 > 9b54db171d531d4a4d1335aa93a3d3122d55ce9b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > dfaac1555fe729efab3f8fdccb914978001fee56 > > > Diff: https://reviews.apache.org/r/55706/diff/2/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
