Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

2018-10-18 Thread Na Li via Review Board 

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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
Lines 1515 (patched)


Sergio is moving owner privilege update from post event listener to 
notification processor. So your code here will be affected


- Na Li


On Oct. 18, 2018, 5:38 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 18, 2018, 5:38 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  214d78c5332d4113e440b76077bb7b2a6dae81d6 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

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

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

(Updated Oct. 18, 2018, 5:38 p.m.)


Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.


Changes
---

addressed review comments.


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


Repository: sentry


Description
---

New metric should be added to track the time taken by sentry server in 
granting/updating owner privileges.


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 214d78c5332d4113e440b76077bb7b2a6dae81d6 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
 709434c388689b63d5db7d71cd6cc47952d647bf 


Diff: https://reviews.apache.org/r/68968/diff/2/

Changes: https://reviews.apache.org/r/68968/diff/1-2/


Testing
---


Thanks,

kalyan kumar kalvagadda

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

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


> On Oct. 9, 2018, 6:19 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
> > Lines 124 (patched)
> > 
> >
> > Can we have one for granting and one for revoking owner privilege?
> 
> kalyan kumar kalvagadda wrote:
> Do you see a value in having seperate merics?
> 
> Arjun Mishra wrote:
> Is there a plan in the future to allow explicit granting and revoking of 
> owner privileges? If so, its better to keep them seperated

There is no plan to provide a way to explcitly revoke owner privilege. I will 
be adding metrics for adding and updating owner privileges.


- kalyan kumar


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


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

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


> On Oct. 11, 2018, 2:52 a.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
> > Lines 124 (patched)
> > 
> >
> > We should use different metric for "granting" vs "update" since update 
> > is much more complicated and would take more time. Once we support "revoke" 
> > owner privilege, we can add a metric for that.

There is no plan to provide a way to explcitly revoke owner privilege. I will 
be adding metrics for adding and updating owner privileges.


- kalyan kumar


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


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

2018-10-10 Thread Na Li via Review Board 

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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
Lines 124 (patched)


We should use different metric for "granting" vs "update" since update is 
much more complicated and would take more time. Once we support "revoke" owner 
privilege, we can add a metric for that.


- Na Li


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

2018-10-09 Thread Arjun Mishra via Review Board 


> On Oct. 9, 2018, 6:19 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
> > Lines 124 (patched)
> > 
> >
> > Can we have one for granting and one for revoking owner privilege?
> 
> kalyan kumar kalvagadda wrote:
> Do you see a value in having seperate merics?

Is there a plan in the future to allow explicit granting and revoking of owner 
privileges? If so, its better to keep them seperated


- Arjun


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


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

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


> On Oct. 9, 2018, 6:19 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
> > Lines 124 (patched)
> > 
> >
> > Can we have one for granting and one for revoking owner privilege?

Do you see a value in having seperate merics?


- kalyan kumar


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


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Re: Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

2018-10-09 Thread Arjun Mishra via Review Board 

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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
Lines 124 (patched)


Can we have one for granting and one for revoking owner privilege?


- Arjun Mishra


On Oct. 9, 2018, 6:01 p.m., kalyan kumar kalvagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68968/
> ---
> 
> (Updated Oct. 9, 2018, 6:01 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2425
> https://issues.apache.org/jira/browse/SENTRY-2425
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> New metric should be added to track the time taken by sentry server in 
> granting/updating owner privileges.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
>  709434c388689b63d5db7d71cd6cc47952d647bf 
> 
> 
> Diff: https://reviews.apache.org/r/68968/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Review Request 68968: SENTRY-2425: Add metric to track the time taken to update the owner privilege

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

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

Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena.


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


Repository: sentry


Description
---

New metric should be added to track the time taken by sentry server in 
granting/updating owner privileges.


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
 d34340cd6a41ba01943bd3b2b42c9bddc9fa722f 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java
 709434c388689b63d5db7d71cd6cc47952d647bf 


Diff: https://reviews.apache.org/r/68968/diff/1/


Testing
---


Thanks,

kalyan kumar kalvagadda