Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

2016-11-30 Thread Alexander Kolbasov 

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

(Updated Dec. 1, 2016, 3:08 a.m.)


Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, kalyan kumar 
kalvagadda, Sravya Tirukkovalur, Vamsee Yarlagadda, and Vadim Spector.


Repository: sentry


Description
---

SENTRY-1518: Add metrics for SentryStore transactions


Diffs (updated)
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
 3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 

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


Testing
---


Thanks,

Alexander Kolbasov

Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

2016-11-30 Thread Hao Hao 

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


Fix it, then Ship it!





sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
 (line 74)


Rename to failedTransactionsCount.


- Hao Hao


On Nov. 18, 2016, 10:50 p.m., Alexander Kolbasov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53905/
> ---
> 
> (Updated Nov. 18, 2016, 10:50 p.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya 
> Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1518: Add metrics for SentryStore transactions
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 
> 
> Diff: https://reviews.apache.org/r/53905/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>

Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

2016-11-19 Thread Vadim Spector 

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


Ship it!




Ship It!

- Vadim Spector


On Nov. 18, 2016, 10:50 p.m., Alexander Kolbasov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53905/
> ---
> 
> (Updated Nov. 18, 2016, 10:50 p.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya 
> Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1518: Add metrics for SentryStore transactions
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 
> 
> Diff: https://reviews.apache.org/r/53905/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>

Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

2016-11-18 Thread Vadim Spector 

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



Since executeTransactionWithRetry() calls executeTransaction(), and both APIs 
are public, I wonder if we care about separate counters for both, to 
distinguish between intermittent and more serious failures.

For the client code, executeTransactionWithRetry() is successful even if it 
some calls to executeTransaction() failed. With two separate counts, if we take 
the number of failed calls to executeTransactionWithRetry() multiplied by 
_transactionRetryMax_, and subtract it from the number of failed 
executeTransaction() calls, we get the number of failed _direct_ calls to 
executeTransaction(), which gives us a complete picture.

Just a thought, not sure if it really matters.

- Vadim Spector


On Nov. 18, 2016, 10:50 p.m., Alexander Kolbasov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53905/
> ---
> 
> (Updated Nov. 18, 2016, 10:50 p.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya 
> Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1518: Add metrics for SentryStore transactions
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
>  3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 
> 
> Diff: https://reviews.apache.org/r/53905/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>

Review Request 53905: SENTRY-1518: Add metrics for SentryStore transactions

2016-11-18 Thread Alexander Kolbasov 

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

Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya 
Tirukkovalur, and Vadim Spector.


Repository: sentry


Description
---

SENTRY-1518: Add metrics for SentryStore transactions


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/TransactionManager.java
 3d0a6ab0b514cb9fbead4b8e527f7cb84865ce28 

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


Testing
---


Thanks,

Alexander Kolbasov