> On Oct. 26, 2022, 2:13 p.m., Abhay Kulkarni wrote:
> > security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java
> > Lines 378 (patched)
> > <https://reviews.apache.org/r/74182/diff/1/?file=2271556#file2271556line378>
> >
> > If the number of newly added/modified service-resources and/or tags is
> > small, then de-deplicating entire updated serviceTags is wasteful. Can this
> > be moved inside RangerServiceTagsDeltaUtil.applyDelta() function? Same
> > comment for handling of ServicePoliciesCache.
>
> Madhan Neethiraj wrote:
> I agree that dedup of entire ServiceTags after applying delta may not
> result in significant savings. I will restore earlier version's #361 and #362.
>
>
> It is not clear what you meant by 'this be moved inside
> RangerServiceTagsDeltaUtil.applyDelta() '. Can you please clarify?
>
> Abhay Kulkarni wrote:
> There are two ways: 1. Keep the strTbl as a instance variable of the
> cache for the given service and pass it to the
> RangerServiceTagsDeltaUtil.applyDelta() function which can use de-duping
> using the same strTbl object. This guarantees that there is exactly one
> string for say, serviceName in entire cache, but requires some refactoring
> and memory overhead of keeping strTbl around with cache, or 2. Apply
> de-duping using a local strTbl inside RangerServiceTagsDeltaUtil.applyDelta()
> only for the delta objects. This will guarantee unique string values *within*
> the given set of deltas, but there will be more than one string object within
> the objects in the cache.
> 2. Apply de-duping using a local strTbl inside
> RangerServiceTagsDeltaUtil.applyDelta() only for the delta objects.
this is implemented currently, with a call to serviceTagsFromDb.dedupStrings()
in #347 - irrespective of whether serviceTagsFromDb is a delta or complete.
> 1. Keep the strTbl as a instance variable of the cache
I didn't chose this approach for the same reasons you called out. Also,
approach #1 can in-effect be implemented by having StirngUtil.dedupString() to
use String.intern() - which will work for strings across
policies/tags/user-store. However, this will result in strings be held in JVM
memory permenantly - which may not be desirable.
public static String dedupString(String str, Map<String, String> strTbl) {
return str != null ? str.intern() : str;
}
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74182/#review224829
-----------------------------------------------------------
On Oct. 26, 2022, 5:34 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74182/
> -----------------------------------------------------------
>
> (Updated Oct. 26, 2022, 5:34 p.m.)
>
>
> Review request for ranger, Ankita Sinha, Kishor Gollapalliwar, Abhay
> Kulkarni, Pradeep Agrawal, Ramesh Mani, Sailaja Polavarapu, Subhrat
> Chaudhary, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-3955
> https://issues.apache.org/jira/browse/RANGER-3955
>
>
> Repository: ranger
>
>
> Description
> -------
>
> - added dedupStrings() method in ServicePolicies, ServiceTags and
> RangerUseStore
> - added Ranger admin options to dedup strings in policies/tags/userstore
> caches
> - added Ranger plugin configuration to dedup strings in
> policies/tags/userstore cache
>
>
> Diffs
> -----
>
>
> agents-common/src/main/java/org/apache/ranger/authorization/utils/StringUtil.java
> e6c96028a
>
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java
> c3d1e7da6
>
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java
> a6dd38705
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerBaseModelObject.java
> 70c0a42e4
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicy.java
> af8150519
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyDelta.java
> 282696344
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceDef.java
> cd8b5d186
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerServiceResource.java
> 1c46ea5c2
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTag.java
> 4154ea32b
>
> agents-common/src/main/java/org/apache/ranger/plugin/model/RangerTagDef.java
> 6fc6f9553
>
> agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java
> dc51078a7
>
> agents-common/src/main/java/org/apache/ranger/plugin/util/RangerUserStore.java
> 0352c53dc
>
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServicePolicies.java
> fd033f05c
> agents-common/src/main/java/org/apache/ranger/plugin/util/ServiceTags.java
> 8a94d9a2e
>
> agents-common/src/test/java/org/apache/ranger/authorization/utils/TestStringUtil.java
> PRE-CREATION
>
> security-admin/src/main/java/org/apache/ranger/common/RangerServicePoliciesCache.java
> 961e87fbf
>
> security-admin/src/main/java/org/apache/ranger/common/RangerServiceTagsCache.java
> 4d80ccb18
>
> security-admin/src/main/java/org/apache/ranger/common/RangerUserStoreCache.java
> 0cb0ccca8
>
>
> Diff: https://reviews.apache.org/r/74182/diff/2/
>
>
> Testing
> -------
>
> - added unit tests to validate dedup
> - verified that all existing unit tests pass successfully
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
