----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review173895 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java Lines 131 (patched) <https://reviews.apache.org/r/58221/#comment246973> Can you update the comment explanining why this check was needed? I think you are using the HiveConf.ConfVars.METASTOREURIS.varname to make sure that the Hive server is already UP while running unit tests. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java Lines 182 (patched) <https://reviews.apache.org/r/58221/#comment246974> As part of shutdownAndAwaitTermination, shutdown is called first to stop new tasks to getting scheduled. Later awaitTermination is called with timeout value while actually the interval at which the tasks are scheduled giving it enough time to terminate before calling shutdownNow. When awaitTermination is called for the second time to log an error, i feel the timeout can be 0, as we need not give any more time for the task to temrinate. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java Line 172 (original), 172 (patched) <https://reviews.apache.org/r/58221/#comment246975> Understand that this delay is causing issues during tests but making it 0 may not be a best choice. It may take some time for the server to initialize the database and be completely up to handle the notifications. If 60000 milli sec is higher value, we should have some smaller value instead. - kalyan kumar kalvagadda On May 4, 2017, 5:48 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58221/ > ----------------------------------------------------------- > > (Updated May 4, 2017, 5:48 a.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, kalyan kumar > kalvagadda, and Sergio Pena. > > > Repository: sentry > > > Description > ------- > > SENTRY-1649 move HMS follower to runServer > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > ec8676e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 132db63 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java > ce73358 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 834ed41 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 1ace07c > > > Diff: https://reviews.apache.org/r/58221/diff/24/ > > > Testing > ------- > > > Thanks, > > Na Li > >
