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/05/03 19:20:05 UTC
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/
-----------------------------------------------------------
(Updated May 3, 2017, 7:20 p.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 (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java ec8676e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
Diff: https://reviews.apache.org/r/58221/diff/23/
Changes: https://reviews.apache.org/r/58221/diff/22-23/
Testing
-------
Thanks,
Na Li
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review174110
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review174176
-----------------------------------------------------------
Ship it!
Ship It!
- kalyan kumar kalvagadda
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/
-----------------------------------------------------------
(Updated May 5, 2017, 3:47 p.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 (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java 99549bc
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
Diff: https://reviews.apache.org/r/58221/diff/25/
Changes: https://reviews.apache.org/r/58221/diff/24-25/
Testing
-------
Thanks,
Na Li
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
> On May 4, 2017, 12:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
> > Line 172 (original), 172 (patched)
> > <https://reviews.apache.org/r/58221/diff/24/?file=1708101#file1708101line172>
> >
> > Understand that this delay is causing issues during tests but making it 0 may not be a best choice.
> >
> > It may take some time for the server to initialize the database and be completely up to handle the notifications.
> >
> > If 60000 milli sec is higher value, we should have some smaller value instead.
>
> Na Li wrote:
> since we prevent connecting to local meta store, HMSFollower does nothing if Hive meta store is not up. It does not hurt to have this value being zero, and allow sentry getting updates sooner.
"sentry-1753 Make HMSFollower initial delay configurable" is created
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173895
-----------------------------------------------------------
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On May 4, 2017, 12:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
> > Line 172 (original), 172 (patched)
> > <https://reviews.apache.org/r/58221/diff/24/?file=1708101#file1708101line172>
> >
> > Understand that this delay is causing issues during tests but making it 0 may not be a best choice.
> >
> > It may take some time for the server to initialize the database and be completely up to handle the notifications.
> >
> > If 60000 milli sec is higher value, we should have some smaller value instead.
>
> Na Li wrote:
> since we prevent connecting to local meta store, HMSFollower does nothing if Hive meta store is not up. It does not hurt to have this value being zero, and allow sentry getting updates sooner.
>
> Na Li wrote:
> "sentry-1753 Make HMSFollower initial delay configurable" is created
The initial delay was coming out of the blue and isn't clear why it is needed at all. What kind of server initialization are you talking about?
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173895
-----------------------------------------------------------
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
> On May 4, 2017, 12:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 131 (patched)
> > <https://reviews.apache.org/r/58221/diff/24/?file=1708098#file1708098line133>
> >
> > Can you update the comment explanining why this check was needed? I think you are using the HiveConf.ConfVars.METASTOREURIS.varname to make sure that the Hive server is already UP while running unit tests.
Yes. We want to prevent HMSFollower from using local meta store.
> On May 4, 2017, 12:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
> > Lines 182 (patched)
> > <https://reviews.apache.org/r/58221/diff/24/?file=1708100#file1708100line182>
> >
> > As part of shutdownAndAwaitTermination, shutdown is called first to stop new tasks to getting scheduled.
> >
> > Later awaitTermination is called with timeout value while actually the interval at which the tasks are scheduled giving it enough time to terminate before calling shutdownNow.
> >
> > When awaitTermination is called for the second time to log an error, i feel the timeout can be 0, as we need not give any more time for the task to temrinate.
If HMSFollower is getting full snapshot, it may take longer. So wait for additional time is useful.
> On May 4, 2017, 12:52 p.m., kalyan kumar kalvagadda wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
> > Line 172 (original), 172 (patched)
> > <https://reviews.apache.org/r/58221/diff/24/?file=1708101#file1708101line172>
> >
> > Understand that this delay is causing issues during tests but making it 0 may not be a best choice.
> >
> > It may take some time for the server to initialize the database and be completely up to handle the notifications.
> >
> > If 60000 milli sec is higher value, we should have some smaller value instead.
since we prevent connecting to local meta store, HMSFollower does nothing if Hive meta store is not up. It does not hurt to have this value being zero, and allow sentry getting updates sooner.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173895
-----------------------------------------------------------
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173895
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 131 (patched)
<https://reviews.apache.org/r/58221/#comment246973>
Can you update the comment explanining why this check was needed? I think you are using the HiveConf.ConfVars.METASTOREURIS.varname to make sure that the Hive server is already UP while running unit tests.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 182 (patched)
<https://reviews.apache.org/r/58221/#comment246974>
As part of shutdownAndAwaitTermination, shutdown is called first to stop new tasks to getting scheduled.
Later awaitTermination is called with timeout value while actually the interval at which the tasks are scheduled giving it enough time to terminate before calling shutdownNow.
When awaitTermination is called for the second time to log an error, i feel the timeout can be 0, as we need not give any more time for the task to temrinate.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Line 172 (original), 172 (patched)
<https://reviews.apache.org/r/58221/#comment246975>
Understand that this delay is causing issues during tests but making it 0 may not be a best choice.
It may take some time for the server to initialize the database and be completely up to handle the notifications.
If 60000 milli sec is higher value, we should have some smaller value instead.
- kalyan kumar kalvagadda
On May 4, 2017, 5:48 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 4, 2017, 5:48 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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/24/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/
-----------------------------------------------------------
(Updated May 4, 2017, 5:48 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 (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java ec8676e
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
Diff: https://reviews.apache.org/r/58221/diff/24/
Changes: https://reviews.apache.org/r/58221/diff/23-24/
Testing
-------
Thanks,
Na Li
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
> On May 4, 2017, 3:07 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 75 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707216#file1707216line75>
> >
> > This shouldn't be here. It is the code from another JIRA.
I can remove it
> On May 4, 2017, 3:07 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 132 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707216#file1707216line138>
> >
> > This shouldn't be here - it is a code from another JIRA
If I remove this code, HMSFollwer will use Hive local meta store, and the unit test will fail
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173861
-----------------------------------------------------------
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
> On May 4, 2017, 3:07 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 75 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707216#file1707216line75>
> >
> > This shouldn't be here. It is the code from another JIRA.
>
> Na Li wrote:
> I can remove it
I need it here to avoid unit test failure when using local meta store. Since my code change changes the timing, it is more likely to use local meta store if we don't prevent it here.
> On May 4, 2017, 3:07 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Line 95 (original)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707216#file1707216line96>
> >
> > Please add a comment explaining why these two constructors are different.
Kalyan mentioned that he has created a Jira to address the difference of the constructors, so can I leave it as it is for this jira?
> On May 4, 2017, 3:07 a.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
> > Line 172 (original), 172 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707219#file1707219line172>
> >
> > Why do we need this delay in the first place? Can it be 0?
I think the reason it was very large is because if the Hive meta store is not up yet, HMSFollower could connect to the local meta store, and the test will fail. So large initial delay is a work around of this issue. Since we avoid connect to the local meta store, we don't need such large delay any more.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173861
-----------------------------------------------------------
On May 5, 2017, 3:47 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 3:47 p.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 99549bc
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 2ebe561
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 212c465
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b5247d0
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/25/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173861
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 75 (patched)
<https://reviews.apache.org/r/58221/#comment246910>
This shouldn't be here. It is the code from another JIRA.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 95 (original)
<https://reviews.apache.org/r/58221/#comment246911>
Please add a comment explaining why these two constructors are different.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 132 (patched)
<https://reviews.apache.org/r/58221/#comment246912>
This shouldn't be here - it is a code from another JIRA
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 313 (patched)
<https://reviews.apache.org/r/58221/#comment246927>
We may end up with hmsfollower without the executor here.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 314 (patched)
<https://reviews.apache.org/r/58221/#comment246925>
finally should be on previous line
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 318 (patched)
<https://reviews.apache.org/r/58221/#comment246924>
It can't be null here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 322 (patched)
<https://reviews.apache.org/r/58221/#comment246928>
This should be moved to the finally clause of the previous try block
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 326 (patched)
<https://reviews.apache.org/r/58221/#comment246926>
catch should be on previous line
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 331 (patched)
<https://reviews.apache.org/r/58221/#comment246932>
Since we are using conf, for consistency let's add conf to the arguments
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 332 (patched)
<https://reviews.apache.org/r/58221/#comment246929>
This can be asserted with precondition
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 363 (patched)
<https://reviews.apache.org/r/58221/#comment246930>
we should set sentrystorecleanservice to null here
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 368 (patched)
<https://reviews.apache.org/r/58221/#comment246931>
we an assert this with precondition
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 24 (patched)
<https://reviews.apache.org/r/58221/#comment246916>
Please avoid wildcard imports for such utility classes, import what you need.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 157 (patched)
<https://reviews.apache.org/r/58221/#comment246919>
Nit, this is a full sentence, so should start with a capital letter and end with a dot.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 165 (patched)
<https://reviews.apache.org/r/58221/#comment246917>
This can be package-private
Also, please use shorter line. Official limit is 120.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 166 (patched)
<https://reviews.apache.org/r/58221/#comment246918>
Why would we call it with pool = null? This probably should be Preconditions.assertNonNull.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 177 (patched)
<https://reviews.apache.org/r/58221/#comment246921>
This can be replaced with be {} instead of %s and format.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 180 (patched)
<https://reviews.apache.org/r/58221/#comment246922>
rename ie to ignored
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Line 172 (original), 172 (patched)
<https://reviews.apache.org/r/58221/#comment246913>
Why do we need this delay in the first place? Can it be 0?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 421 (original), 424 (patched)
<https://reviews.apache.org/r/58221/#comment246914>
Can you explain this change?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 441 (original), 444 (patched)
<https://reviews.apache.org/r/58221/#comment246915>
Please use string constant for col1_role instead of copying it in multiple places.
- Alexander Kolbasov
On May 3, 2017, 7:20 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 3, 2017, 7:20 p.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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/23/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On May 4, 2017, 5:47 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
> > Lines 322 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707217#file1707217line352>
> >
> > it would be easier to read the code by keeping this section outside of previous finally block
This isn't about being easier to read. What may happen is that you get exception and the rest of the code isn't executed. The question is what code *must* be executed.
> On May 4, 2017, 5:47 a.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
> > Lines 165 (patched)
> > <https://reviews.apache.org/r/58221/diff/23/?file=1707218#file1707218line165>
> >
> > Other functions in this class are public. Why do we want to limit it to just package private?
Usually we do not want to make anything public unless we need to. This particular function is new and there is no need for it to be public.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173866
-----------------------------------------------------------
On May 4, 2017, 5:48 a.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 4, 2017, 5:48 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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/24/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173866
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 95 (original)
<https://reviews.apache.org/r/58221/#comment246936>
I need to revert it back. The code should be in constructor here, not in the constructor used for testing.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 132 (patched)
<https://reviews.apache.org/r/58221/#comment246937>
I will remove it
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Line 171 (original), 171 (patched)
<https://reviews.apache.org/r/58221/#comment246938>
I will move it to start service
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 313 (patched)
<https://reviews.apache.org/r/58221/#comment246940>
I will check both
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 314 (patched)
<https://reviews.apache.org/r/58221/#comment246941>
will update
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 318 (patched)
<https://reviews.apache.org/r/58221/#comment246942>
it does not hurt to check here.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 322 (patched)
<https://reviews.apache.org/r/58221/#comment246943>
it would be easier to read the code by keeping this section outside of previous finally block
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 326 (patched)
<https://reviews.apache.org/r/58221/#comment246944>
will move
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 331 (patched)
<https://reviews.apache.org/r/58221/#comment246945>
will add
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 332 (patched)
<https://reviews.apache.org/r/58221/#comment246946>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 363 (patched)
<https://reviews.apache.org/r/58221/#comment246947>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 368 (patched)
<https://reviews.apache.org/r/58221/#comment246948>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 24 (patched)
<https://reviews.apache.org/r/58221/#comment246950>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 157 (patched)
<https://reviews.apache.org/r/58221/#comment246951>
Will fix
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 165 (patched)
<https://reviews.apache.org/r/58221/#comment246952>
Other functions in this class are public. Why do we want to limit it to just package private?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 166 (patched)
<https://reviews.apache.org/r/58221/#comment246953>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 177 (patched)
<https://reviews.apache.org/r/58221/#comment246954>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 180 (patched)
<https://reviews.apache.org/r/58221/#comment246955>
will change
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Line 172 (original), 172 (patched)
<https://reviews.apache.org/r/58221/#comment246957>
I believe it can be zero if it does not create local meta store.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 421 (original), 424 (patched)
<https://reviews.apache.org/r/58221/#comment246958>
I want to avoid two tests interfere with each other if they are running in parallel. I was trying to check what caused the test failure
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 441 (original), 444 (patched)
<https://reviews.apache.org/r/58221/#comment246959>
will do
- Na Li
On May 3, 2017, 7:20 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 3, 2017, 7:20 p.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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/23/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173863
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Line 171 (original), 171 (patched)
<https://reviews.apache.org/r/58221/#comment246933>
It looks like we can move it back to the start service since you fixed the problem with HMSFollower.
- Alexander Kolbasov
On May 3, 2017, 7:20 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 3, 2017, 7:20 p.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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/23/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58221/#review173795
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 107 (patched)
<https://reviews.apache.org/r/58221/#comment246830>
We have to move the code here to avoid running full snapshot multiple times. after the first time, getting empty full snapshot at run() will cause exception.
SENTRY-1712 Add capability to force Sentry to get full snapshot from HMS will provide more flexibility to get full snapshot.
Example of the exception if this code is put in run() instead of constructor.
2017-05-03 09:58:42,209 (pool-5-thread-1) [ERROR - org.apache.sentry.service.thrift.HMSFollower.run(HMSFollower.java:304)] Caught unexpected exception in HMSFollower! Caused by: AuthzObj: default already exists
org.apache.sentry.core.common.exception.SentryAlreadyExistsException: AuthzObj: default already exists
at org.apache.sentry.provider.db.service.persistent.SentryStore.createAuthzPathsMappingCore(SentryStore.java:2541)
at org.apache.sentry.provider.db.service.persistent.SentryStore.access$3000(SentryStore.java:101)
at org.apache.sentry.provider.db.service.persistent.SentryStore$42.execute(SentryStore.java:2515)
at org.apache.sentry.provider.db.service.persistent.TransactionManager.executeTransaction(TransactionManager.java:114)
at org.apache.sentry.provider.db.service.persistent.TransactionManager.executeTransactionWithRetry(TransactionManager.java:181)
at org.apache.sentry.provider.db.service.persistent.SentryStore.persistFullPathsImage(SentryStore.java:2511)
at org.apache.sentry.service.thrift.HMSFollower.run(HMSFollower.java:269)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Line 172 (original), 172 (patched)
<https://reviews.apache.org/r/58221/#comment246829>
since we start HMSFollower in SentryService.runServer() now, having a long delay will cause the unit test to fail. The reason is that HMSFollower has not run before the test starts and finishes, so no permission is obtained.
- Na Li
On May 3, 2017, 7:20 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
>
> (Updated May 3, 2017, 7:20 p.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 ec8676e
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 132db63
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java ce73358
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 834ed41
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java 1ace07c
>
>
> Diff: https://reviews.apache.org/r/58221/diff/23/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Na Li
>
>