You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Prasad Mujumdar <pr...@cloudera.com> on 2015/02/10 18:41:48 UTC
Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/
-----------------------------------------------------------
Review request for sentry and Dapeng Sun.
Bugs: SENTRY-648
https://issues.apache.org/jira/browse/SENTRY-648
Repository: sentry
Description
-------
- Update test framework to allow creating multiple services
- Sentry service abstraction to enable start/stop of individual servers from test
- Added test cases
- Fixed issues ZK session leak found with the new tests
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
Diff: https://reviews.apache.org/r/30839/diff/
Testing
-------
Added New tests -
- Normal Sentry operations with HA enabled
- Testing service failover
Thanks,
Prasad Mujumdar
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java, line 91
> > <https://reviews.apache.org/r/30839/diff/2/?file=860511#file860511line91>
> >
> > Hi Prasad, the change will create a curatorFramework for each renew operation, I think a close method should be added at InvocationHandler, I added a close method to HAClientInvocationHandler in the connection pool jira, but patch is under reviewing by Lenni, so I'm agreed with the change currently
I did go down that path initially and added called closed() from all client access paths, but then decided to keep it simple. Given that the renew will only happen for failover, it's shouldn't be a major overhead.
Anyway, I too will take a look at the connection pool patch. Thanks!
> On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java, line 93
> > <https://reviews.apache.org/r/30839/diff/2/?file=860516#file860516line93>
> >
> > If the first sentry server got an exception when do stopping, other servers may never execute stop()
Added exception handling in stopAll()
> On Feb. 12, 2015, 6:55 a.m., Dapeng Sun wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java, line 99
> > <https://reviews.apache.org/r/30839/diff/2/?file=860516#file860516line99>
> >
> > Nit: shutdown() may be a little ambiguous, how about clean() or close()
renamed to close()
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/#review71944
-----------------------------------------------------------
On Feb. 10, 2015, 9:22 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30839/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2015, 9:22 p.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-648
> https://issues.apache.org/jira/browse/SENTRY-648
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Update test framework to allow creating multiple services
> - Sentry service abstraction to enable start/stop of individual servers from test
> - Added test cases
> - Fixed issues ZK session leak found with the new tests
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30839/diff/
>
>
> Testing
> -------
>
> Added New tests -
> - Normal Sentry operations with HA enabled
> - Testing service failover
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Dapeng Sun <da...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/#review71944
-----------------------------------------------------------
Ship it!
Hi Prasad, most of the patch looks good to me, some minor comments below.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
<https://reviews.apache.org/r/30839/#comment117860>
Hi Prasad, the change will create a curatorFramework for each renew operation, I think a close method should be added at InvocationHandler, I added a close method to HAClientInvocationHandler in the connection pool jira, but patch is under reviewing by Lenni, so I'm agreed with the change currently
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment117864>
It seems here is 180s
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment118075>
need we catch the exception of stop?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment118074>
If the first sentry server got an exception when do stopping, other servers may never execute stop()
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment118064>
Nit: shutdown() may be a little ambiguous, how about clean() or close()
- Dapeng Sun
On 二月 11, 2015, 5:22 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30839/
> -----------------------------------------------------------
>
> (Updated 二月 11, 2015, 5:22 a.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-648
> https://issues.apache.org/jira/browse/SENTRY-648
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Update test framework to allow creating multiple services
> - Sentry service abstraction to enable start/stop of individual servers from test
> - Added test cases
> - Fixed issues ZK session leak found with the new tests
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30839/diff/
>
>
> Testing
> -------
>
> Added New tests -
> - Normal Sentry operations with HA enabled
> - Testing service failover
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/#review72986
-----------------------------------------------------------
Ship it!
Ship It!
- Lenni Kuff
On Feb. 12, 2015, 8:40 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30839/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2015, 8:40 a.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-648
> https://issues.apache.org/jira/browse/SENTRY-648
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Update test framework to allow creating multiple services
> - Sentry service abstraction to enable start/stop of individual servers from test
> - Added test cases
> - Fixed issues ZK session leak found with the new tests
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrvFactory.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30839/diff/
>
>
> Testing
> -------
>
> Added New tests -
> - Normal Sentry operations with HA enabled
> - Testing service failover
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/
-----------------------------------------------------------
(Updated Feb. 12, 2015, 8:40 a.m.)
Review request for sentry and Dapeng Sun.
Changes
-------
Addressed review feedback
Bugs: SENTRY-648
https://issues.apache.org/jira/browse/SENTRY-648
Repository: sentry
Description
-------
- Update test framework to allow creating multiple services
- Sentry service abstraction to enable start/stop of individual servers from test
- Added test cases
- Fixed issues ZK session leak found with the new tests
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrvFactory.java PRE-CREATION
Diff: https://reviews.apache.org/r/30839/diff/
Testing
-------
Added New tests -
- Normal Sentry operations with HA enabled
- Testing service failover
Thanks,
Prasad Mujumdar
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On Feb. 11, 2015, 9:04 p.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java, line 77
> > <https://reviews.apache.org/r/30839/diff/2/?file=860514#file860514line77>
> >
> > Add one negative test case for privileges on an object.
It covers negative cases like all verifying errors with all services down.
> On Feb. 11, 2015, 9:04 p.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java, line 44
> > <https://reviews.apache.org/r/30839/diff/2/?file=860516#file860516line44>
> >
> > it's not immediately obvious that setting > 1 servers automatically enables HA. Mention this behavior. It might be more clear if you have two params: "bool enableHa, int numServers" and maybe throw an error if enableHa=false && numServers > 1.
> > Also throw an IllegalArgumentException if numServers <= 0.
Added comments and error for numServers <= 0.
> On Feb. 11, 2015, 9:04 p.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java, line 26
> > <https://reviews.apache.org/r/30839/diff/2/?file=860517#file860517line26>
> >
> > Since we don't support external perhaps just leave this out for now, but mention how we might use it in the future?
A dummy external server implementation will be in a followup ticket which should include the details. will file the jira shortly.
> On Feb. 11, 2015, 9:04 p.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java, line 34
> > <https://reviews.apache.org/r/30839/diff/2/?file=860518#file860518line34>
> >
> > Should this be part of the interface? Seems like it is only applicable in the HA case.
It should be part of the interface. The clients would need the ZK connection string in order to connect to Sentry service.
> On Feb. 11, 2015, 9:04 p.m., Lenni Kuff wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java, line 36
> > <https://reviews.apache.org/r/30839/diff/2/?file=860518#file860518line36>
> >
> > what's the difference between stopAll and shutdown?
Shutdonw closes the ZK along with all the running servers. The SentrySrv is not usable after shutdown. Added the comment on the interface describing the difference. Also added a check to validate the state for all APIs.
On Feb. 11, 2015, 9:04 p.m., Prasad Mujumdar wrote:
> > This is very cool. Would it also be possible to allow running all the existing tests with HA enabled (no failover)?
Added a property sentry.e2etest.enable.service.ha that can be set to true via maven commandline to run tests with HA. Since the patch changed the framework to use the new SentrySrv abstraction, this can be used to run any tests with HA. Just that it slows donw the test runs and currently not enabled by default.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/#review72018
-----------------------------------------------------------
On Feb. 10, 2015, 9:22 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30839/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2015, 9:22 p.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-648
> https://issues.apache.org/jira/browse/SENTRY-648
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Update test framework to allow creating multiple services
> - Sentry service abstraction to enable start/stop of individual servers from test
> - Added test cases
> - Fixed issues ZK session leak found with the new tests
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30839/diff/
>
>
> Testing
> -------
>
> Added New tests -
> - Normal Sentry operations with HA enabled
> - Testing service failover
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/#review72018
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java
<https://reviews.apache.org/r/30839/#comment117956>
While you are here can you add a brief comment to this class?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java
<https://reviews.apache.org/r/30839/#comment117943>
perhaps add this in a "finally" block so we ensure it gets closed even in the event of an error?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
<https://reviews.apache.org/r/30839/#comment117944>
rename "future" to something more descriptive of what it actually is.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
<https://reviews.apache.org/r/30839/#comment117945>
Comment on what this test suite is covering.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java
<https://reviews.apache.org/r/30839/#comment117946>
Add one negative test case for privileges on an object.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
<https://reviews.apache.org/r/30839/#comment117947>
nit getSentrySrv
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment117954>
comment - only set if HA is enabled (numServers>1).
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java
<https://reviews.apache.org/r/30839/#comment117953>
it's not immediately obvious that setting > 1 servers automatically enables HA. Mention this behavior. It might be more clear if you have two params: "bool enableHa, int numServers" and maybe throw an error if enableHa=false && numServers > 1.
Also throw an IllegalArgumentException if numServers <= 0.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java
<https://reviews.apache.org/r/30839/#comment117951>
Consistent naming - sentrySrv or sentryServ or sentryService. all of them are fine, but pick one and use it consistently.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java
<https://reviews.apache.org/r/30839/#comment117952>
Since we don't support external perhaps just leave this out for now, but mention how we might use it in the future?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
<https://reviews.apache.org/r/30839/#comment117948>
Comment on public interface and all public methods. Clients should know how to implement these.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
<https://reviews.apache.org/r/30839/#comment117949>
Should this be part of the interface? Seems like it is only applicable in the HA case.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java
<https://reviews.apache.org/r/30839/#comment117955>
what's the difference between stopAll and shutdown?
This is very cool. Would it also be possible to allow running all the existing tests with HA enabled (no failover)?
- Lenni Kuff
On Feb. 10, 2015, 9:22 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30839/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2015, 9:22 p.m.)
>
>
> Review request for sentry and Dapeng Sun.
>
>
> Bugs: SENTRY-648
> https://issues.apache.org/jira/browse/SENTRY-648
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Update test framework to allow creating multiple services
> - Sentry service abstraction to enable start/stop of individual servers from test
> - Added test cases
> - Fixed issues ZK session leak found with the new tests
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/30839/diff/
>
>
> Testing
> -------
>
> Added New tests -
> - Normal Sentry operations with HA enabled
> - Testing service failover
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 30839: SENTRY-648: Add e2e tests for Sentry HA
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30839/
-----------------------------------------------------------
(Updated Feb. 10, 2015, 9:22 p.m.)
Review request for sentry and Dapeng Sun.
Changes
-------
Fix test failures
Bugs: SENTRY-648
https://issues.apache.org/jira/browse/SENTRY-648
Repository: sentry
Description
-------
- Update test framework to allow creating multiple services
- Sentry service abstraction to enable start/stop of individual servers from test
- Added test cases
- Fixed issues ZK session leak found with the new tests
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ServiceManager.java 2274f00
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HAClientInvocationHandler.java c6e265f
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java 02788aa
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ddc5930
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestHaEnd2End.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java d08b4ee
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentryServFactory.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java PRE-CREATION
Diff: https://reviews.apache.org/r/30839/diff/
Testing
-------
Added New tests -
- Normal Sentry operations with HA enabled
- Testing service failover
Thanks,
Prasad Mujumdar