> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 78 (patched)
> > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line78>
> >
> >     Do you ever expect concurrent calls to close() or not?
> 
> Na Li wrote:
>     It is possible. I will add "synchronized" in front of it.

Under what circumstances is it possible? If it is possible, it may not be 
sufficient to add synchronized() - you may need to protect access to the 
variables as well, but I a not convinced that it is possible. Do we run our 
tests in parallel?


> On April 12, 2017, 4:50 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 96 (patched)
> > <https://reviews.apache.org/r/58221/diff/5/?file=1689530#file1689530line96>
> >
> >     I think this should never happen.
> 
> Na Li wrote:
>     From API, it could be thrown

from the API - yes, but this shouldn't happen in our case, so we should log 
something about the fact that this should never happen.


- Alexander


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


On April 12, 2017, 5:38 a.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
> 
> (Updated April 12, 2017, 5:38 a.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/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to