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



sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/DBPrivilegeCache.java
 (line 40)
<https://reviews.apache.org/r/28800/#comment165979>

    how about adding the time unit in the variable name, eg. 
DEFAULT_SENTRY_PRIVILEGE_CACHE_EXPIRE_SEC



sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/DBPrivilegeCache.java
 (line 48)
<https://reviews.apache.org/r/28800/#comment165980>

    I think the Concurrency issue that Anne asked is that HashMap is not thread 
safe.



sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForCachedResponse.java
 (line 2)
<https://reviews.apache.org/r/28800/#comment165996>

    I am a little confused. So we have not moified the auto generated file 
actually? If it is the case, I think we do not need add these auto gen files 
into review, but the original thrift file.


- Li Li


On Nov. 13, 2015, 4:10 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28800/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2015, 4:10 a.m.)
> 
> 
> Review request for sentry, Lenni Kuff and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Currently, when get the metadata from hive, eg, "show tables", "show 
> databases". Sentry will filter the result and output the authorized entities. 
> There will be many RPC calls when filtering the result. The related code is 
> in HiveAuthzBinding, for example, in filterShowTables:
> 
> ......
> for (String tableName : queryResult) {
>   ......
>   hiveAuthzBinding.authorize(operation, tableMetaDataPrivilege, subject, 
> inputHierarchy,
>             outputHierarchy, providedPrivileges);
>   ......
> }
> ......
> 
> hiveAuthzBinding.authorize will get the privileges from sentry service, if 
> there are many tables in the hive, the filtering process will spend much 
> time. Considering sentry also need to filter the column, HiveAuthzBinding 
> should be improved to reduce the number of rpc calls when doing the filter.
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-cache/pom.xml c67f094 
>   
> sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/CachedPrivilegeWrap.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/DBPrivilegeCache.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java
>  4b98447 
>   
> sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/PrivilegeCacheTestImpl.java
>  a7566e7 
>   
> sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestCacheProvider.java
>  e5b29b8 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/SentryPolicyService.java
>  0c24449 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForCachedRequest.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForCachedResponse.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  8c9401c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
>  cbc0aaf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  74f379a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  4f8c834 
>   
> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
>  40889e8 
> 
> Diff: https://reviews.apache.org/r/28800/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to