> On Feb. 16, 2017, 10:47 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java,
> >  line 89
> > <https://reviews.apache.org/r/56720/diff/2/?file=1638136#file1638136line89>
> >
> >     That's wrong - isLeader is a snapshot of the state. You need to pass 
> > actual leaderMonitor.
> >     
> >     Also, it is weird that we are creating SentryStore in HMSFollower - 
> > something is wrong here.

Good point. I think we should not put ```leaderMonitor``` into SentryStore 
though, as it leaks the impl details (i.e., which service is leader and how to 
handle failover between leader / follower) from o.a.s.service.thrift. 

I will add a ```public void SentryStore#purgeDeltaTables()``` function, and let 
the leader from {{o.a.s.service.thrift}} to call it.


> On Feb. 16, 2017, 10:47 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java,
> >  line 230
> > <https://reviews.apache.org/r/56720/diff/2/?file=1638135#file1638135line230>
> >
> >     it should use leaderMonitor.isLeader() method instead.

As mentioned below, I think leaderMonitor should not be in 
```o.a.s.service.model```.


- Lei


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


On Feb. 16, 2017, 3:37 p.m., Lei Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56720/
> -----------------------------------------------------------
> 
> (Updated Feb. 16, 2017, 3:37 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov and Hao Hao.
> 
> 
> Bugs: SENTRY-1611
>     https://issues.apache.org/jira/browse/SENTRY-1611
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Provide a background service to periodically purge path and permission delta 
> change tables.
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  4cc123fd0de05a055b4276d213eafc0eb070213d 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  783c0fcf5e338897b56c4e17ba1669394c02e8a3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  e10ddfd0372c985e5a8bd6d5b3ca07d5cb618a68 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  dfaac1555fe729efab3f8fdccb914978001fee56 
> 
> Diff: https://reviews.apache.org/r/56720/diff/
> 
> 
> Testing
> -------
> 
> Run `mvn test -Dtest=TestSentryStore`
> 
> 
> Thanks,
> 
> Lei Xu
> 
>

Reply via email to