> On June 28, 2017, 9:41 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 87 (patched)
> > <https://reviews.apache.org/r/60518/diff/2/?file=1766565#file1766565line87>
> >
> >     this function can be called for path update or permission update. for 
> > permission update, should we use SentryStore.EMPTY_PERMISSION_SNAPSHOT_ID?
> >     
> >     Will this code work for both path update and permission update?

It will work as both constants are 0. It is not readable, though, as this 
method should work with path and perm, but mentions a path constant only. Also, 
permission updates do not use SNAPSHOT_ID.


- Sergio


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179180
-----------------------------------------------------------


On June 28, 2017, 9:18 p.m., Sergio Pena wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
> 
> (Updated June 28, 2017, 9:18 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na 
> Li, and Vamsee Yarlagadda.
> 
> 
> Bugs: sentry-1815
>     https://issues.apache.org/jira/browse/sentry-1815
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> This patch uses the new generation ID for HMS snapshots on the HDFS sync as 
> well. When HDFS requestes an older generation ID, then this patch will return 
> a snapshot with the newer generation ID.
> 
> Help for the reviewer:
> 
> HDFS side
> 
> - SentryAuthorizationInfo is a thread that runs every 500ms to request new 
> updates from Sentry. This request is called from 
> SentryAuthorizationInfo.update()
> 
> - SentryAuthorizationInfo.update() calls SentryUpdater.getUpdates() which 
> calls SentryHDFSServiceClient.getAllUpdatesFrom() which calls
>   SentryHDFSServiceProcessor.get_authz_updates(). 
>   
>   The SentryAuthorizationInfo class has the information of path and 
> permission ID stored on the UpdateableAuthzPaths class. 
>   SentryUpdater gets such information from the UpdateableAuthzPaths gotten 
> from SentryAuthorizationInfo, and then calls the mentioned methods above until
>   arrive to the get_authz_updates() which is the call to the Thrift client.
>   
> SENTRY side
> 
> - Sentry receives the HDFS thrift call on the 
> SentryHDFSServiceProcessor.get_authz_updates() method which calls 
> SentryPlugin.getAllPathsUpdatesFrom().
> 
> - The SentryPlugin will call DBUpdateForwarder.getAllUpdatesFrom() later for 
> each update type, PathUpdates and PermissionUpdates.
> 
> SENTRY PATH/PERM UPDATES
> 
> - The DBUpdateForwarder.getAllPathsUpdatesFrom() gets full images or delta 
> updates based on the conditions.
> 
> - For full images, it uses the ImageRetriever interface (PathImageRetriever, 
> PermImageRetriever).
> - For delta changes, it uses the DeltaRetriever interface 
> (PathDeltaRetriever, PermDeltaRetriever).
> 
> - These two retrieves call the SentryStore to get either full images or delta 
> changes.
> 
> SENTRY STORE
> 
> - The call to the SentryStore to retrieve a full paths image will pass the 
> latest image ID so it can be sent to HDFS.
> 
> 
> Diffs
> -----
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java
>  e96140dc5322e25d20ff14b8fe85d769f0103362 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
>  49befee21953c7a0d77424379cefc5d3af43337a 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PermissionsUpdate.java
>  ebb0b96f5250d08a3bd262255da82afe2e6c313f 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
>  12baaa4b82fcad14aa07fa73c4e67c84d885c580 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
>  ad7f8c9ee3eec11324e562f0cabf273259084401 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
>  90ba72184ce55013c88905bf61314c908612b00a 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryUpdater.java
>  49f39b119cabd660d4890192c462b37a05ca479e 
>   
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
>  0259f4496050dc546e52a66c11493071ec36bc0b 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
>  f4086fb22007639fcc4a8dda8cea8a91e6a5fafd 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
>  de9474313b47be35b3803fd22fcb3d89864bf326 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PermImageRetriever.java
>  bad10995b23706321c974efdaaec6c60ec6ecf34 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClient.java
>  b6b78bc0e8ef556d6eeba51341729f221c49f04e 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java
>  86d0f6252c943d139575666ef022f41b00226f23 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
>  1c7061b96a9f19d7f789d0ce7e330f0f80424320 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  0bd08338dd1881849b180462de94293c022a1323 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDeltaRetriever.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java
>  PRE-CREATION 
>   
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java
>  fd56ce226d43b8201355942772cb93116acfa3e9 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PermissionsImage.java
>  f03e93f7aed2b1543270337291e9e6b6e0b3df59 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  9ad97bc15b0479b1fd0fb9b16752529c54f52e38 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  ac266fe6ef7bc1658341d34a7651b2e06f0c42c1 
> 
> 
> Diff: https://reviews.apache.org/r/60518/diff/2/
> 
> 
> Testing
> -------
> 
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Reply via email to