You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Alexander Kolbasov <ak...@gmail.com> on 2016/11/18 22:50:21 UTC
Review Request 53905: SENTRY-1518: Add metrics for SentryStore
transactions
-----------------------------------------------------------
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
Re: Review Request 53905: SENTRY-1518: Add metrics for SentryStore
transactions
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/53905/#comment228111>
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
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
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
Posted by Vadim Spector <vs...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Alexander Kolbasov <ak...@gmail.com>.
> On Nov. 18, 2016, 11:59 p.m., Vadim Spector wrote:
> > 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.
I don't think we need to distinguish at this level - I do want to see failures grouped by exception type and this is captured here. In general these counters are useful as a way to get a feel for a system health, so a simpler model is better.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53905/#review156357
-----------------------------------------------------------
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
Posted by Vadim Spector <vs...@cloudera.com>.
-----------------------------------------------------------
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
>
>