> 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
>
>