You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li <li...@cloudera.com> on 2017/08/18 00:51:37 UTC
Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/
-----------------------------------------------------------
Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
Repository: sentry
Description
-------
when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
Diff: https://reviews.apache.org/r/61724/diff/1/
Testing
-------
unit tests
Thanks,
Na Li
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/#review183224
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Aug. 18, 2017, 3:32 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61724/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 3:32 p.m.)
>
>
> Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
>
> In HMSFollower.createFullSnapshot(), you can see the notification ID is saved into data store regardless HDFS is enabled or not.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 2d6b92a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
>
>
> Diff: https://reviews.apache.org/r/61724/diff/2/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/
-----------------------------------------------------------
(Updated Aug. 18, 2017, 3:32 p.m.)
Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
Repository: sentry
Description
-------
when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
In HMSFollower.createFullSnapshot(), you can see the notification ID is saved into data store regardless HDFS is enabled or not.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 2d6b92a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
Diff: https://reviews.apache.org/r/61724/diff/2/
Changes: https://reviews.apache.org/r/61724/diff/1-2/
Testing
-------
unit tests
Thanks,
Na Li
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/
-----------------------------------------------------------
(Updated Aug. 18, 2017, 3:10 p.m.)
Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
Repository: sentry
Description (updated)
-------
when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
In HMSFollower.createFullSnapshot(), you can see the notification ID is saved into data store regardless HDFS is enabled or not.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
Diff: https://reviews.apache.org/r/61724/diff/1/
Testing
-------
unit tests
Thanks,
Na Li
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Na Li <li...@cloudera.com>.
> On Aug. 18, 2017, 2:40 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 3033 (patched)
> > <https://reviews.apache.org/r/61724/diff/1/?file=1799894#file1799894line3033>
> >
> > This method is the same as isNotificationIDTableEmpty(). Can we we just make isNotificationIDTableEmpty() public and use it instead?
will merge these two functions.
> On Aug. 18, 2017, 2:40 p.m., Sergio Pena wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
> > Line 784 (original), 785 (patched)
> > <https://reviews.apache.org/r/61724/diff/1/?file=1799896#file1799896line785>
> >
> > Do we need this mock? This method is not used anymore on the HMSFollower. I think is safe to remove it and any other mock to isAuthPathsMappingEmpty() as well.
It is not called. But I want to mock it and to verify it is not called in case someone changes the behavior. We are testing what should be done and what should not be done.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/#review183214
-----------------------------------------------------------
On Aug. 18, 2017, 3:10 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61724/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 3:10 p.m.)
>
>
> Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
>
> In HMSFollower.createFullSnapshot(), you can see the notification ID is saved into data store regardless HDFS is enabled or not.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
>
>
> Diff: https://reviews.apache.org/r/61724/diff/1/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Sergio Pena <se...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/#review183214
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 3033 (patched)
<https://reviews.apache.org/r/61724/#comment259232>
This method is the same as isNotificationIDTableEmpty(). Can we we just make isNotificationIDTableEmpty() public and use it instead?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
Line 784 (original), 785 (patched)
<https://reviews.apache.org/r/61724/#comment259234>
Do we need this mock? This method is not used anymore on the HMSFollower. I think is safe to remove it and any other mock to isAuthPathsMappingEmpty() as well.
- Sergio Pena
On Aug. 18, 2017, 12:51 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61724/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 12:51 a.m.)
>
>
> Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
>
>
> Diff: https://reviews.apache.org/r/61724/diff/1/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 61724: SENTRY-1890: HMSFollower keep getting full
snapshot when HDFS is disabled
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61724/#review183216
-----------------------------------------------------------
Ship it!
Ship It!
- Vamsee Yarlagadda
On Aug. 18, 2017, 12:51 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61724/
> -----------------------------------------------------------
>
> (Updated Aug. 18, 2017, 12:51 a.m.)
>
>
> Review request for sentry, Brian Towles, Sergio Pena, and Vamsee Yarlagadda.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> when deciding if we should take full snapshot of HMS, check notification table intead of the authorization path table because notification table is always updated regardless HDFS is enabled or not
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e9de73a
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 1d8200d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java 48b009f
>
>
> Diff: https://reviews.apache.org/r/61724/diff/1/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Na Li
>
>