----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28800/#review106716 -----------------------------------------------------------
sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/DBPrivilegeCache.java (line 18) <https://reviews.apache.org/r/28800/#comment165452> Since it's about cache and will impact performance, I have some general design questions (or it would be better you can put into a wiki or just javadoc this class): 1. How to retire cached items? The general strateg. If the load is very heavy, Expire time is not specified at a reasonable value, there would be too many cached items coming in in a very short time period so that the hash object will keep increasing fast and dramatically. Do we have a machemism to retire oldest items when the object size is large enough? Or limit cache size. We've seen server has an issue which full GC keeps increasing and young GC keeps propogating data into full GC. This impact performance much. 2. Concurrency. If there are multiple clients trying to make change about the same group, do we consider concurrency here? sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/SentryPolicyService.java (line 12345) <https://reviews.apache.org/r/28800/#comment165455> Minor issues: please take care of white spaces. - Anne Yu 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 > >
