----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28800/#review107728 -----------------------------------------------------------
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java (line 946) <https://reviews.apache.org/r/28800/#comment167004> typo - "privilege" sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java (line 244) <https://reviews.apache.org/r/28800/#comment167003> Can you just create a new instance of SimpleCacheProviderBackend directly? sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java (line 62) <https://reviews.apache.org/r/28800/#comment167002> Do we need to expose this config value? Seems like we can just instantiate the class directly since we always expect it to be the same. sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/UserHiveMetadataPrivilegeCache.java (line 28) <https://reviews.apache.org/r/28800/#comment167001> The name makes me think this is caching Hive metadata. Consider renaming to something like "SimplePrivilegeCache" sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java (line 74) <https://reviews.apache.org/r/28800/#comment166998> is null okay to return here? Comment on why you return null. Looks much better now. Just to confirm - you are still getting the same performance improvements after making this change? - Lenni Kuff On Nov. 24, 2015, 5:59 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28800/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 5:59 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-binding/sentry-binding-hive/pom.xml 6d57a58 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 18b8a8f > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 3071475 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java > 3919de7 > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/UserHiveMetadataPrivilegeCache.java > PRE-CREATION > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java > a88d2f8 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java > a814527 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 06573b7 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java > ae790f0 > > Diff: https://reviews.apache.org/r/28800/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
