> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 350 (patched) > > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line380> > > > > This is most likely wrong. What does the cancel of the future do? What > > task does it actually cancel given that this is periodic executor? U think > > you should just use shutdown on the executor instead of this.
It seems to be not exactly wrong, but makes things more complicated then they should be. You need to shutdown the executor on exit anyway, so why cancel scheduling using futures? - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review171666 ----------------------------------------------------------- On April 12, 2017, 3:47 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58221/ > ----------------------------------------------------------- > > (Updated April 12, 2017, 3:47 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 > 6c14f5e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > 132db63 > > > Diff: https://reviews.apache.org/r/58221/diff/5/ > > > Testing > ------- > > > Thanks, > > Na Li > >
