Re: Review Request 49777: SENTRY-1321: Implement HMSFollower in Sentry service which reads the NotificationLog entries

2016-07-27 Thread Sravya Tirukkovalur 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49777/
---

(Updated July 27, 2016, 11:58 p.m.)


Review request for sentry and Hao Hao.


Changes
---

Adding unit tests. Adding more negative tests in a follow on jira to unblock 
dependant patches.


Repository: sentry


Description
---

- Adding HMSFollower and changes in Sentryservice to use it.
- Test changes - start sentryservice before hive
- Moving HiveAuthzConf to sentry-binding-hive-conf, so that sentry-provider-db 
does not have to depend on sentry-binding-hive-commong which creates a circular 
dependency
- Moving messaging/json to sentry-binding-hive-follower to avoid circular 
dependency between sentry-hive-binding and sentry-provider-db


Diffs (updated)
-

  sentry-binding/pom.xml 30bca146786a833391fa44c62e036777b694a7b1 
  sentry-binding/sentry-binding-hive-common/pom.xml 
18b422d5a688e636af4e01b382fa3e5677ac884b 
  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
 ad19b3754527e25c6509571a47f3e31a077b9e56 
  sentry-binding/sentry-binding-hive-conf/pom.xml PRE-CREATION 
  
sentry-binding/sentry-binding-hive-conf/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java
 PRE-CREATION 
  sentry-binding/sentry-binding-hive-follower/pom.xml PRE-CREATION 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterPartitionMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/JSONAlterTableMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAddPartitionMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterPartitionMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONAlterTableMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateDatabaseMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONCreateTableMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropDatabaseMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropPartitionMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONDropTableMessage.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java
  
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java
  
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryInvalidHMSEventException.java
 PRE-CREATION 
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryInvalidInputException.java
 903eddc60e12ae80c005bf8153ba5356532ac1b6 
  sentry-provider/sentry-provider-db/pom.xml 
b8143ffa3adca9e47e7cb092131d65064d57c86b 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java
 f54f161b381088285486a5ca74972f93ee620547 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 51dde0e5fcfb9dd93a47351efa3a78692bb60202 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
 abc3f58d21bb774427a34399b6e9f51a37ba51db 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
 PRE-CREATION 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
 439b9de1fc85957bb3b26de145dbf9bd365c5347 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java
 7dc3d0f30583b9edd16015218b199e298d192e8c 
  
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java
 847da45dcc884d7d573b10a18d3122292754c8ad 

Diff: https://reviews.apache.org/r/49777/diff/


Testing
---

Testing not complete yet. Submmitting a preview of changes for review as this 
work also is important for SENTRY-1371


Thanks,

Sravya Tirukkovalur

Re: Review Request 50537: SENTRY-1413:Changes to get the Fencer working with Oracle and MySQL

2016-07-27 Thread Rahul Sharma 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50537/
---

(Updated July 27, 2016, 10:59 p.m.)


Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Repository: sentry


Description
---

SENTRY-1413:Changes to get the Fencer working with Oracle and MySQL


Diffs (updated)
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SqlAccessor.java
 9879e67 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
 e32e1db 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatus.java
 b1c4b31 

Diff: https://reviews.apache.org/r/50537/diff/


Testing
---

Tested fencing with mysql and oracle.


Thanks,

Rahul Sharma

Review Request 50537: SENTRY-1413:Changes to get the Fencer working with Oracle and MySQL

2016-07-27 Thread Rahul Sharma 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50537/
---

Review request for sentry, Hao Hao and Sravya Tirukkovalur.


Repository: sentry


Description
---

SENTRY-1413:Changes to get the Fencer working with Oracle and MySQL


Diffs
-

  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SqlAccessor.java
 9879e6743a09bdf8b72204d30db43cfbfeb83bed 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
 e32e1db5bbb92d35ad4063e9410c583231743edf 

Diff: https://reviews.apache.org/r/50537/diff/


Testing
---

Tested fencing with mysql and oracle.


Thanks,

Rahul Sharma

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

2016-07-27 Thread Anne Yu 


> On July 27, 2016, 9:37 p.m., Rahul Sharma wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > 
> >
> > Are you making that configurable anywhere, like number of servers to be 
> > used? . If yes, then if you commit that code, I can change this case.

Yeah, I am going to make it configurable passing from maven; default value will 
still be 2. In this case, you might want to make it a loop to look for active 
server id.


- Anne


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50404/#review143825
---


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

2016-07-27 Thread Anne Yu 


> On July 26, 2016, 6:39 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java,
> >  line 42
> > 
> >
> > It would be better to test like the below:
> > 
> > 1. stop the active server;
> > 2. assert active server is stopped correctly; give test a time out, in 
> > case in a freezing mode;
> > 3. ensure passive server to become active; meaning the new active 
> > server is the previous passive one;
> 
> Rahul Sharma wrote:
> So this is just for a single service unit case, to verify that the server 
> is down. I have added a similar case to what you suggest in the 
> TestFailover.java

ic, sounds good. Thanks for clarification.


- Anne


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50404/#review143579
---


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

2016-07-27 Thread Anne Yu 


> On July 26, 2016, 7:16 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > 
> >
> > Will it be possible to have more than 2 servers?
> 
> Rahul Sharma wrote:
> I just used the InternalSrv, which current supports 2 for HA scenario.

oh, I am changing that to >= 2 case; I meant by design, seems ha supports >= 2 
case. If so test needs to accommodate it; if not, you can ignore my comment.


- Anne


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50404/#review143588
---


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

2016-07-27 Thread Rahul Sharma 


> On July 26, 2016, 7:16 p.m., Anne Yu wrote:
> > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java,
> >  line 47
> > 
> >
> > Will it be possible to have more than 2 servers?

I just used the InternalSrv, which current supports 2 for HA scenario.


- Rahul


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50404/#review143588
---


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>

Re: Review Request 50404: SENTRY-1415: Provide a hook to stop the active sentry sevice

2016-07-27 Thread Rahul Sharma 


> On July 26, 2016, 6:39 p.m., Anne Yu wrote:
> > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java,
> >  line 42
> > 
> >
> > It would be better to test like the below:
> > 
> > 1. stop the active server;
> > 2. assert active server is stopped correctly; give test a time out, in 
> > case in a freezing mode;
> > 3. ensure passive server to become active; meaning the new active 
> > server is the previous passive one;

So this is just for a single service unit case, to verify that the server is 
down. I have added a similar case to what you suggest in the TestFailover.java


- Rahul


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50404/#review143579
---


On July 26, 2016, 12:18 a.m., Rahul Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50404/
> ---
> 
> (Updated July 26, 2016, 12:18 a.m.)
> 
> 
> Review request for sentry, Anne Yu and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1415: Provide a hook to stop the active sentry sevice
> 
> 
> Diffs
> -
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Activator.java
>  730dbb1b6e290c54ad60a5074009fad778d7b77c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatus.java
>  e32e1db5bbb92d35ad4063e9410c583231743edf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  51dde0e5fcfb9dd93a47351efa3a78692bb60202 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestActivator.java
>  PRE-CREATION 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/ha/TestFailover.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Rahul Sharma
> 
>