----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31321/#review73769 -----------------------------------------------------------
Ship it! sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java <https://reviews.apache.org/r/31321/#comment120153> is it possible to verify that this gets incremented to 1 when there is an active connection? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java <https://reviews.apache.org/r/31321/#comment120154> Does this need to be thread safe (I think it does). If so, I think you need to protect clientList. Should clientList actually be a Set though? If this does not need to be thread safe, change these from AtomicLong long for claririty. Comment on thread safety. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java <https://reviews.apache.org/r/31321/#comment120155> can you get rid of clientCount and just return clientList.size()? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java <https://reviews.apache.org/r/31321/#comment120156> Get the - Lenni Kuff On Feb. 23, 2015, 9:11 p.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31321/ > ----------------------------------------------------------- > > (Updated Feb. 23, 2015, 9:11 p.m.) > > > Review request for sentry, Arun Suresh and Lenni Kuff. > > > Bugs: SENTRY-658 > https://issues.apache.org/jira/browse/SENTRY-658 > > > Repository: sentry > > > Description > ------- > > Ensure that the hive binding client is closed at the end of the operation > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java > 0cbb250 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > e311398 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > b4b69e1 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java > 5e26e83 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 0b4b39a > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbConnections.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java > 68bc9ee > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java > e9ae5fa > > Diff: https://reviews.apache.org/r/31321/diff/ > > > Testing > ------- > > - Updated miniSentry framework to track active clients > - Added new test to verify client disconnection for various Hive operations > > > Thanks, > > Prasad Mujumdar > >
