> On Nov. 21, 2015, 9:57 a.m., Lenni Kuff wrote:
> > Thanks for putting in the effort for this patch! I'm a little confused
> > about what this is doing and how it is solving the problem. Is the problem
> > that when you do a SHOW TABLES that we make a RPC to Sentry to check the
> > same privielges for each one of the tables? My comments below assume that's
> > the case:
> >
> > 1) Why are we adding any new thrift interfaces here? It seems like this
> > should be contained within the HMS plugin?
> > 2) Do we need to have a cache that expires? That seems to add complexity
> > here.
> > 3) What does it even mean to have an RPC for a cached privilege set?
> >
> > I would expect a solution that looked like (using psedo code):
> >
> >
> > List<Privilege> getPrivilegesForUse(String username);
> >
> >
> >
> > List<String> filterTables() {
> >
> >
> > List<Privilege> userPrivileges = getPrivilegesForUser(username);
> >
> > // Note, we create a new cache each time filterTables() is called.
> > PrivilegeCache cache = new PrivilegeCache(userPrivileges);
> > AuthProvider provider = new AuthProvider(cache);
> >
> > for table in getAllTables() {
> > if (privider.hasAccess(table) ) {
> > // include this table;
> > }
> > }
> > }
> >
> > It appears that your solution is trying to maintain a global cache, which I
> > don't think is necessary. I also don't think that any thrift changes should
> > be required to make this work.
> >
> > Does this make sense? Am I missing something?
Thanks for the suggestion, I think your solution make sense for the metadata
filter problems. And I'll update another patch with this solution.
- Colin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28800/#review107488
-----------------------------------------------------------
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
>
>
