> On June 22, 2016, 12:45 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java, > > line 33 > > <https://reviews.apache.org/r/48761/diff/2/?file=1426679#file1426679line33> > > > > Add "It handles both the HA and non-HA case.", to be more specific in > > the comment?
OK. > On June 22, 2016, 12:45 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java, > > line 127 > > <https://reviews.apache.org/r/48761/diff/2/?file=1426679#file1426679line127> > > > > Should we specify the sentry server name in the log? OK > On June 22, 2016, 12:45 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > line 172 > > <https://reviews.apache.org/r/48761/diff/2/?file=1426681#file1426681line172> > > > > Probably move the start into call? This depends on later changes. We need to get this patch in first. > On June 22, 2016, 12:45 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > line 308 > > <https://reviews.apache.org/r/48761/diff/2/?file=1426681#file1426681line308> > > > > Should leaderstatus.close() be called here? yeah > On June 22, 2016, 12:45 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java, > > line 181 > > <https://reviews.apache.org/r/48761/diff/2/?file=1426686#file1426686line181> > > > > Should we start a sentry server here? Or we just testing a general > > server? This is just a test of LeaderStatus. We will have end-to-end tests for HA later when more is implemented. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48761/#review138965 ----------------------------------------------------------- On June 21, 2016, 4:18 p.m., Colin McCabe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48761/ > ----------------------------------------------------------- > > (Updated June 21, 2016, 4:18 p.m.) > > > Review request for sentry. > > > Bugs: SENTRY-1316 > https://issues.apache.org/jira/browse/SENTRY-1316 > > > Repository: sentry > > > Description > ------- > > Implement Sentry leadership election > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientFactory.java > 6c9c8bb > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/ha/HdfsHAClientInvocationHandler.java > 6138b8c > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePluginWithHA.java > 6476a01 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarder.java > 7387281 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/UpdateForwarderWithHA.java > 574627c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java > 9f921d4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java > d97a07e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 6883bf4 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java > 48ee66a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java > 3a38b24 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 32a4044 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryServiceDiscovery.java > 7cbcc11 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java > 07d74b5 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > ced9d1c > > Diff: https://reviews.apache.org/r/48761/diff/ > > > Testing > ------- > > > Thanks, > > Colin McCabe > >
