> On May 6, 2016, 8:34 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java, > > line 123 > > <https://reviews.apache.org/r/46909/diff/2/?file=1374087#file1374087line123> > > > > For the filter, some situations should be considered: > > eg, db=db1->table=*, with the current implementation, it will be ignore > > and the authorization will be failed. > > Just return all the privileges for user, and do the filter as the > > improvement. > > Ashish Singh wrote: > Makes sense as this is anyway coming from cache. Will have to amend tests > accordingly. Created SENTRY-1245 to track this.
Thanks > On May 6, 2016, 8:34 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java, > > line 91 > > <https://reviews.apache.org/r/46909/diff/2/?file=1374089#file1374089line91> > > > > If Sentry client has to get all metadatas from server side, I think > > there should be one RPC call for this. > > Too many rpc calls are in loadFromRemote() if there has many roles, > > this will be a performance issue. SENTRY-197 is the feature about > > import/export for Hive, and it can be a reference to add a new interface to > > get all these metadatas. > > Ashish Singh wrote: > You are right, however given that the fetch happens in background the > perf hit should not affect end users. Filed a follow up jira, SENTRY-1246, to > track this. Makes sense? Thanks for tracking this. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46909/#review132002 ----------------------------------------------------------- On May 8, 2016, 1:15 a.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46909/ > ----------------------------------------------------------- > > (Updated May 8, 2016, 1:15 a.m.) > > > Review request for sentry, Dapeng Sun, Gregory Chanan, Hao Hao, and Sravya > Tirukkovalur. > > > Bugs: SENTRY-1229 > https://issues.apache.org/jira/browse/SENTRY-1229 > > > Repository: sentry > > > Description > ------- > > SENTRY-1229: Add caching to SentryGenericProviderBackend. > > > Diffs > ----- > > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/Cache.java > PRE-CREATION > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java > PRE-CREATION > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java > ffd3af4903dd59f37ea1ff4a55138742fdfa74da > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java > 222b6fd530a0e26ea625d1be0fd68b0828558316 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java > 76ff15b91fddad6b5aa429d2439eb2a5086544d7 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > 74b6963ab4015521462fb3c25559d0d12292424f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 00e3fbde76ebf84704ee110adfca30869845a7b8 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java > 3c0e349443e3917812b46aa4547c3cab14dab052 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/TableCache.java > PRE-CREATION > > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java > a2cfa28da2af548a872ec0d2e5620ae1c27041b3 > > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java > 250522ed33394a667b1a0d6c47c68b210e0214a3 > > Diff: https://reviews.apache.org/r/46909/diff/ > > > Testing > ------- > > Extended Kafka e2e tests. Will be running perf tests for Kafka-Sentry > integration with caching turned on. > > > Thanks, > > Ashish Singh > >
