You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sergio Pena <se...@cloudera.com> on 2017/06/28 21:15:01 UTC
Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
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/1/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> 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
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179180
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 87 (patched)
<https://reviews.apache.org/r/60518/#comment253754>
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?
- Na Li
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
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179183
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 64 (original), 101 (patched)
<https://reviews.apache.org/r/60518/#comment253756>
When requested imagNum == -1, curSeqNum == 0 and seqNum == 1 (when HDFS starts, there is no path changes or permission changes, it should get full snapshot), this code return empty list, but a full snapshot should be returned.
We should include the fix in SENTRY-1784, so the behavior in above scenario is correct
- Na Li
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
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On June 30, 2017, 5:15 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 74 (patched)
> > <https://reviews.apache.org/r/60518/diff/3/?file=1767187#file1767187line74>
> >
> > Can you use java doc format with <p>?
<p> is not needed in this line because this is the end of the paragraph.
> On June 30, 2017, 5:15 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
> > Lines 158 (patched)
> > <https://reviews.apache.org/r/60518/diff/3/?file=1767193#file1767193line158>
> >
> > can we get rid of NO_LAST_SEEN_HMS_PATH_SEQ_NUM and NO_LAST_SEEN_HMS_PATH_IMG_NUM? instead we can use UPDATEABLE_UNINITIALIZED_SEQUENCE_NUMBER and UNUSED_PATH_UPDATE_IMG_NUM
I cannot use the UPDATEABLE_UNINITIALIZED_SEQUENCE_NUMBER because this is a -1 that has a meaning for the HDFS client side that it has not initialized the sequence. Then it will send a 0, and getAllUpdatesFrom() understands the 0 value only. And the UNUSED has a -1, that means that getAllUpdatesFrom() should not use it at all.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179383
-----------------------------------------------------------
On June 29, 2017, 8:18 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 8: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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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/3/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179383
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 67 (patched)
<https://reviews.apache.org/r/60518/#comment254047>
It could retrieve delta updates or full updates
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 69 (patched)
<https://reviews.apache.org/r/60518/#comment254048>
typo: and/ro should be "and/or"
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 74 (patched)
<https://reviews.apache.org/r/60518/#comment254046>
Can you use java doc format with <p>?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 76 (patched)
<https://reviews.apache.org/r/60518/#comment254049>
returned could be list of full snapshot
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 104 (original), 105 (patched)
<https://reviews.apache.org/r/60518/#comment254052>
We don't need to define these two constants. Just use existing ones are sufficient and it is more consistant.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Lines 158 (patched)
<https://reviews.apache.org/r/60518/#comment254051>
can we get rid of NO_LAST_SEEN_HMS_PATH_SEQ_NUM and NO_LAST_SEEN_HMS_PATH_IMG_NUM? instead we can use UPDATEABLE_UNINITIALIZED_SEQUENCE_NUMBER and UNUSED_PATH_UPDATE_IMG_NUM
- Na Li
On June 29, 2017, 8:18 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 8: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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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/3/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179378
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
Lines 65 (patched)
<https://reviews.apache.org/r/60518/#comment254041>
This looks weird. Usually, the constructor with less input parameters call the constructor with more input and the missing input takes default value.
- Na Li
On June 29, 2017, 8:18 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 8: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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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/3/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179381
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 165 (original), 164 (patched)
<https://reviews.apache.org/r/60518/#comment254045>
Can we use string format here?
- Na Li
On June 29, 2017, 8:18 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 8: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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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/3/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On June 30, 2017, 8:02 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 163 (original), 163 (patched)
> > <https://reviews.apache.org/r/60518/diff/3-4/?file=1767184#file1767184line163>
> >
> > what's the string of getClass().toString()? Could it contain info we don't need? will it be simpler to have getClass().getSimpleName()?
It contains the name + a hash.
The getSimpleName() method does more things just to figure out the name.
I will use getName() instead of it returns a variable value instead of having other conditions.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179404
-----------------------------------------------------------
On June 30, 2017, 7 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 30, 2017, 7 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/4/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179404
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 163 (original), 163 (patched)
<https://reviews.apache.org/r/60518/#comment254085>
what's the string of getClass().toString()? Could it contain info we don't need? will it be simpler to have getClass().getSimpleName()?
- Na Li
On June 30, 2017, 7 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 30, 2017, 7 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/4/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On July 6, 2017, 3:25 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Line 60 (original), 62 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line62>
> >
> > I think, the term "generation" is better then "imgNum".
The image name is used on the thrift specificiation and als on SentryStore methods (retrievesFullPathsImage, retrieveFullPermssionsImage). I think we should continue using image.
> On July 6, 2017, 3:25 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line72>
> >
> > You don't need an "else" clause since previous clause ended with return.
It is needed because I'm checking if there is a new image. If there is no new image (curImgNum == imgNum), then I should continue with the rest of the code to check delta updates.
I just added a comment for this.
> On July 6, 2017, 3:25 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Line 72 (original), 89 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line89>
> >
> > Add info-level log here - it is important to know where we get full updates.
> >
> > Before, when list of deltas was empty we were returning the full update, now we don't. Is this the desired behavior?
I had another condition before where the last function return was going to return an empty list, so I didn't see the necessary empty condition check here, but after testing, I found that I should return a full update, and I forgot to add the condition back. Thanks for letting me know this.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179783
-----------------------------------------------------------
On July 5, 2017, 3:59 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 5, 2017, 3:59 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/5/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On July 6, 2017, 3:25 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line72>
> >
> > You don't need an "else" clause since previous clause ended with return.
>
> Sergio Pena wrote:
> It is needed because I'm checking if there is a new image. If there is no new image (curImgNum == imgNum), then I should continue with the rest of the code to check delta updates.
> I just added a comment for this.
Here is what I mean:
if (condition) {
...
return value;
} else if (another_condition) ...
Here else is not needed - this is the same as
if (condition) {
...
return value;
}
if (another condition) {
...
}
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179783
-----------------------------------------------------------
On July 7, 2017, 5:07 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 5:07 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/7/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179783
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 51 (original), 51 (patched)
<https://reviews.apache.org/r/60518/#comment254623>
In the case of full update it doesn't return all updates from the requested sequence number, it returns a single full update with its own sequence number. Please clarify this in the comment.
Also, please clarify what is an "image number". It isn't clear from the discussion below.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 55 (patched)
<https://reviews.apache.org/r/60518/#comment254624>
I don't understand this sentence. Are you saying that the requested image exists and there are deltas available from this image snapshot?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 58 (original), 60 (patched)
<https://reviews.apache.org/r/60518/#comment254625>
Please document that a full update is returned as a single-element list - there is some code that assumes it.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 60 (original), 62 (patched)
<https://reviews.apache.org/r/60518/#comment254626>
I think, the term "generation" is better then "imgNum".
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 66 (patched)
<https://reviews.apache.org/r/60518/#comment254627>
Why would imgNum be less then zero? Do you use some special value for "unknown"? Then use this value explicitely
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 72 (patched)
<https://reviews.apache.org/r/60518/#comment254629>
You don't need an "else" clause since previous clause ended with return.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 73 (patched)
<https://reviews.apache.org/r/60518/#comment254631>
A newer image is available, return it.
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 74 (patched)
<https://reviews.apache.org/r/60518/#comment254635>
Add info log here - it is important to know when we get full images
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 77 (patched)
<https://reviews.apache.org/r/60518/#comment254633>
Add a comment here - something like
"The requsted image is available, return list of deltas or the full image".
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Line 72 (original), 89 (patched)
<https://reviews.apache.org/r/60518/#comment254634>
Add info-level log here - it is important to know where we get full updates.
Before, when list of deltas was empty we were returning the full update, now we don't. Is this the desired behavior?
- Alexander Kolbasov
On July 5, 2017, 3:59 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 5, 2017, 3:59 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/5/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179645
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
Lines 48 (patched)
<https://reviews.apache.org/r/60518/#comment254456>
What is the known bugs? Is it because the image ID is not provided? Can you make the error message more precious?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
Lines 48 (patched)
<https://reviews.apache.org/r/60518/#comment254457>
What is the known bugs? Is it because the image ID is not provided? Can you make the error message more precious?
- Na Li
On July 5, 2017, 3:59 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 5, 2017, 3:59 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/5/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179646
-----------------------------------------------------------
Ship it!
Ship It!
- Na Li
On July 5, 2017, 3:59 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 5, 2017, 3:59 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/5/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review180090
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Lines 142 (patched)
<https://reviews.apache.org/r/60518/#comment255107>
Yeah, this is rather ugly as well... The String.format was at least cleaner :-(
- Alexander Kolbasov
On July 7, 2017, 5:07 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 5:07 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/7/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review180103
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On July 10, 2017, 9:12 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 10, 2017, 9:12 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/8/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated July 10, 2017, 9:12 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Use String.format() on the LOGGER.debug() functions instead of built-in.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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 1402ab17592bf952614fb58c4ce15f22c6faee71
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/8/
Changes: https://reviews.apache.org/r/60518/diff/7-8/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated July 7, 2017, 5:07 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Addressed Sasha's changes.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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 1402ab17592bf952614fb58c4ce15f22c6faee71
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/7/
Changes: https://reviews.apache.org/r/60518/diff/6-7/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 136 (original), 136 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line136>
> >
> > Is it correct to use != here or this should use equals?
This condition detects if there was a change on the updates fetched. The processUpdates() will return the authzPaths or authzPermissions if no update is found, or create a new object with the updates. so this condition checks if the returned object is not different (meaning new updates are found).
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 139 (original), 139 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line139>
> >
> > Can you use builtin {} formatting instead?
There are some compilation errors when trying to use more than 2 parameters after the string. I really did not know how to fix it, so I move to the String.format()
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 143 (original), 143 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line143>
> >
> > Please use builtin formatting instead
Why? Lina asked me to move to this one. This helped me to read the formatting better. I don't see the difference witht the builtin.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 163 (original), 162 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line163>
> >
> > Here and below please use built-in formatting
Same question as before.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 69 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line69>
> >
> > In another place you return UNUSED_PATH_UPDATE_IMG_NUM instead - should it be the value checked here? In general, would this code work correctly with perm images?
This is really confusing. The DBUpdateForwarder is used for both path and perm updates. Last time I used the UNUSED_PATH_UPDATE_IMG_NUM, but Lina got confused if this was generic, so now I changed it to EMPTY_CHANGE_ID, but is confusing as well. I also created another constant, but Lina recommended to not use many constants with the same value.
What is your recommendation?
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179789
-----------------------------------------------------------
On July 6, 2017, 5:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 6, 2017, 5:01 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/6/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 139 (original), 139 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line139>
> >
> > Can you use builtin {} formatting instead?
>
> Sergio Pena wrote:
> There are some compilation errors when trying to use more than 2 parameters after the string. I really did not know how to fix it, so I move to the String.format()
>
> Alexander Kolbasov wrote:
> I had this problem as well. Looking at the Logger code I see this:
>
> ```public void info(String format, Object[] argArray);```
>
> Can this be used?
>
> Sergio Pena wrote:
> It works, but I'm still wonder, what's the difference on using String.format() vs the builtin function of the logger?
>
> LOG.debug("FULL Updated paths seq Num [old={}], [new={}] img Num [old={}], [new={}]",
> new Object[]{
> authzPaths.getLastUpdatedSeqNum(), newAuthzPaths.getLastUpdatedSeqNum(),
> authzPaths.getLastUpdatedImgNum(), newAuthzPaths.getLastUpdatedImgNum()
> });
>
> LOG.debug(String.format("FULL Updated paths seq Num [old=%d], [new=%d] img Num [old=%d], [new=%d]",
> authzPaths.getLastUpdatedSeqNum(), newAuthzPaths.getLastUpdatedSeqNum(),
> authzPaths.getLastUpdatedImgNum(), newAuthzPaths.getLastUpdatedImgNum()));
The major difference is that String.format() is called always whereis the built-in formatting is only called when DEBUG logging is enabled. This may be fine - e.g. where you only log in error cases, but would be good to avoid in normal cases. You can also verify whether debug logging is enabled yourself.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179789
-----------------------------------------------------------
On July 7, 2017, 5:07 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 5:07 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/7/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 139 (original), 139 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line139>
> >
> > Can you use builtin {} formatting instead?
>
> Sergio Pena wrote:
> There are some compilation errors when trying to use more than 2 parameters after the string. I really did not know how to fix it, so I move to the String.format()
>
> Alexander Kolbasov wrote:
> I had this problem as well. Looking at the Logger code I see this:
>
> ```public void info(String format, Object[] argArray);```
>
> Can this be used?
It works, but I'm still wonder, what's the difference on using String.format() vs the builtin function of the logger?
LOG.debug("FULL Updated paths seq Num [old={}], [new={}] img Num [old={}], [new={}]",
new Object[]{
authzPaths.getLastUpdatedSeqNum(), newAuthzPaths.getLastUpdatedSeqNum(),
authzPaths.getLastUpdatedImgNum(), newAuthzPaths.getLastUpdatedImgNum()
});
LOG.debug(String.format("FULL Updated paths seq Num [old=%d], [new=%d] img Num [old=%d], [new=%d]",
authzPaths.getLastUpdatedSeqNum(), newAuthzPaths.getLastUpdatedSeqNum(),
authzPaths.getLastUpdatedImgNum(), newAuthzPaths.getLastUpdatedImgNum()));
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 143 (original), 143 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line143>
> >
> > Please use builtin formatting instead
>
> Sergio Pena wrote:
> Why? Lina asked me to move to this one. This helped me to read the formatting better. I don't see the difference witht the builtin.
>
> Alexander Kolbasov wrote:
> The String.format is called whether DEBUG logging is enabled or not. built-in formatting only does formatting when it is enabled.
Oh ok, that sounds convincing
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 69 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line69>
> >
> > In another place you return UNUSED_PATH_UPDATE_IMG_NUM instead - should it be the value checked here? In general, would this code work correctly with perm images?
>
> Sergio Pena wrote:
> This is really confusing. The DBUpdateForwarder is used for both path and perm updates. Last time I used the UNUSED_PATH_UPDATE_IMG_NUM, but Lina got confused if this was generic, so now I changed it to EMPTY_CHANGE_ID, but is confusing as well. I also created another constant, but Lina recommended to not use many constants with the same value.
>
> What is your recommendation?
>
> Alexander Kolbasov wrote:
> Is there any reason we would use different number? Since for perm updates this doesn't make much sense anyway I'd just have a single constant - something like IMG_VALUE_UNITIALIZED or something like that and use it in all the places that we have. Would this work?
Yes, that would work.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179789
-----------------------------------------------------------
On July 7, 2017, 5:07 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 5:07 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/7/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 136 (original), 136 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line136>
> >
> > Is it correct to use != here or this should use equals?
>
> Sergio Pena wrote:
> This condition detects if there was a change on the updates fetched. The processUpdates() will return the authzPaths or authzPermissions if no update is found, or create a new object with the updates. so this condition checks if the returned object is not different (meaning new updates are found).
I see. Can you add a comment here, explaining this - this is a non-trivial piece of knowledge.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 139 (original), 139 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line139>
> >
> > Can you use builtin {} formatting instead?
>
> Sergio Pena wrote:
> There are some compilation errors when trying to use more than 2 parameters after the string. I really did not know how to fix it, so I move to the String.format()
I had this problem as well. Looking at the Logger code I see this:
```public void info(String format, Object[] argArray);```
Can this be used?
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
> > Line 143 (original), 143 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769582#file1769582line143>
> >
> > Please use builtin formatting instead
>
> Sergio Pena wrote:
> Why? Lina asked me to move to this one. This helped me to read the formatting better. I don't see the difference witht the builtin.
The String.format is called whether DEBUG logging is enabled or not. built-in formatting only does formatting when it is enabled.
> On July 6, 2017, 5:06 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
> > Lines 69 (patched)
> > <https://reviews.apache.org/r/60518/diff/5/?file=1769585#file1769585line69>
> >
> > In another place you return UNUSED_PATH_UPDATE_IMG_NUM instead - should it be the value checked here? In general, would this code work correctly with perm images?
>
> Sergio Pena wrote:
> This is really confusing. The DBUpdateForwarder is used for both path and perm updates. Last time I used the UNUSED_PATH_UPDATE_IMG_NUM, but Lina got confused if this was generic, so now I changed it to EMPTY_CHANGE_ID, but is confusing as well. I also created another constant, but Lina recommended to not use many constants with the same value.
>
> What is your recommendation?
Is there any reason we would use different number? Since for perm updates this doesn't make much sense anyway I'd just have a single constant - something like IMG_VALUE_UNITIALIZED or something like that and use it in all the places that we have. Would this work?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179789
-----------------------------------------------------------
On July 6, 2017, 5:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 6, 2017, 5:01 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/6/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179789
-----------------------------------------------------------
The Description section here is nice, it would be good to put it as a comment in the appropriate place in the code.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ImageRetriever.java
Lines 45 (patched)
<https://reviews.apache.org/r/60518/#comment254636>
This doesn't add much - it is sufficient to describe the return value - this just repeats it.
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 136 (original), 136 (patched)
<https://reviews.apache.org/r/60518/#comment254649>
Is it correct to use != here or this should use equals?
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 139 (original), 139 (patched)
<https://reviews.apache.org/r/60518/#comment254650>
Can you use builtin {} formatting instead?
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 143 (original), 143 (patched)
<https://reviews.apache.org/r/60518/#comment254651>
Please use builtin formatting instead
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java
Line 163 (original), 162 (patched)
<https://reviews.apache.org/r/60518/#comment254652>
Here and below please use built-in formatting
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java
Lines 69 (patched)
<https://reviews.apache.org/r/60518/#comment254646>
In another place you return UNUSED_PATH_UPDATE_IMG_NUM instead - should it be the value checked here? In general, would this code work correctly with perm images?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/PathImageRetriever.java
Line 28 (original), 28 (patched)
<https://reviews.apache.org/r/60518/#comment254637>
This and the next imports are unused
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java
Line 27 (original), 29 (patched)
<https://reviews.apache.org/r/60518/#comment254638>
In other places it is called image ID but here it is called snapshot id which is inconsistent.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PathsImage.java
Line 36 (original), 39 (patched)
<https://reviews.apache.org/r/60518/#comment254639>
This is public member of public class - why do you need @VisibleForTesting here?
- Alexander Kolbasov
On July 6, 2017, 5:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 6, 2017, 5:01 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/6/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
> > Lines 43 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770628#file1770628line43>
> >
> > Is this setter used by anything?
It's not used. I removed it now.
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
> > Line 49 (original), 52 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770632#file1770632line52>
> >
> > Why is this field Atomic?
I don't know the reason of this. I just used the an atomic variable as the sequence # already uses it.
I would leave it for now as the patch is not intending to fix any multi-threading issues for now, but I would investigate more about it.
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770638#file1770638line76>
> >
> > Do we really want to print the actual update?
I don't know, it was already there. I just removed it as I don't see why we want that on debug.
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Line 80 (original), 80 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770646#file1770646line80>
> >
> > Ths shows as an unused import
It is not shown as unsed to me.
It is used in line 2031:
if (e.getCause() instanceof MissingTableException) {
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2543 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770646#file1770646line2543>
> >
> > Why not just call getMaxPersistedIDCore()?
I think you're talking about getCurrentAuthzPathsSnapshotID(), right?
There are a few places where this method ID is used, and it looks better to call that method with the void parameter than this:
getMaxPersistedIDCore(pm, MAuthzPathsSnapshotId.class, "authzSnapshotID", EMPTY_PATHS_SNAPSHOT_ID);
> On July 6, 2017, 8:29 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 3653 (patched)
> > <https://reviews.apache.org/r/60518/diff/6/?file=1770646#file1770646line3654>
> >
> > Why not call getMaxPersistedIDCore() directly?
Same answer to the previous question. getCurrentAuthzPathsSnapshotID() is called several times.
- Sergio
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179806
-----------------------------------------------------------
On July 6, 2017, 5:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 6, 2017, 5:01 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/6/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179806
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java
Lines 43 (patched)
<https://reviews.apache.org/r/60518/#comment254680>
Is this setter used by anything?
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java
Line 36 (original), 39 (patched)
<https://reviews.apache.org/r/60518/#comment254681>
Why is this one and the next one Atomic?
sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java
Line 49 (original), 52 (patched)
<https://reviews.apache.org/r/60518/#comment254683>
Why is this field Atomic?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
Line 36 (original), 36 (patched)
<https://reviews.apache.org/r/60518/#comment254653>
This is an unused import now
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceProcessor.java
Lines 76 (patched)
<https://reviews.apache.org/r/60518/#comment254654>
Do we really want to print the actual update?
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 24 (original), 24 (patched)
<https://reviews.apache.org/r/60518/#comment254675>
This seems to be unused
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 63 (original), 66 (patched)
<https://reviews.apache.org/r/60518/#comment254677>
That's a good place to put your comments from RB
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Line 140 (original), 134 (patched)
<https://reviews.apache.org/r/60518/#comment254678>
Since last clause ended with return, no need for `else' here.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 80 (original), 80 (patched)
<https://reviews.apache.org/r/60518/#comment254670>
Ths shows as an unused import
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2543 (patched)
<https://reviews.apache.org/r/60518/#comment254673>
Why not just call getMaxPersistedIDCore()?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3653 (patched)
<https://reviews.apache.org/r/60518/#comment254674>
Why not call getMaxPersistedIDCore() directly?
- Alexander Kolbasov
On July 6, 2017, 5:01 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated July 6, 2017, 5:01 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/6/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated July 6, 2017, 5:01 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Addressed Sasha's comments.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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 1402ab17592bf952614fb58c4ce15f22c6faee71
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/6/
Changes: https://reviews.apache.org/r/60518/diff/5-6/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated July 5, 2017, 3:59 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Changes:
- removed DBUpdateForwarder.getAllUpdatesFrom(long seqNum) as it is not used anymore
- removed SentryPlugin.getAllPathsUpdatesFrom(long pathSeqNum) as it is not used anymore
- removed SentryPlugin.NO_LAST_SEEN_HMS_PATH_SEQ_NUM as it is not used anymore
- mark SentryHDFSServiceProcessor.get_all_authz_updates_from() as unsupported due to a bug cuasing full snapshots not be sent. The new fix to make it work + the new improvement to use path images are not compatible with the old implementation.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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 1402ab17592bf952614fb58c4ce15f22c6faee71
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/5/
Changes: https://reviews.apache.org/r/60518/diff/4-5/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179405
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
Lines 157 (patched)
<https://reviews.apache.org/r/60518/#comment254086>
We can get rid of NO_LAST_SEEN_HMS_PATH_SEQ_NUM and NO_LAST_SEEN_HMS_PATH_SEQ_NUM
- Na Li
On June 30, 2017, 7 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 30, 2017, 7 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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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 1402ab17592bf952614fb58c4ce15f22c6faee71
> 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/4/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated June 30, 2017, 7 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Addressed all Lina's changes.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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 1402ab17592bf952614fb58c4ce15f22c6faee71
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/4/
Changes: https://reviews.apache.org/r/60518/diff/3-4/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/#review179391
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
Lines 67 (patched)
<https://reviews.apache.org/r/60518/#comment254059>
the intial sequence number at client is -1, and request sequence number is 0. So the input sequence number should be 0 for first request, not 1.
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
Lines 75 (patched)
<https://reviews.apache.org/r/60518/#comment254061>
Can you add a test case when ther is no delta changes for first request and image number is unused?
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestDBUpdateForwarder.java
Lines 80 (patched)
<https://reviews.apache.org/r/60518/#comment254060>
input sequence number should be for first request
- Na Li
On June 29, 2017, 8:18 p.m., Sergio Pena wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60518/
> -----------------------------------------------------------
>
> (Updated June 29, 2017, 8: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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
> 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/3/
>
>
> Testing
> -------
>
> Added new unit tests:
> - TestDBUpdateForwarder
> - TestImageRetriever
> - TestDeltaRetriever
>
>
> Thanks,
>
> Sergio Pena
>
>
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60518/
-----------------------------------------------------------
(Updated June 29, 2017, 8:18 p.m.)
Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na Li, and Vamsee Yarlagadda.
Changes
-------
Addressing changes from Lina.
- Fix scenario when permissions cannot get a full sync in the first request and no deltas are available.
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 (updated)
-----
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/ServiceConstants.java 0741ebcbe9520fb70de20d2d45c6f0a741bb73c0
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/3/
Changes: https://reviews.apache.org/r/60518/diff/2-3/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena
Re: Review Request 60518: SENTRY-1815: Send new HMS snapshots to HDFS
requesting an old generation ID
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
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.
Changes
-------
Rebased patch.
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 (updated)
-----
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/
Changes: https://reviews.apache.org/r/60518/diff/1-2/
Testing
-------
Added new unit tests:
- TestDBUpdateForwarder
- TestImageRetriever
- TestDeltaRetriever
Thanks,
Sergio Pena