> On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java, > > line 140 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451990#file1451990line140> > > > > It is not clear to me what is the intent of close versus becomeStandby?
If I have two servers S1(active) and S2(passive), If I call becomeStandby on S1, it becomes passive and S2 is selected as the leader. If I now call becomeStandby on S2, S1 becomes the leader. Whereas Close() will close the leaderStatusAdopter and the curator hook is removed from the S1. If we later call close on S2 too, there wont be any service for curator to pick as active. > On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java, > > line 89 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451989#file1451989line89> > > > > Add @VisibleForTesting annotation, so that the intent of this public > > method is clear? Ok > On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > line 335 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451991#file1451991line335> > > > > Should this be Becoming standby instead? We are forcing the leader to becomeStandby(), so become seems right? > On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > line 339 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451991#file1451991line339> > > > > Fix the comment? Ok > On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java, > > line 476 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451991#file1451991line476> > > > > @VisibleForTesting ? Ok > On July 25, 2016, 8:37 p.m., Sravya Tirukkovalur wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java, > > line 77 > > <https://reviews.apache.org/r/50404/diff/1/?file=1451993#file1451993line77> > > > > nit: not -> now Ok - Rahul ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50404/#review143423 ----------------------------------------------------------- On July 25, 2016, 6:43 p.m., Rahul Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50404/ > ----------------------------------------------------------- > > (Updated July 25, 2016, 6:43 p.m.) > > > Review request for sentry, Anne Yu and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-1415: Provide a hook to stop the active sentry sevice > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java > 730dbb1b6e290c54ad60a5074009fad778d7b77c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java > e32e1db5bbb92d35ad4063e9410c583231743edf > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 51dde0e5fcfb9dd93a47351efa3a78692bb60202 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/50404/diff/ > > > Testing > ------- > > > Thanks, > > Rahul Sharma > >
