> On 十一月 1, 2014, 9:35 a.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java,
> > lines 263-264
> > <https://reviews.apache.org/r/27425/diff/2/?file=746586#file746586line263>
> >
> > Nit: This is redundant as both the conditions are being checked again?
>
> Dapeng Sun wrote:
> Sravya, Thank you for your time. I think **status** is used for thrift
> server, so we should check both status of thrift service and web service.
> correct me if I misunderstood.
>
> Sravya Tirukkovalur wrote:
> That is true, but we are again testing for thrift service aliveness on
> line 267 and web server aliveness on line 279 isnt it?
Yes, you're right, I will remove **if (status == Status.NOT_STARTED &&
!isWebServerRunning()) { return; }** Thank you for your review.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27425/#review59448
-----------------------------------------------------------
On 十一月 1, 2014, 9:01 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27425/
> -----------------------------------------------------------
>
> (Updated 十一月 1, 2014, 9:01 a.m.)
>
>
> Review request for sentry, Arun Suresh, Lenni Kuff, Prasad Mujumdar, and
> Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-513
> https://issues.apache.org/jira/browse/SENTRY-513
>
>
> Repository: sentry
>
>
> Description
> -------
>
> ````java
> public synchronized void stop() throws Exception{
> if (status == Status.NOT_STARTED) {
> return;
> }
> LOGGER.info("Attempting to stop...");
>
> if (thriftServer.isServing()) {
> thriftServer.stop(); // XXX If got exception here, the code after it
> will never be executed.
> }
> thriftServer = null;
> stopSentryWebServer();
> status = Status.NOT_STARTED;
> LOGGER.info("Stopped...");
> }
> ````
>
> if {{thriftServer}} got an exception when do stop, the method
> {{stopSentryWebServer()}} will be passed
>
>
> Diffs
> -----
>
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryWebServer.java
> 0243c48
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> ec17480
>
> Diff: https://reviews.apache.org/r/27425/diff/
>
>
> Testing
> -------
>
> All Unit Tests passed in my jenkins.
>
>
> Thanks,
>
> Dapeng Sun
>
>