> On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java, > > line 91 > > <https://reviews.apache.org/r/30839/diff/2/?file=860511#file860511line91> > > > > Hi Prasad, the change will create a curatorFramework for each renew > > operation, I think a close method should be added at InvocationHandler, I > > added a close method to HAClientInvocationHandler in the connection pool > > jira, but patch is under reviewing by Lenni, so I'm agreed with the change > > currently
I did go down that path initially and added called closed() from all client access paths, but then decided to keep it simple. Given that the renew will only happen for failover, it's shouldn't be a major overhead. Anyway, I too will take a look at the connection pool patch. Thanks! > On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java, > > line 93 > > <https://reviews.apache.org/r/30839/diff/2/?file=860516#file860516line93> > > > > If the first sentry server got an exception when do stopping, other > > servers may never execute stop() Added exception handling in stopAll() > On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java, > > line 99 > > <https://reviews.apache.org/r/30839/diff/2/?file=860516#file860516line99> > > > > Nit: shutdown() may be a little ambiguous, how about clean() or close() renamed to close() - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30839/#review71944 ----------------------------------------------------------- On Feb. 10, 2015, 9:22 p.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30839/ > ----------------------------------------------------------- > > (Updated Feb. 10, 2015, 9:22 p.m.) > > > Review request for sentry and Dapeng Sun. > > > Bugs: SENTRY-648 > https://issues.apache.org/jira/browse/SENTRY-648 > > > Repository: sentry > > > Description > ------- > > - Update test framework to allow creating multiple services > - Sentry service abstraction to enable start/stop of individual servers from > test > - Added test cases > - Fixed issues ZK session leak found with the new tests > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java > 2274f00 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java > c6e265f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 02788aa > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > ddc5930 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > d08b4ee > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/30839/diff/ > > > Testing > ------- > > Added New tests - > - Normal Sentry operations with HA enabled > - Testing service failover > > > Thanks, > > Prasad Mujumdar > >
