> On Oct. 21, 2015, 12:48 a.m., Sravya Tirukkovalur wrote:
> > Thanks for the update Colin! Will it be possible for you to add description 
> > on how you are solving the problem. It would be easy to follow along and 
> > also would help review the approach before delving into the code details.

hi, Sravya, the following is the current idea for the cached privilege:
1. CachedPrivilegeManagement is responsible for getPrivileges, and it likes an 
agent for providerBackend. CachedPrivilegeManagement will be initialized in 
HiveAuthzBinding according to the configuration file.
2. All cached privileges are stored as HashMap in CachedPrivilegeManagement. 
The key for this map is username, and the value is wrapped in the 
CachedPrivilege including all privileges for this username and the timestamp 
when refresh the privilege.
3. When do the authorization, SimpleDBPolicyEngine.getPrivileges will be 
called, and CachedPrivilegeManagement.getPrivileges will be called. If 
CachedPrivilegeManagement is initialized and cached privilege enable in 
configuraion, CachedPrivilegeManagement.getPrivileges will get the privileges 
from cache first, if not exist or expire, get the privileges by backend and 
refresh the cache. The expired time can be configured and default is 5 mins.

The core part for this problem is: HiveAuthzBinding, CachedPrivilege, 
CachedPrivilegeManagement, SimpleDBPolicyEngine, TestCachedPrivilege. The other 
code is just for the interface change.
The disadvantage for this solution is if admin change the privilege for user, 
this will not be effect immediately. There will be delay according to the 
expire time setting in configuration file. I think this is accepted for the 
authorization management.

Feel free for any comments, thanks for your review.


- Colin


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


On Oct. 16, 2015, 11:18 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28800/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2015, 11:18 a.m.)
> 
> 
> Review request for sentry and Lenni Kuff.
> 
> 
> 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-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  3071475 
>   
> sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PolicyEngine.java
>  38a5b65 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/CachedPrivilege.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/CachedPrivilegeManagement.java
>  PRE-CREATION 
>   
> sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/SimpleDBPolicyEngine.java
>  a03794e 
>   
> sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/AbstractTestSimplePolicyEngine.java
>  d1151e3 
>   
> sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestPolicyParsingNegative.java
>  5f7c671 
>   
> sentry-policy/sentry-policy-db/src/test/java/org/apache/sentry/policy/db/TestSimpleDBPolicyEngineDFS.java
>  f8c36e2 
>   
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java
>  2f4bc1d 
>   
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/AbstractTestIndexerPolicyEngine.java
>  d7d1ae2 
>   
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyNegative.java
>  0706560 
>   
> sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java
>  bc518fb 
>   
> sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/AbstractTestSearchPolicyEngine.java
>  d1c415b 
>   
> sentry-policy/sentry-policy-search/src/test/java/org/apache/sentry/policy/search/TestSearchPolicyNegative.java
>  2abe8f2 
>   
> sentry-policy/sentry-policy-sqoop/src/main/java/org/apache/sentry/policy/sqoop/SimpleSqoopPolicyEngine.java
>  e8615a0 
>   
> sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/AbstractTestSqoopPolicyEngine.java
>  1389fca 
>   
> sentry-policy/sentry-policy-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopPolicyNegative.java
>  406e53f 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  06573b7 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestGetGroupMapping.java
>  f57198a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  bc35742 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestCachedPrivilege.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
>  cc5daef 
> 
> Diff: https://reviews.apache.org/r/28800/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to