You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda <kk...@cloudera.com> on 2017/05/24 01:36:54 UTC
Review Request 59510: SENTRY-1774 HMSFollower should always depend on
persisted information to decide is full snapshot is needed
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/
-----------------------------------------------------------
Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Bugs: SENTRY-1774
https://issues.apache.org/jira/browse/SENTRY-1774
Repository: sentry
Description
-------
HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
Diff: https://reviews.apache.org/r/59510/diff/1/
Testing
-------
Added new testto check new functionality added and also ran HMSFollower tests
Thanks,
kalyan kumar kalvagadda
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176227
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176231
-----------------------------------------------------------
Fix it, then Ship it!
Good idea to make this more generic!
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2891 (patched)
<https://reviews.apache.org/r/59510/#comment249575>
Please camelize the name to isTableEmpty
- Alexander Kolbasov
On May 26, 2017, 9:30 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 26, 2017, 9:30 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/3/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176230
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On May 26, 2017, 9:30 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 26, 2017, 9:30 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/3/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/
-----------------------------------------------------------
(Updated May 26, 2017, 9:30 p.m.)
Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Made the API more generic so that it can be re-used.
Bugs: SENTRY-1774
https://issues.apache.org/jira/browse/SENTRY-1774
Repository: sentry
Description
-------
HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
Diff: https://reviews.apache.org/r/59510/diff/3/
Changes: https://reviews.apache.org/r/59510/diff/2-3/
Testing
-------
Added new test to check new functionality added and also ran HMSFollower tests
Thanks,
kalyan kumar kalvagadda
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On May 26, 2017, 8:13 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2890 (patched)
> > <https://reviews.apache.org/r/59510/diff/2/?file=1732547#file1732547line2890>
> >
> > The name of the method does not match with the result. It says 'getFirst..' but the caller might expect more than 1 result due to the list? We should only return the one object instead. Notice that the object might be null.
> >
> > Another idea is just return True or False if at least 1 row is found. Do you expect that there are other callers which require only one row of the table? If not, why not keeping this block of code inside isAuthzPathsMappingEmpty()? It is only a 2-3 lines of code.
with the query.setRange(0, 1) the query only returns the first record even if there are more. The returned list could either be empty OR have one entry in it.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176222
-----------------------------------------------------------
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176222
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2890 (patched)
<https://reviews.apache.org/r/59510/#comment249563>
The name of the method does not match with the result. It says 'getFirst..' but the caller might expect more than 1 result due to the list? We should only return the one object instead. Notice that the object might be null.
Another idea is just return True or False if at least 1 row is found. Do you expect that there are other callers which require only one row of the table? If not, why not keeping this block of code inside isAuthzPathsMappingEmpty()? It is only a 2-3 lines of code.
- Sergio Pena
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review176105
-----------------------------------------------------------
Ship it!
Ship It!
- Vamsee Yarlagadda
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/
-----------------------------------------------------------
(Updated May 25, 2017, 1:41 p.m.)
Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Addressed review comments.
Bugs: SENTRY-1774
https://issues.apache.org/jira/browse/SENTRY-1774
Repository: sentry
Description
-------
HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
Diff: https://reviews.apache.org/r/59510/diff/2/
Changes: https://reviews.apache.org/r/59510/diff/1-2/
Testing
-------
Added new test to check new functionality added and also ran HMSFollower tests
Thanks,
kalyan kumar kalvagadda
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On May 24, 2017, 7:56 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2814 (patched)
> > <https://reviews.apache.org/r/59510/diff/1/?file=1730920#file1730920line2814>
> >
> > According to the spec, getFirstMAuthzPathsMappingCore never returns null, so the first check is not needed and the second check is cleaner with using isEmpty().
Will fix
> On May 24, 2017, 7:56 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2815 (patched)
> > <https://reviews.apache.org/r/59510/diff/1/?file=1730920#file1730920line2815>
> >
> > Please do not use if statements to return true/false values, return conditions directly.
> >
> > return some_boolean_condition
Will fix
> On May 24, 2017, 7:56 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Line 97 (original), 96 (patched)
> > <https://reviews.apache.org/r/59510/diff/1/?file=1730921#file1730921line97>
> >
> > With your change, why do we still need this?
Not needed. Will fix
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review175982
-----------------------------------------------------------
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review175982
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2814 (patched)
<https://reviews.apache.org/r/59510/#comment249331>
According to the spec, getFirstMAuthzPathsMappingCore never returns null, so the first check is not needed and the second check is cleaner with using isEmpty().
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2815 (patched)
<https://reviews.apache.org/r/59510/#comment249332>
Please do not use if statements to return true/false values, return conditions directly.
return some_boolean_condition
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 97 (original), 96 (patched)
<https://reviews.apache.org/r/59510/#comment249329>
With your change, why do we still need this?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 103 (original), 101 (patched)
<https://reviews.apache.org/r/59510/#comment249330>
This return is redundand now
- Alexander Kolbasov
On May 24, 2017, 1:36 a.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 24, 2017, 1:36 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/1/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
> On May 24, 2017, 3:23 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 268 (patched)
> > <https://reviews.apache.org/r/59510/diff/1/?file=1730921#file1730921line272>
> >
> > Is it possible that before previous full update request is returned, another full update request is sent?
There is only one thread processing the HMSFollower right. If that's the case, there shouldn't be multiple snapshot requests pending.
- Vamsee
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review175939
-----------------------------------------------------------
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On May 24, 2017, 3:23 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 268 (patched)
> > <https://reviews.apache.org/r/59510/diff/1/?file=1730921#file1730921line272>
> >
> > Is it possible that before previous full update request is returned, another full update request is sent?
>
> Vamsee Yarlagadda wrote:
> There is only one thread processing the HMSFollower right. If that's the case, there shouldn't be multiple snapshot requests pending.
As, Executors for HMSFollower has only one thread, this will not be possible.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review175939
-----------------------------------------------------------
On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 25, 2017, 1:41 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/2/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 59510: SENTRY-1774 HMSFollower should always
depend on persisted information to decide is full snapshot is needed
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59510/#review175939
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 268 (patched)
<https://reviews.apache.org/r/59510/#comment249281>
Is it possible that before previous full update request is returned, another full update request is sent?
- Na Li
On May 24, 2017, 1:36 a.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59510/
> -----------------------------------------------------------
>
> (Updated May 24, 2017, 1:36 a.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1774
> https://issues.apache.org/jira/browse/SENTRY-1774
>
>
> Repository: sentry
>
>
> Description
> -------
>
> HMSFollower should always depend on persisted information to decide is full snapshot is needed instead of depending on in-memory information.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 8450402
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 58e8881
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 440b0e9
>
>
> Diff: https://reviews.apache.org/r/59510/diff/1/
>
>
> Testing
> -------
>
> Added new test to check new functionality added and also ran HMSFollower tests
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>