> 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
>
>
