> On April 19, 2017, 9 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 321 (patched)
> > <https://reviews.apache.org/r/58221/diff/17/?file=1694265#file1694265line353>
> >
> >     I think you may need to wait for termination of HMSFOllower here. 
> > Otherwice it may be still running when you call close() and this may cause 
> > troubles.
> 
> Na Li wrote:
>     Do you mean waiting for termination of hmsFollowerExecutor before call 
> HMSFOllower.close()?
>     
>     The code has tried to wait for termination of the executor, and only 
> close HMSFollower at end. What else can I do to wait further?
>     
>         long timeoutValue = 
> conf.getLong(ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS,
>                 ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT);
>         try {
>           hmsFollowerExecutor.shutdown();
>           boolean terminated = 
> hmsFollowerExecutor.awaitTermination(timeoutValue, TimeUnit.MILLISECONDS);
>           if (!terminated) {
>             hmsFollowerExecutor.shutdownNow();
>           }
>         } catch (SecurityException e) {
>           LOGGER.error("MSFollower executor shutdown failed at 
> SecurityException which should never happen", e);
>         } catch (InterruptedException e) {
>           LOGGER.error("Could not await HMSFollower executor to terminate", 
> e);
>     
>           try {
>             hmsFollowerExecutor.shutdownNow();
>           } catch (SecurityException ex) {
>             LOGGER.error("HMSFollower executor shutdownNow failed at 
> SecurityException which should never happen", ex);
>           }
>         }
>         finally {
>           hmsFollowerExecutor = null;
>     
>           // close connections
>           if (hmsFollower != null) {
>             hmsFollower.close();
>           }
>         }
> 
> Na Li wrote:
>     I added "hmsFollowerExecutor.awaitTermination(timeoutValue, 
> TimeUnit.MILLISECONDS)" after shutdownNow().

Sasha, can you take a look at this and see if it is fixed?


- Na


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review172406
-----------------------------------------------------------


On April 25, 2017, 9:15 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
> 
> (Updated April 25, 2017, 9:15 p.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
>  57b7f88 
>   
> 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 
> 
> 
> Diff: https://reviews.apache.org/r/58221/diff/22/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to