You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Abhay Kulkarni <ak...@hortonworks.com> on 2016/04/01 21:31:14 UTC
Review Request 45600: Optimize updates to Tag objects
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45600/
-----------------------------------------------------------
Review request for ranger and Madhan Neethiraj.
Bugs: RANGER-903
https://issues.apache.org/jira/browse/RANGER-903
Repository: ranger
Description
-------
Optimized processing of ServiceTags to minimize creation/deletion of Tag-related objects
Diffs
-----
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java 78040ba
agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractTagStore.java 7d2e130
agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java 69bd628
agents-common/src/main/java/org/apache/ranger/plugin/store/file/TagFileStore.java 58052cf
agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java 738a947
agents-common/src/test/java/org/apache/ranger/plugin/store/TestTagStore.java aaace89
security-admin/db/sqlserver/patches/021-update-tag-for-owner.sql dabe841
security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java 3b7555e
security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java cc2386e
security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java cdc44e0
security-admin/src/main/java/org/apache/ranger/db/XXTagAttributeDao.java c993477
security-admin/src/main/java/org/apache/ranger/db/XXTagDao.java 3c9370b
security-admin/src/main/java/org/apache/ranger/entity/XXTag.java 526557e
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 0dbd042
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java ebe71eb
security-admin/src/main/resources/META-INF/jpa_named_queries.xml 2bb66ca
tagsync/samples/tags.json 3028f9d
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasNotificationMapper.java 2168983
tagsync/src/main/resources/etc/ranger/data/tags.json b4cd736
Diff: https://reviews.apache.org/r/45600/diff/
Testing
-------
Uploaded ServiceTags containing Tag objects which already existed in the Ranger. Ensured that existing objects where updated wherever necessary, but no unnecessary deletions/creations were done.
Thanks,
Abhay Kulkarni
Re: Review Request 45600: Optimize updates to Tag objects
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45600/#review126662
-----------------------------------------------------------
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java (line 44)
<https://reviews.apache.org/r/45600/#comment189672>
Since this object is serialized (to/from Json), use Short instead of short.
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 92)
<https://reviews.apache.org/r/45600/#comment189679>
Looks like createOrUpdate is never set to 'false'. In that case, consider removing this variable and code its usage in rest of the method - to simplify the code.
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 124)
<https://reviews.apache.org/r/45600/#comment189678>
Looks like tagDefsInStore would never be null (line #89). Please review and remove this null-check if unnecessary.
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 157)
<https://reviews.apache.org/r/45600/#comment189680>
shouldn't the lookup based on resourceSignature be restricted to a service? To avoid incorrect updates to same named resources (like database=finance) in 2 different services (prod_hive, test_hive)
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 234)
<https://reviews.apache.org/r/45600/#comment189681>
shouldn't ID of the creted tag be added to associatedTagIds? Also review other blocks that create a tag.
Consider using another list to track the tags that need to be saved - like tagsToRetain; populate this list as you iterate to tagIds. At the end of processing delete the tags that are in associatedTagIds but not in tagsToRetain.
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 262)
<https://reviews.apache.org/r/45600/#comment189682>
why remove it from associatedTags?
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 277)
<https://reviews.apache.org/r/45600/#comment189675>
shouldn't the existing resource-private tag be removed in this case?
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 294)
<https://reviews.apache.org/r/45600/#comment189676>
can this query be replaced with "associatedTagIds.contains(matchingTag.getId())"?
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java (line 361)
<https://reviews.apache.org/r/45600/#comment189683>
This would end up retrieving the same tag multiple times. Consider using "List<RangerTag> existingTags" - instead of tagIds.
security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 894)
<https://reviews.apache.org/r/45600/#comment189673>
Consider replacing 0 with a parameter named :owner; and rename findPrivateTagsByServiceId ==> findTagsByOwnerAndServiceId.
security-admin/src/main/resources/META-INF/jpa_named_queries.xml (line 912)
<https://reviews.apache.org/r/45600/#comment189674>
Consider replacing 0 with a parameter named :owner; and rename findPrivateTagAttributesByServiceId ==> findTagAttributesByOwnerAndServiceId.
- Madhan Neethiraj
On April 1, 2016, 7:31 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45600/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 7:31 p.m.)
>
>
> Review request for ranger and Madhan Neethiraj.
>
>
> Bugs: RANGER-903
> https://issues.apache.org/jira/browse/RANGER-903
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Optimized processing of ServiceTags to minimize creation/deletion of Tag-related objects
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java 78040ba
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractTagStore.java 7d2e130
> agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java 69bd628
> agents-common/src/main/java/org/apache/ranger/plugin/store/file/TagFileStore.java 58052cf
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java 738a947
> agents-common/src/test/java/org/apache/ranger/plugin/store/TestTagStore.java aaace89
> security-admin/db/sqlserver/patches/021-update-tag-for-owner.sql dabe841
> security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java 3b7555e
> security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java cc2386e
> security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java cdc44e0
> security-admin/src/main/java/org/apache/ranger/db/XXTagAttributeDao.java c993477
> security-admin/src/main/java/org/apache/ranger/db/XXTagDao.java 3c9370b
> security-admin/src/main/java/org/apache/ranger/entity/XXTag.java 526557e
> security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 0dbd042
> security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java ebe71eb
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 2bb66ca
> tagsync/samples/tags.json 3028f9d
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasNotificationMapper.java 2168983
> tagsync/src/main/resources/etc/ranger/data/tags.json b4cd736
>
> Diff: https://reviews.apache.org/r/45600/diff/
>
>
> Testing
> -------
>
> Uploaded ServiceTags containing Tag objects which already existed in the Ranger. Ensured that existing objects where updated wherever necessary, but no unnecessary deletions/creations were done.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 45600: Optimize updates to Tag objects
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45600/#review126929
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On April 4, 2016, 7:54 p.m., Abhay Kulkarni wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45600/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 7:54 p.m.)
>
>
> Review request for ranger and Madhan Neethiraj.
>
>
> Bugs: RANGER-903
> https://issues.apache.org/jira/browse/RANGER-903
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Optimized processing of ServiceTags to minimize creation/deletion of Tag-related objects
>
>
> Diffs
> -----
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java 78040ba
> agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractTagStore.java 7d2e130
> agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java 69bd628
> agents-common/src/main/java/org/apache/ranger/plugin/store/file/TagFileStore.java 58052cf
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java 738a947
> agents-common/src/test/java/org/apache/ranger/plugin/store/TestTagStore.java aaace89
> security-admin/db/sqlserver/patches/021-update-tag-for-owner.sql dabe841
> security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java 3b7555e
> security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java cc2386e
> security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java cdc44e0
> security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceDao.java c990ae7
> security-admin/src/main/java/org/apache/ranger/db/XXTagAttributeDao.java c993477
> security-admin/src/main/java/org/apache/ranger/db/XXTagDao.java 3c9370b
> security-admin/src/main/java/org/apache/ranger/entity/XXTag.java 526557e
> security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 0dbd042
> security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java ebe71eb
> security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 378ff0b
> security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 5cf26c9
> security-admin/src/main/resources/META-INF/jpa_named_queries.xml 2bb66ca
> tagsync/samples/tags.json 3028f9d
> tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasNotificationMapper.java 2168983
> tagsync/src/main/resources/etc/ranger/data/tags.json b4cd736
>
> Diff: https://reviews.apache.org/r/45600/diff/
>
>
> Testing
> -------
>
> Uploaded ServiceTags containing Tag objects which already existed in the Ranger. Ensured that existing objects where updated wherever necessary, but no unnecessary deletions/creations were done.
>
>
> Thanks,
>
> Abhay Kulkarni
>
>
Re: Review Request 45600: Optimize updates to Tag objects
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45600/
-----------------------------------------------------------
(Updated April 4, 2016, 7:54 p.m.)
Review request for ranger and Madhan Neethiraj.
Changes
-------
Addressed review comments.
Bugs: RANGER-903
https://issues.apache.org/jira/browse/RANGER-903
Repository: ranger
Description
-------
Optimized processing of ServiceTags to minimize creation/deletion of Tag-related objects
Diffs (updated)
-----
agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java 78040ba
agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractTagStore.java 7d2e130
agents-common/src/main/java/org/apache/ranger/plugin/store/TagStore.java 69bd628
agents-common/src/main/java/org/apache/ranger/plugin/store/file/TagFileStore.java 58052cf
agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java 738a947
agents-common/src/test/java/org/apache/ranger/plugin/store/TestTagStore.java aaace89
security-admin/db/sqlserver/patches/021-update-tag-for-owner.sql dabe841
security-admin/src/main/java/org/apache/ranger/biz/RangerTagDBRetriever.java 3b7555e
security-admin/src/main/java/org/apache/ranger/biz/TagDBStore.java cc2386e
security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java cdc44e0
security-admin/src/main/java/org/apache/ranger/db/XXServiceResourceDao.java c990ae7
security-admin/src/main/java/org/apache/ranger/db/XXTagAttributeDao.java c993477
security-admin/src/main/java/org/apache/ranger/db/XXTagDao.java 3c9370b
security-admin/src/main/java/org/apache/ranger/entity/XXTag.java 526557e
security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 0dbd042
security-admin/src/main/java/org/apache/ranger/rest/ServiceTagsProcessor.java ebe71eb
security-admin/src/main/java/org/apache/ranger/rest/TagREST.java 378ff0b
security-admin/src/main/java/org/apache/ranger/service/RangerServiceResourceService.java 5cf26c9
security-admin/src/main/resources/META-INF/jpa_named_queries.xml 2bb66ca
tagsync/samples/tags.json 3028f9d
tagsync/src/main/java/org/apache/ranger/tagsync/source/atlas/AtlasNotificationMapper.java 2168983
tagsync/src/main/resources/etc/ranger/data/tags.json b4cd736
Diff: https://reviews.apache.org/r/45600/diff/
Testing
-------
Uploaded ServiceTags containing Tag objects which already existed in the Ranger. Ensured that existing objects where updated wherever necessary, but no unnecessary deletions/creations were done.
Thanks,
Abhay Kulkarni