You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2017/01/19 06:40:34 UTC
Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/
-----------------------------------------------------------
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.
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 9ecd9e4d9a862804eab26594c28aa691b89947aa
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 47c9f9dd7105ba5440a81ace9292d730df26f3cd
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 0712e2c9246ba7fe0c81d42861628c60eee2cfa0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java a35c8d7dde485cf46d61968a211d1dbb6d9d6076
Diff: https://reviews.apache.org/r/55706/diff/
Testing
-------
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> 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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review164467
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java (line 23)
<https://reviews.apache.org/r/55706/#comment236206>
This is a very vague interface. The only supported method (if you ignore the name) is a map from long ID to any object of type K, so it is way too generic and has nothing to do with image retrieval.
Can we restrict K to be at least a class extending Update?
It seems that it would be good to consider having Full Update as a subclass of Update, but this can be done outside the scope of this change.
In any case please provide more documentation of what implementors of this interface are supposed to actually implement.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java (line 32)
<https://reviews.apache.org/r/55706/#comment236218>
What is the semantics of this call in terms of seqNum?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 157)
<https://reviews.apache.org/r/55706/#comment236212>
These seem to be very generic - can we have them in some common Sentry code?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 160)
<https://reviews.apache.org/r/55706/#comment236208>
What is paths?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 161)
<https://reviews.apache.org/r/55706/#comment236209>
What does it return?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 163)
<https://reviews.apache.org/r/55706/#comment236207>
Can we relax the arg to be Iterable?
Also name - something like joinPath or pathJoin seems to be better
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 169)
<https://reviews.apache.org/r/55706/#comment236210>
What is path?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java (line 171)
<https://reviews.apache.org/r/55706/#comment236211>
What does it return?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java (line 38)
<https://reviews.apache.org/r/55706/#comment236215>
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?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java (line 42)
<https://reviews.apache.org/r/55706/#comment236213>
There is already call to time() above in the try statement
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java (line 43)
<https://reviews.apache.org/r/55706/#comment236214>
May be just
// Read the most up-to-date snapshot of Sentry paths information.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java (line 46)
<https://reviews.apache.org/r/55706/#comment236216>
This isn't used anywhere - but may be it should be used on line 51?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java (line 51)
<https://reviews.apache.org/r/55706/#comment236217>
should it be seqNum or curSeqNum? This is mentioned in TODO, so I guess this is the plan?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java (line 44)
<https://reviews.apache.org/r/55706/#comment236220>
This is alreday done on line 42
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java (line 46)
<https://reviews.apache.org/r/55706/#comment236221>
See comment for PathImageRetriever
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java (line 56)
<https://reviews.apache.org/r/55706/#comment236222>
Why do we construct it with curSeqNum but later in line 75 change it to seqNum?
- Alexander Kolbasov
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/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hao Hao
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review167617
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
Lines 157 (patched)
<https://reviews.apache.org/r/55706/#comment239516>
I have moved those code to SENTRY-1612
- Hao Hao
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review167631
-----------------------------------------------------------
- Hao Hao
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On Feb. 7, 2017, 2:28 p.m., Lei Xu wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 2239 (original), 2291 (patched)
> > <https://reviews.apache.org/r/55706/diff/2/?file=1623205#file1623205line2300>
> >
> > `pm` is not used?
It is used in line 2393. Am I missing anything?
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review164454
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On Feb. 7, 2017, 2:28 p.m., Lei Xu wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
> > Line 74 (original), 68 (patched)
> > <https://reviews.apache.org/r/55706/diff/2/?file=1623200#file1623200line74>
> >
> > Could you add comments for the parameters to these constructors?
> >
> > Especially, what is the purpose of `shouldInit` flag.
Those code is just for backward compatible with old design. this class will be completely removed in SENTRY-1613. Would you mind I do it in that jira if needed?
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review164454
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Lei Xu <le...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review164454
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java (line 68)
<https://reviews.apache.org/r/55706/#comment236179>
Could you add comments for the parameters to these constructors?
Especially, what is the purpose of `shouldInit` flag.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java (line 31)
<https://reviews.apache.org/r/55706/#comment236180>
Add `final` to these fields.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2291)
<https://reviews.apache.org/r/55706/#comment236272>
`pm` is not used?
- Lei Xu
On Feb. 2, 2017, 11:53 p.m., Hao Hao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55706/
> -----------------------------------------------------------
>
> (Updated Feb. 2, 2017, 11:53 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 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/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hao Hao
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On March 2, 2017, 12:33 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2180 (patched)
> > <https://reviews.apache.org/r/55706/diff/2/?file=1623205#file1623205line2180>
> >
> > I am not quite follow this comment. Are you saying that when HDFS sync is disabled, the changeId is always 0?
Yes, to be more accurate if Sentryplugin is disabled. Then the changeID for perm delta would always be 0. As we are monitoring the delta change in Sentryplugin class.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review165043
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On March 2, 2017, 12:33 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 2212 (original), 2243 (patched)
> > <https://reviews.apache.org/r/55706/diff/2/?file=1623205#file1623205line2247>
> >
> > This is Ok, but this class can be simplified a bit:
> >
> > You can get all roles with getAllRoles() call. You know that all roles are unique.
> >
> > So you get something along the lines of:
> >
> > roles = getAllRoles
> > for(role: roles) {
> > retval.put(role.getRoleName()) = roleGroups(role)
> > }
> >
> > where roleFroups returns names of groups belonging to a role.
One thing with the simplification is could it be a role that dose not associate with any groups? It looks like alterSentryRoleDeleteGroups does not delete a role if no groups associated with it. Until this gets changed, I do not think we can do in the simplification way.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review165043
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review165043
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2180 (patched)
<https://reviews.apache.org/r/55706/#comment236880>
I am not quite follow this comment. Are you saying that when HDFS sync is disabled, the changeId is always 0?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2212 (original), 2243 (patched)
<https://reviews.apache.org/r/55706/#comment236881>
This is Ok, but this class can be simplified a bit:
You can get all roles with getAllRoles() call. You know that all roles are unique.
So you get something along the lines of:
roles = getAllRoles
for(role: roles) {
retval.put(role.getRoleName()) = roleGroups(role)
}
where roleFroups returns names of groups belonging to a role.
- Alexander Kolbasov
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review167629
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2212 (original), 2243 (patched)
<https://reviews.apache.org/r/55706/#comment239535>
One thing with the simplification is could it be a role that dose not associate with any groups? It looks like alterSentryRoleDeleteGroups does not delete a role if no groups associated with it. Until this gets changed, I do not think we can do in the simplification way.
- Hao Hao
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On March 7, 2017, 1:58 a.m., Alexander Kolbasov 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>
> >
> > query.execute() will not return in this context. It only returns null when a single object is returned. Otherwise it returns an empty collection.
Yeah, according to http://www.datanucleus.org/products/datanucleus/jdo/query.html. It could return null for case when 'query.setUnique(true);' is used. I will remove this.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168077
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168077
-----------------------------------------------------------
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/#comment240201>
query.execute() will not return in this context. It only returns null when a single object is returned. Otherwise it returns an empty collection.
- Alexander Kolbasov
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168556
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On March 9, 2017, 11:43 p.m., Hao Hao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55706/
> -----------------------------------------------------------
>
> (Updated March 9, 2017, 11: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 a5a835e7d463e4de95ef537f76f2872105278c84
> 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/5/
>
>
> Testing
> -------
>
> Unit test added in TestSentryStore. End to end can be peroformed after following changes in Sentry-1613 and Sentry-1587.
>
>
> Thanks,
>
> Hao Hao
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/
-----------------------------------------------------------
(Updated March 9, 2017, 11: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 (updated)
-----
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 a5a835e7d463e4de95ef537f76f2872105278c84
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/5/
Changes: https://reviews.apache.org/r/55706/diff/4-5/
Testing (updated)
-------
Unit test added in TestSentryStore. End to end can be peroformed after following changes in Sentry-1613 and Sentry-1587.
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On March 8, 2017, 6:03 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
> > Lines 45 (patched)
> > <https://reviews.apache.org/r/55706/diff/4/?file=1654007#file1654007line45>
> >
> > curSeqnum is never used is it needed?
Yes, this is intentional. And I have documented it in TODO.
> On March 8, 2017, 6:03 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
> > Lines 73 (patched)
> > <https://reviews.apache.org/r/55706/diff/4/?file=1654008#file1654008line73>
> >
> > Can we pass seqNum in constructor above?
This line of code would be removed once we change to use curSeqNum instead.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168230
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168230
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java
Lines 23 (patched)
<https://reviews.apache.org/r/55706/#comment240436>
Style: The first sentense shouldn't use links, but may use {@code}.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
Lines 25 (patched)
<https://reviews.apache.org/r/55706/#comment240438>
hashmap is an unused import
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
Lines 45 (patched)
<https://reviews.apache.org/r/55706/#comment240437>
curSeqnum is never used is it needed?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
Lines 50 (patched)
<https://reviews.apache.org/r/55706/#comment240440>
Please add a comment, explaining what the code below is doing
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 35 (patched)
<https://reviews.apache.org/r/55706/#comment240441>
can be package-private
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 50 (patched)
<https://reviews.apache.org/r/55706/#comment240447>
Here and in all the places below it should be List instead of LinkedList
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 58 (patched)
<https://reviews.apache.org/r/55706/#comment240451>
Use Map instead of HashMap here and below. HashMap should only be used for instance creation
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 73 (patched)
<https://reviews.apache.org/r/55706/#comment240442>
Can we pass seqNum in constructor above?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 147 (original)
<https://reviews.apache.org/r/55706/#comment240452>
Use Map instead of HashMap
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 151 (original)
<https://reviews.apache.org/r/55706/#comment240448>
should be List instead of LinkedList
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java
Lines 25 (patched)
<https://reviews.apache.org/r/55706/#comment240445>
s/of/for
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
Lines 21 (patched)
<https://reviews.apache.org/r/55706/#comment240453>
Use Map instead of HashMap everywhere here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
Lines 26 (patched)
<https://reviews.apache.org/r/55706/#comment240446>
s/of/for
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
Lines 32 (patched)
<https://reviews.apache.org/r/55706/#comment240449>
Indeed, it is better to use List rather then LinkedList
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
Lines 33 (patched)
<https://reviews.apache.org/r/55706/#comment240450>
Please document what is the structure of the map - what is the String and what is the internal map.
Also, please use Map rather than HashMap here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2237 (patched)
<https://reviews.apache.org/r/55706/#comment240454>
You don't need to document what is called internally in the javadoc (you are very welcome to document this in implementation comments). But it is a good idea to say what is a snapshot - that it consists of full privilege and role images.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2253 (patched)
<https://reviews.apache.org/r/55706/#comment240455>
Use List here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2254 (patched)
<https://reviews.apache.org/r/55706/#comment240456>
Use Map here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2267 (patched)
<https://reviews.apache.org/r/55706/#comment240457>
Use Map here
Also, since this is a map from String to String please document what is the key and the value
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2240 (original), 2271 (patched)
<https://reviews.apache.org/r/55706/#comment240458>
Use Map on the left
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2292 (original), 2323 (patched)
<https://reviews.apache.org/r/55706/#comment240459>
Better to use List here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2338 (patched)
<https://reviews.apache.org/r/55706/#comment240460>
End first sentence with dot.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2339 (patched)
<https://reviews.apache.org/r/55706/#comment240461>
Instead of telling what it calls internally, it is better to explain what is done/returned
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2319 (original), 2367 (patched)
<https://reviews.apache.org/r/55706/#comment240462>
Here Iterable can be used instead of List
- Alexander Kolbasov
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> On March 8, 2017, 6:05 a.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java
> > Lines 36 (patched)
> > <https://reviews.apache.org/r/55706/diff/4/?file=1654005#file1654005line36>
> >
> > What is seqNum?
This seqNum is a legacy field because of old design. I am going to remove it in next patch: Sentry-1613.
- Hao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168247
-----------------------------------------------------------
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168247
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java
Lines 36 (patched)
<https://reviews.apache.org/r/55706/#comment240463>
What is seqNum?
- Alexander Kolbasov
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168248
-----------------------------------------------------------
My comments regarding the use of interfaces (List, Map) rather then concrete types are a recommendation - it is Ok to leave as is. I actually tried converting to interfaces and it turned out to be pretty small change.
- Alexander Kolbasov
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
> 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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Lei Xu <le...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review168025
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
Lines 29 (patched)
<https://reviews.apache.org/r/55706/#comment240069>
Please add some comments for the class.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 31 (patched)
<https://reviews.apache.org/r/55706/#comment240070>
Please add comments for the class.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 68 (patched)
<https://reviews.apache.org/r/55706/#comment240083>
You might want to break this line.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
Lines 72 (patched)
<https://reviews.apache.org/r/55706/#comment240082>
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?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
Lines 32 (patched)
<https://reviews.apache.org/r/55706/#comment240032>
Can we use the interface here?
I.e, `private final Map<String, List<String>> roleImage;` as well as the other places in this file.
Btw, Could you add comments of these fields, especially, what does each `String` field represent?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2281 (original), 2314 (patched)
<https://reviews.apache.org/r/55706/#comment240056>
It can be `private`. Btw, it is better to return `Map<String, List<String>>`.
Would you consider rename it to `retrieveFullRollImageCore`?
It is not executed in a transaction, so it might not suppose to be called outside this class?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2311 (original), 2364 (patched)
<https://reviews.apache.org/r/55706/#comment240042>
It should be a private function.
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/#comment240061>
Will `query.execute()` return `null`?
http://www.datanucleus.org/javadocs/javax.jdo/3.2/javax/jdo/Query.html#execute--
- Lei Xu
On March 2, 2017, 1: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, 1: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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-----
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/
Changes: https://reviews.apache.org/r/55706/diff/3-4/
Testing
-------
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/
-----------------------------------------------------------
(Updated March 2, 2017, 1:49 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 (updated)
-----
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 b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
Diff: https://reviews.apache.org/r/55706/diff/3/
Changes: https://reviews.apache.org/r/55706/diff/2-3/
Testing
-------
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/#review167622
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java
Line 74 (original), 68 (patched)
<https://reviews.apache.org/r/55706/#comment239528>
Those code is just for backward compatible with old design. this class will be completely removed in SENTRY-1613. Would you mind I do it in that jira if needed?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 2239 (original), 2291 (patched)
<https://reviews.apache.org/r/55706/#comment239530>
It is used in line 2393. Am I missing anything?
- Hao Hao
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
>
>
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-----
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/
Testing
-------
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/
-----------------------------------------------------------
(Updated Jan. 23, 2017, 7:55 p.m.)
Review request for sentry.
Repository: sentry
Description (updated)
-------
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 9ecd9e4d9a862804eab26594c28aa691b89947aa
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 47c9f9dd7105ba5440a81ace9292d730df26f3cd
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 0712e2c9246ba7fe0c81d42861628c60eee2cfa0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java a35c8d7dde485cf46d61968a211d1dbb6d9d6076
Diff: https://reviews.apache.org/r/55706/diff/
Testing
-------
Thanks,
Hao Hao
Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot
available for NN plugin
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55706/
-----------------------------------------------------------
(Updated Jan. 23, 2017, 7:55 p.m.)
Review request for sentry.
Repository: sentry
Description (updated)
-------
Read full permission and path snapshot from SentryDB and make the update available for NN plugin upon requests.
In detail:
Added Path/PermissionImage classes to represent corresponding Path/Permission snapshot read from DB.
Refactor full snapshot retriever APIs in SentryStore to become a single transaction to ensure snapshot consistency.
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 9ecd9e4d9a862804eab26594c28aa691b89947aa
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 47c9f9dd7105ba5440a81ace9292d730df26f3cd
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 0712e2c9246ba7fe0c81d42861628c60eee2cfa0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java a35c8d7dde485cf46d61968a211d1dbb6d9d6076
Diff: https://reviews.apache.org/r/55706/diff/
Testing
-------
Thanks,
Hao Hao