> On Jan. 6, 2017, 1:52 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java, > > line 67 > > <https://reviews.apache.org/r/55190/diff/1/?file=1596843#file1596843line67> > > > > This should be made final for consistency.
HAContext class is restored to what it was before with some small changes > On Jan. 6, 2017, 1:52 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java, > > line 123 > > <https://reviews.apache.org/r/55190/diff/1/?file=1596843#file1596843line123> > > > > The "use ..." javadoc for the "get..." method sounds a bit incoherent. It is like that in the original version. Will add some comment. > On Jan. 6, 2017, 1:52 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java, > > line 149 > > <https://reviews.apache.org/r/55190/diff/1/?file=1596843#file1596843line149> > > > > Make this a proper javadoc please? > > > > Also, is it an established naming scheme, where "HAServerContext" means > > a HAContext with ACLs checked? Maybe getSecureHAContext() or some such? That's how it was before - I'd rather not change it. > On Jan. 6, 2017, 1:52 a.m., Misha Dmitriev wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java, > > line 170 > > <https://reviews.apache.org/r/55190/diff/1/?file=1596843#file1596843line170> > > > > Does this code have an official line length limit? Usually there is > > some, like 100 characters. This line and several others in this method look > > very long, which would make working with them harder in the future. I don't think there is an official limit, but I split long lines. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55190/#review160624 ----------------------------------------------------------- On Jan. 4, 2017, 10:51 p.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55190/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2017, 10:51 p.m.) > > > Review request for sentry, Colin Ma, Misha Dmitriev, Dapeng Sun, Hao Hao, > kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1583 > https://issues.apache.org/jira/browse/SENTRY-1583 > > > Repository: sentry > > > Description > ------- > > - Restored HAContext class > - Removed Fencing support from HA > - Removed Activator, Activators, and LeaderStatus classes > - Renamed LeaderStatusAdaptor to LeaderStatusMonitor which is now the sole > class responsible for leader status monitoring. > > > Diffs > ----- > > pom.xml df26edf74c04fffcd9fbc3da07e949362eb94728 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java > c094058142706a6b564c54fa69ddff5f1e2e5048 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java > f80605c4d68ef266a24e65d14f51066388b48417 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java > c3df4d8a6568f236dc5e05ab190cd1f484f7967a > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activators.java > 2926eeb97ec450f12d0d08086165c091709ead99 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java > 2467921b3cdc9ebd267d308d5cdcce52a836f207 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusAdaptor.java > e87b3d13dcf2d10eb7ef00d21b8fa8846b4d16c0 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > a9d49f86961f1ced1af40afcfe6172f884a23d44 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > d80fa1e71c165b7f1faf1c50c451e049d76b770b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java > 60cfc735832433fb4dbeae1c2d617dd713fc3f3e > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java > bc375e32685dcbcbf0b6398856dceced3de789e6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java > c796fabc21005479fa6afc687cf1df7af76538a9 > > Diff: https://reviews.apache.org/r/55190/diff/ > > > Testing > ------- > > Performed manual testing with two instances of Sentry running. Tried the > following actions: > > - Sending SIGUSR2 to processes which changes leadership status (verified via > log messages and emtrics) > - Killing active server - new sserver is elected as a leader > - Restarting ZK service - a leader is elected once ZK service is back. > > > Thanks, > > Alexander Kolbasov > >
