Re: Review Request 69204: SENTRY-2437: When granting privileges a single transaction per grant causes long delays

2018-10-30 Thread Arjun Mishra via Review Board 

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

(Updated Oct. 30, 2018, 5:39 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
---

Final diff for history


Bugs: SENTRY-2437
https://issues.apache.org/jira/browse/SENTRY-2437


Repository: sentry


Description
---

Currently sentry creates a transaction for each TSentryPrivilege object it 
needs to grant. If the list of privileges is very large creating a single 
transaction for each significantly affects performance. This is particularly 
impactful for tables with large columns and if a user grants privileges to many 
of those columns


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 01b363479 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
 97407fff5 
  
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 1d87b0b66 
  
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
 f2f38a38f 


Diff: https://reviews.apache.org/r/69204/diff/4/

Changes: https://reviews.apache.org/r/69204/diff/3-4/


Testing
---

$ mvn -f sentry-service/sentry-service-server/pom.xml test 
-Dtest=TestSentryStore
$ mvn -f sentry-service/sentry-service-server/pom.xml test 
-Dtest=TestHMSFollowerSentryStoreIntegration


Thanks,

Arjun Mishra

Re: Review Request 69204: SENTRY-2437: When granting privileges a single transaction per grant causes long delays

2018-10-30 Thread Arjun Mishra via Review Board 

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

(Updated Oct. 30, 2018, 5:37 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Bugs: SENTRY-2437
https://issues.apache.org/jira/browse/SENTRY-2437


Repository: sentry


Description
---

Currently sentry creates a transaction for each TSentryPrivilege object it 
needs to grant. If the list of privileges is very large creating a single 
transaction for each significantly affects performance. This is particularly 
impactful for tables with large columns and if a user grants privileges to many 
of those columns


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 01b363479 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
 97407fff5 
  
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 1d87b0b66 
  
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
 f2f38a38f 


Diff: https://reviews.apache.org/r/69204/diff/3/

Changes: https://reviews.apache.org/r/69204/diff/2-3/


Testing
---

$ mvn -f sentry-service/sentry-service-server/pom.xml test 
-Dtest=TestSentryStore
$ mvn -f sentry-service/sentry-service-server/pom.xml test 
-Dtest=TestHMSFollowerSentryStoreIntegration


Thanks,

Arjun Mishra

Re: Review Request 69201: SENTRY-2436 Add annotations for classes that are used in binding as public

2018-10-30 Thread Sergio Pena via Review Board 

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


Ship it!




Ship It!

- Sergio Pena


On Oct. 29, 2018, 4:45 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69201/
> ---
> 
> (Updated Oct. 29, 2018, 4:45 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Sentry bindings are using some of the classes in sentry.core and common. 
> These classes are annotated as public
> 
> 
> Diffs
> -
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/ActiveRoleSet.java
>  c24a6cde 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/Subject.java
>  88457c0d 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/Column.java
>  e36b09a1 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAction.java
>  c5842d98 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizable.java
>  4ce01b2c 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/Database.java
>  e8dc1406 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/Server.java
>  41693c25 
>   
> sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/Table.java
>  5a981588 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  aecfe5b5 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java
>  761fb527 
> 
> 
> Diff: https://reviews.apache.org/r/69201/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean install
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 69204: SENTRY-2437: When granting privileges a single transaction per grant causes long delays

2018-10-30 Thread kalyan kumar kalvagadda via Review Board 

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


Fix it, then Ship it!




With this change you are able to levarage the batching capability of 
datanucelus which makes it faster. 
Please fix them and ship it.


sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 734 (original), 732 (patched)


Could you please add a comment for this method explaning it. It will make 
code readable. It was missing before. It's good to add them when we get a 
chance.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Line 1002 (original), 993 (patched)


Could you please add a comment for this method explaning it. It will make 
code readable. It was missing before. It's good to add them when we get a 
chance.


- kalyan kumar kalvagadda


On Oct. 29, 2018, 9:35 p.m., Arjun Mishra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69204/
> ---
> 
> (Updated Oct. 29, 2018, 9:35 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2437
> https://issues.apache.org/jira/browse/SENTRY-2437
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Currently sentry creates a transaction for each TSentryPrivilege object it 
> needs to grant. If the list of privileges is very large creating a single 
> transaction for each significantly affects performance. This is particularly 
> impactful for tables with large columns and if a user grants privileges to 
> many of those columns
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  01b363479 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java
>  97407fff5 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  1d87b0b66 
>   
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  f2f38a38f 
> 
> 
> Diff: https://reviews.apache.org/r/69204/diff/2/
> 
> 
> Testing
> ---
> 
> $ mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestSentryStore
> $ mvn -f sentry-service/sentry-service-server/pom.xml test 
> -Dtest=TestHMSFollowerSentryStoreIntegration
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>