You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arjun Mishra via Review Board <no...@reviews.apache.org> on 2019/01/03 18:37:41 UTC
Re: Review Request 69596: SENTRY-1797: SentryKerberosContext should
use
periodic executor instead of managing periodic execution via run() method.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69596/#review211644
-----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java
Lines 40 (patched)
<https://reviews.apache.org/r/69596/#comment297046>
Can you explain the reasoning behind 30L? Before we would check ever sec
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java
Line 47 (original), 46 (patched)
<https://reviews.apache.org/r/69596/#comment297049>
Going by SentryService class, could we use the ScheduledExecutorService class since it doesn't have to be boxed? Thoughts?
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java
Line 148 (original), 132 (patched)
<https://reviews.apache.org/r/69596/#comment297048>
Can we could also keep the renewerThreadFactor, since it has an identifiable thread name "kerberos-renewer-%d". Then we could pass in the threadfactory into Executors.newScheduledThreadPool(1,renewerThreadFactory) method. Thoughts?
- Arjun Mishra
On Dec. 19, 2018, 5:25 p.m., Haley Reeve wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69596/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2018, 5:25 p.m.)
>
>
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
>
>
> Bugs: SENTRY-1797
> https://issues.apache.org/jira/browse/SENTRY-1797
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The SentryKerberosContext class tries to simulate periodic executor using while loop with a sleep in the run() method. Instead it should use periodic executor.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java efb8ae6
>
>
> Diff: https://reviews.apache.org/r/69596/diff/1/
>
>
> Testing
> -------
>
> Ran unit tests.
>
>
> Thanks,
>
> Haley Reeve
>
>