> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 221 (patched)
> > <https://reviews.apache.org/r/58221/diff/10/?file=1692735#file1692735line249>
> >
> >     SentryStore cleaner need not be restarted every time sentry service is 
> > stoped and started.

in normal operation, stop() is the last call to clean up the resources. And we 
need to stop SentryStore cleaner.


> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 222 (patched)
> > <https://reviews.apache.org/r/58221/diff/10/?file=1692735#file1692735line250>
> >
> >     I feel we should start call startHMSFollower at the end of runServer 
> > function. If there are no issues in starting the sentry service, we can 
> > start HMSFolloeer at the end.

The original code to start HMSFollower is in constructor. When I move it to 
runServer(), putting it at the beginning is the closest approach to the 
original in terms of the ordering. I am not familar with the system, and moving 
it to the end of runServer() may cause some unknown problems.


> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Line 373 (original), 472 (patched)
> > <https://reviews.apache.org/r/58221/diff/10/?file=1692735#file1692735line501>
> >
> >     HMSFollower need not be stopped when sentry service is stoped.

in normal operation, stop() is the last call to clean up the resources. And we 
need to stop HMSFollower.


> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Line 375 (original), 474 (patched)
> > <https://reviews.apache.org/r/58221/diff/10/?file=1692735#file1692735line503>
> >
> >     SentryStore cleaner need not be restarted every time sentry service is 
> > stoped and started.

Since we have to stop SentryStore cleaner at stop(). When test calls start() 
after stop(), we have to start it in start(). Otherwise, it won't work, and 
will cause test failure.


- Na


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


On April 14, 2017, 8:36 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
> 
> (Updated April 14, 2017, 8:36 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
>  16676fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  132db63 
> 
> 
> Diff: https://reviews.apache.org/r/58221/diff/11/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to