You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Lenni Kuff <ls...@cloudera.com> on 2015/01/05 22:39:15 UTC
Re: Review Request 29393: SENTRY-599 Sentry service may report
incorrect status when service is restarting
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29393/#review66731
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
<https://reviews.apache.org/r/29393/#comment110337>
curious, why did you need to change this timeout? Does it really take 90s for the server to come up? We might want to investigate that....
- Lenni Kuff
On Dec. 24, 2014, 8:55 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29393/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2014, 8:55 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-599
> https://issues.apache.org/jira/browse/SENTRY-599
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch is fixing a issue that, when sentry service is restarting using same configuration and context. ** server.isRunning()** will report incorrect status.
> The root cause is **finally** may run after **future = serviceExecutor.submit(this);**, the status will be **NOT_STARTED** even the thrift server is started
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 6d96565
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd
>
> Diff: https://reviews.apache.org/r/29393/diff/
>
>
> Testing
> -------
>
> Unit Tests passed in apache jenkins
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29393: SENTRY-599 Sentry service may report
incorrect status when service is restarting
Posted by Lenni Kuff <ls...@cloudera.com>.
> On Jan. 5, 2015, 9:39 p.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java, line 112
> > <https://reviews.apache.org/r/29393/diff/1/?file=800135#file800135line112>
> >
> > curious, why did you need to change this timeout? Does it really take 90s for the server to come up? We might want to investigate that....
>
> Dapeng Sun wrote:
> Thank you very much for you comments, Lenni. I'm agree with you, I will continue investigating it.
Was this introduced with this change? If so, it should probably be fixed as part of this code review. If it is an unrelated intermittent issue feel free to file a JIRA and resolve that independently.
- Lenni
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29393/#review66731
-----------------------------------------------------------
On Dec. 24, 2014, 8:55 a.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29393/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2014, 8:55 a.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-599
> https://issues.apache.org/jira/browse/SENTRY-599
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch is fixing a issue that, when sentry service is restarting using same configuration and context. ** server.isRunning()** will report incorrect status.
> The root cause is **finally** may run after **future = serviceExecutor.submit(this);**, the status will be **NOT_STARTED** even the thrift server is started
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 6d96565
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd
>
> Diff: https://reviews.apache.org/r/29393/diff/
>
>
> Testing
> -------
>
> Unit Tests passed in apache jenkins
>
>
> Thanks,
>
> Dapeng Sun
>
>
Re: Review Request 29393: SENTRY-599 Sentry service may report
incorrect status when service is restarting
Posted by Dapeng Sun <da...@intel.com>.
> On 一月 6, 2015, 5:39 a.m., Lenni Kuff wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java, line 112
> > <https://reviews.apache.org/r/29393/diff/1/?file=800135#file800135line112>
> >
> > curious, why did you need to change this timeout? Does it really take 90s for the server to come up? We might want to investigate that....
Thank you very much for you comments, Lenni. I'm agree with you, I will continue investigating it.
- Dapeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29393/#review66731
-----------------------------------------------------------
On 十二月 24, 2014, 4:55 p.m., Dapeng Sun wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29393/
> -----------------------------------------------------------
>
> (Updated 十二月 24, 2014, 4:55 p.m.)
>
>
> Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, shen guoquan, Lenni Kuff, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-599
> https://issues.apache.org/jira/browse/SENTRY-599
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The patch is fixing a issue that, when sentry service is restarting using same configuration and context. ** server.isRunning()** will report incorrect status.
> The root cause is **finally** may run after **future = serviceExecutor.submit(this);**, the status will be **NOT_STARTED** even the thrift server is started
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 6d96565
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 4a6cac9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java be14afd
>
> Diff: https://reviews.apache.org/r/29393/diff/
>
>
> Testing
> -------
>
> Unit Tests passed in apache jenkins
>
>
> Thanks,
>
> Dapeng Sun
>
>