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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
Lines 251 (patched)
<https://reviews.apache.org/r/64661/#comment290912>

    If you look at implemenation of "public Set<TSentryRole> 
getTSentryRolesByGroupName" in SentryStore, it converts list of MSentryRole to 
list of "org.apache.sentry.api.service.thrift.TSentryRole".
    
    Then in this function, you convert each one to " 
org.apache.sentry.api.generic.thrift.TSentryRole".
    
    For 5000 roles, for example, it is not trivil.
    
    To get best performance, you can just get list of MSentryRole, and convert 
them directly to " org.apache.sentry.api.generic.thrift.TSentryRole"


- Na Li


On Aug. 17, 2018, 2:01 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64661/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2018, 2:01 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, Steve Moist, and 
> Sergio Pena.
> 
> 
> Bugs: SENTRY-1944
>     https://issues.apache.org/jira/browse/SENTRY-1944
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> When Solr is using Sentry server for authorization, it issues a lot of calls 
> to getGroupsByRoles() function in DelegateSentryStore.
> 
> This function isn't very efficient - it walks over each role in the set, 
> obtains role by name, get groups for each role, and collects all group names 
> into a set.
> 
> It may be possible to optimize it.
> 
> Also, in SentryGenericPolicyProcessor class method 
> list_sentry_roles_by_group() would make N transactions to build the roles to 
> set of groups map. Instead, make it to a single transaction. This will 
> significantly speed up operation
> 
> Attach one or more files to this issue
> 
> 
> Diffs
> -----
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/generic/thrift/SentryGenericPolicyProcessor.java
>  1cc4b1b37 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  3026a6225 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreLayer.java
>  eec2757d3 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/generic/thrift/TestSentryGenericPolicyProcessor.java
>  4c207e9b4 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestDelegateSentryStore.java
>  69d16238f 
> 
> 
> Diff: https://reviews.apache.org/r/64661/diff/9/
> 
> 
> Testing
> -------
> 
> mvn -f sentry-provider/sentry-provider-db/pom.xml test
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Reply via email to