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/05 18:44:38 UTC
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/
-----------------------------------------------------------
(Updated May 5, 2017, 6:44 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
updated the summary of code changes.
Bugs: SENTRY-1708
https://issues.apache.org/jira/browse/SENTRY-1708
Repository: sentry
Description (updated)
-------
Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
List of sentry sentry servers
List of the address used for these servers
This is learnt so re-use the same port when the servers are re-started
API's to start/stop all the servers
API's to start/stop specific servers
API to lean the server addresses
API to update the sentry server config list for the clients to use
Diffs
-----
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java a315843
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java 53cbd00
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java b8695b2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
Diff: https://reviews.apache.org/r/58727/diff/1/
Testing
-------
Made sure that all the db-provider unit tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by Vamsee Yarlagadda <va...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review176967
-----------------------------------------------------------
First pass comments. Will take a deeper look.
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
Lines 56-62 (original), 56-59 (patched)
<https://reviews.apache.org/r/58727/#comment250506>
Can't we set the value to pass all the live sentry servers like we do on real clusters?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
Lines 141-148 (patched)
<https://reviews.apache.org/r/58727/#comment250507>
Do we handle to cleanup as part of create method?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
Lines 150 (patched)
<https://reviews.apache.org/r/58727/#comment250508>
I would prefer to see this being set by the tests itself whether they want a single server or multi server.
I was thinking for the fact that we have a good set of unit tests coverage, one could simply parameterize the tests to run the same sceanrio in both single server and multi server mode to get lot of coverage for free.
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java
Lines 256-258 (original), 382-388 (patched)
<https://reviews.apache.org/r/58727/#comment250509>
shutdownAllSentryServices() ?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
Lines 119-123 (original), 113-117 (patched)
<https://reviews.apache.org/r/58727/#comment250510>
shutdownAllSentryServices()?
- Vamsee Yarlagadda
On May 12, 2017, 3:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated May 12, 2017, 3:49 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> List of sentry sentry servers
> List of the address used for these servers
> This is learnt so re-use the same port when the servers are re-started
> API's to start/stop all the servers
> API's to start/stop specific servers
> API to lean the server addresses
> API to update the sentry server config list for the clients to use
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 17a2d1e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java 53cbd00
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
>
>
> Diff: https://reviews.apache.org/r/58727/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the db-provider unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/
-----------------------------------------------------------
(Updated Feb. 20, 2018, 11:27 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Addressed review comments.
Bugs: SENTRY-1708
https://issues.apache.org/jira/browse/SENTRY-1708
Repository: sentry
Description
-------
Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
* List of sentry sentry servers
* List of the address used for these servers
* This is learnt so re-use the same port when the servers are re-started
* API's to start/stop all the servers
* API's to start/stop specific servers
* API to learn the server addresses
* API to update the sentry server config list for the clients to use
* API to for service failover
There are some HA test classes added to demonstrate the how these abstract base classes can be used to perform HA tests.
Model HA Tests using Abstract class providing Ha capability
* TestColumnEndToEndWithHa
* TestGrantUserToRoleWithHa
* TestPrivilegeWithHAGrantOption
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/DBUpdateForwarder.java eae7861728f2bc11b4c1b44aa3b61b881a87740b
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java cf764eda1a006ce96f301e3ecb87749e05ba4a09
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java 2f2b98412e7dfdcc847ffe7975a70f452554e747
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java e7558370025c6acd83492615be093f2bd16a202b
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java edea5b64d8f98c93aafc1fe43fa97e00c2ce2948
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 96c6810baa4d554db2b7d3739a28e3ff7e8b33a0
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java 79030780c35e5bda432e3ec3f01328e627cb50a6
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java 4cd00e6672730773c74b9840247d1f4d5f7bdfe4
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationTogglingConf.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java e64f5cd687bf59133d6475c912ebdd7930601151
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 91397061e78a6524410d35643dd3a33be8353ecc
Diff: https://reviews.apache.org/r/58727/diff/5/
Changes: https://reviews.apache.org/r/58727/diff/4-5/
Testing
-------
Made sure that all the unit tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On July 13, 2017, 1:43 a.m., Na Li wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java
> > Line 37 (original), 34 (patched)
> > <https://reviews.apache.org/r/58727/diff/4/?file=1774911#file1774911line37>
> >
> > do you want to enable these tests
i can enable it.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review180389
-----------------------------------------------------------
On July 12, 2017, 4:44 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated July 12, 2017, 4:44 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> * List of sentry sentry servers
> * List of the address used for these servers
> * This is learnt so re-use the same port when the servers are re-started
> * API's to start/stop all the servers
> * API's to start/stop specific servers
> * API to learn the server addresses
> * API to update the sentry server config list for the clients to use
> * API to for service failover
>
> There are some HA test classes added to demonstrate the how these abstract base classes can be used to perform HA tests.
> Model HA Tests using Abstract class providing Ha capability
>
> * TestColumnEndToEndWithHa
> * TestGrantUserToRoleWithHa
> * TestPrivilegeWithHAGrantOption
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java fd07887
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java 3ee3724
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClientWithHa.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 2f4e8f6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java 32e67b9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 6895720
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java b416ef8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java aa99595
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java 5364937
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java f3db301
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestColumnEndToEndWithHa.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestGrantUserToRoleWithHa.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 6ea6763
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 4cfb1f7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 9139706
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrvFactory.java 9381e88
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java 7c45999
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java a52c8d6
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 80f158a
>
>
> Diff: https://reviews.apache.org/r/58727/diff/4/
>
>
> Testing
> -------
>
> Made sure that all the unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review180389
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java
Line 37 (original), 34 (patched)
<https://reviews.apache.org/r/58727/#comment255538>
do you want to enable these tests
- Na Li
On July 12, 2017, 4:44 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated July 12, 2017, 4:44 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> * List of sentry sentry servers
> * List of the address used for these servers
> * This is learnt so re-use the same port when the servers are re-started
> * API's to start/stop all the servers
> * API's to start/stop specific servers
> * API to learn the server addresses
> * API to update the sentry server config list for the clients to use
> * API to for service failover
>
> There are some HA test classes added to demonstrate the how these abstract base classes can be used to perform HA tests.
> Model HA Tests using Abstract class providing Ha capability
>
> * TestColumnEndToEndWithHa
> * TestGrantUserToRoleWithHa
> * TestPrivilegeWithHAGrantOption
>
>
> Diffs
> -----
>
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java fd07887
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java 3ee3724
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClientWithHa.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 2f4e8f6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java 32e67b9
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 6895720
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java b416ef8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java aa99595
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java 5364937
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java f3db301
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestColumnEndToEndWithHa.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestGrantUserToRoleWithHa.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 6ea6763
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 4cfb1f7
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 9139706
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrvFactory.java 9381e88
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java 7c45999
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java a52c8d6
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 80f158a
>
>
> Diff: https://reviews.apache.org/r/58727/diff/4/
>
>
> Testing
> -------
>
> Made sure that all the unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/
-----------------------------------------------------------
(Updated July 12, 2017, 4:44 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
There is one test failure doesn;t seem to be related to changes done.I'm looking into it.
Bugs: SENTRY-1708
https://issues.apache.org/jira/browse/SENTRY-1708
Repository: sentry
Description (updated)
-------
Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
* List of sentry sentry servers
* List of the address used for these servers
* This is learnt so re-use the same port when the servers are re-started
* API's to start/stop all the servers
* API's to start/stop specific servers
* API to learn the server addresses
* API to update the sentry server config list for the clients to use
* API to for service failover
There are some HA test classes added to demonstrate the how these abstract base classes can be used to perform HA tests.
Model HA Tests using Abstract class providing Ha capability
* TestColumnEndToEndWithHa
* TestGrantUserToRoleWithHa
* TestPrivilegeWithHAGrantOption
Diffs (updated)
-----
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java fd07887
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java 3ee3724
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClientWithHa.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 2f4e8f6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java 32e67b9
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java 6895720
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java b416ef8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java aa99595
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestGrantUserToRole.java 5364937
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java f3db301
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestColumnEndToEndWithHa.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestGrantUserToRoleWithHa.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 6ea6763
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 4cfb1f7
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java 9139706
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrvFactory.java 9381e88
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java 7c45999
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAclsCrud.java a52c8d6
sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 80f158a
Diff: https://reviews.apache.org/r/58727/diff/4/
Changes: https://reviews.apache.org/r/58727/diff/3-4/
Testing (updated)
-------
Made sure that all the unit tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/
-----------------------------------------------------------
(Updated May 12, 2017, 3:49 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
Fixed the test failure observed upstream and also addressed review comments.
Bugs: SENTRY-1708
https://issues.apache.org/jira/browse/SENTRY-1708
Repository: sentry
Description
-------
Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
List of sentry sentry servers
List of the address used for these servers
This is learnt so re-use the same port when the servers are re-started
API's to start/stop all the servers
API's to start/stop specific servers
API to lean the server addresses
API to update the sentry server config list for the clients to use
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 17a2d1e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java 53cbd00
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
Diff: https://reviews.apache.org/r/58727/diff/3/
Changes: https://reviews.apache.org/r/58727/diff/2-3/
Testing
-------
Made sure that all the db-provider unit tests passed.
Thanks,
kalyan kumar kalvagadda
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On May 8, 2017, 7:48 p.m., Na Li wrote:
> > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
> > Line 62 (original), 59 (patched)
> > <https://reviews.apache.org/r/58727/diff/1/?file=1700180#file1700180line62>
> >
> > When we run multiple servers, they will have multiple addresses (IP and port). How do you configure all of them, not just the first one?
All the sentry servers share the same IP but have different port. When the server comes-up for the first time, a free port is taken and then stored. When that instance of server is restarted same is re used again.
> On May 8, 2017, 7:48 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java
> > Line 25 (original), 25 (patched)
> > <https://reviews.apache.org/r/58727/diff/1/?file=1700182#file1700182line25>
> >
> > remove the line if you don't need it
Fixed in the later patches.
> On May 8, 2017, 7:48 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
> > Line 59 (original), 59 (patched)
> > <https://reviews.apache.org/r/58727/diff/1/?file=1700183#file1700183line59>
> >
> > You only check the first server here. should you test the other servers?
Scope of this Jira to extend all the base classes to have capabilty to deal with multiple serves. None of the tests are changes. They still behave the same way.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review173643
-----------------------------------------------------------
On May 12, 2017, 3:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated May 12, 2017, 3:49 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> List of sentry sentry servers
> List of the address used for these servers
> This is learnt so re-use the same port when the servers are re-started
> API's to start/stop all the servers
> API's to start/stop specific servers
> API to lean the server addresses
> API to update the sentry server config list for the clients to use
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dd37e7e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java 17a2d1e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestPrivilegeWithHAGrantOption.java 53cbd00
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
>
>
> Diff: https://reviews.apache.org/r/58727/diff/3/
>
>
> Testing
> -------
>
> Made sure that all the db-provider unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by Na Li <li...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review173643
-----------------------------------------------------------
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java
Line 62 (original), 59 (patched)
<https://reviews.apache.org/r/58727/#comment246647>
When we run multiple servers, they will have multiple addresses (IP and port). How do you configure all of them, not just the first one?
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java
Line 25 (original), 25 (patched)
<https://reviews.apache.org/r/58727/#comment246660>
remove the line if you don't need it
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java
Line 59 (original), 59 (patched)
<https://reviews.apache.org/r/58727/#comment246663>
You only check the first server here. should you test the other servers?
- Na Li
On May 5, 2017, 6:52 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 6:52 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> List of sentry sentry servers
> List of the address used for these servers
> This is learnt so re-use the same port when the servers are re-started
> API's to start/stop all the servers
> API's to start/stop specific servers
> API to lean the server addresses
> API to update the sentry server config list for the clients to use
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> 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/TestPrivilegeWithHAGrantOption.java 53cbd00
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
>
>
> Diff: https://reviews.apache.org/r/58727/diff/2/
>
>
> Testing
> -------
>
> Made sure that all the db-provider unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 25 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line25>
> >
> > Please remove unused imports, don't just comment them
I missed some of them. Corrected the rest.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Line 65 (original), 74 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line74>
> >
> > Is it final or not? Do we always start two servers?
> > The actual patch has this set to 1 - which one is correct?
It should still be 1. Based on the test appropriate classes would overide this value.
My recent patches have test failure upstream so I have not updated the review board.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 75 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line75>
> >
> > should this be final?
No it should not. They should be initialized later based on the sentryServerCount. This could updated by the child classes.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line76>
> >
> > Should this be final?
No it should not. They should be initialized later based on the sentryServerCount. This could updated by the child classes.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 120 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line120>
> >
> > This seems to be unused
This change set provides the API's needed for the failover tests to use. New tests for failover will use it.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 138 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line138>
> >
> > Why would servers be ever null?
When the all the servers are shutdown, servers is made null.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 142 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line142>
> >
> > This essentially means:
> >
> > boolean reuseCurrentConfig = !serverAddresses.isEmpty();
Yes, at the beginning of the method only.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 144 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line144>
> >
> > We already have servers allocated - why allocate again?
I have initialized it null now. Based on sentryServerCount value servers and serverAddresses are initialized.
Test classes for HA will update the sentryServerCount so that they have multiple sentry servers.
> On May 9, 2017, 12:35 a.m., Alexander Kolbasov wrote:
> > sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
> > Lines 149 (patched)
> > <https://reviews.apache.org/r/58727/diff/2/?file=1709540#file1709540line149>
> >
> > Note that even if you reuse existing configs, the new server may be started on a different port.
That is what we want to avoid. we store the serverAddresses seperately so that they can be re-used when the server is restarted.
When the server is restarted, we take port number from corresponding serverAddresses and update the configuration with the same port. It is not guarenteed that it works but the chances of the port that is been released to get assigned is pretty low.
Why do you think server may be started on a different port?
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review174254
-----------------------------------------------------------
On May 5, 2017, 6:52 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 6:52 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> List of sentry sentry servers
> List of the address used for these servers
> This is learnt so re-use the same port when the servers are re-started
> API's to start/stop all the servers
> API's to start/stop specific servers
> API to lean the server addresses
> API to update the sentry server config list for the clients to use
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> 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/TestPrivilegeWithHAGrantOption.java 53cbd00
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
>
>
> Diff: https://reviews.apache.org/r/58727/diff/2/
>
>
> Testing
> -------
>
> Made sure that all the db-provider unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/#review174254
-----------------------------------------------------------
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 25 (patched)
<https://reviews.apache.org/r/58727/#comment247384>
Please remove unused imports, don't just comment them
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Line 65 (original), 74 (patched)
<https://reviews.apache.org/r/58727/#comment247388>
Is it final or not? Do we always start two servers?
The actual patch has this set to 1 - which one is correct?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 75 (patched)
<https://reviews.apache.org/r/58727/#comment247385>
should this be final?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 76 (patched)
<https://reviews.apache.org/r/58727/#comment247386>
Should this be final?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 120 (patched)
<https://reviews.apache.org/r/58727/#comment247387>
This seems to be unused
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 138 (patched)
<https://reviews.apache.org/r/58727/#comment247389>
Why would servers be ever null?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 142 (patched)
<https://reviews.apache.org/r/58727/#comment247391>
This essentially means:
boolean reuseCurrentConfig = !serverAddresses.isEmpty();
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 144 (patched)
<https://reviews.apache.org/r/58727/#comment247392>
We already have servers allocated - why allocate again?
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java
Lines 149 (patched)
<https://reviews.apache.org/r/58727/#comment247393>
Note that even if you reuse existing configs, the new server may be started on a different port.
- Alexander Kolbasov
On May 5, 2017, 6:52 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58727/
> -----------------------------------------------------------
>
> (Updated May 5, 2017, 6:52 p.m.)
>
>
> Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
>
>
> Bugs: SENTRY-1708
> https://issues.apache.org/jira/browse/SENTRY-1708
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
> List of sentry sentry servers
> List of the address used for these servers
> This is learnt so re-use the same port when the servers are re-started
> API's to start/stop all the servers
> API's to start/stop specific servers
> API to lean the server addresses
> API to update the sentry server config list for the clients to use
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
> 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/TestPrivilegeWithHAGrantOption.java 53cbd00
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
> sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
>
>
> Diff: https://reviews.apache.org/r/58727/diff/2/
>
>
> Testing
> -------
>
> Made sure that all the db-provider unit tests passed.
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 58727: SENTRY-1708 Extend the current test classes
to handle multiple sentry servers
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58727/
-----------------------------------------------------------
(Updated May 5, 2017, 6:52 p.m.)
Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
Changes
-------
updated the base classes used for e2e tests
Bugs: SENTRY-1708
https://issues.apache.org/jira/browse/SENTRY-1708
Repository: sentry
Description
-------
Here is the abstract of the code changes done. There are basically changes to the base classes used tests to be able to handle multiple servers
List of sentry sentry servers
List of the address used for these servers
This is learnt so re-use the same port when the servers are re-started
API's to start/stop all the servers
API's to start/stop specific servers
API to lean the server addresses
API to update the sentry server config list for the clients to use
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/SentryHdfsServiceIntegrationBase.java eccf83b
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceClient.java 8959ad8
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyServiceClient.java 3b3b30e
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceClientPool.java fe4164d
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java 7c7ebab
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java a33b03a
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java a4dd8a6
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java dfd79ae
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/TestPrivilegeWithHAGrantOption.java 53cbd00
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java e2fb36a
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 054b193
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java dac1151
sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java bead003
sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java 8a01e1c
Diff: https://reviews.apache.org/r/58727/diff/2/
Changes: https://reviews.apache.org/r/58727/diff/1-2/
Testing
-------
Made sure that all the db-provider unit tests passed.
Thanks,
kalyan kumar kalvagadda