> On July 28, 2016, 10:18 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java,
> >  lines 273-278
> > <https://reviews.apache.org/r/50578/diff/2/?file=1456877#file1456877line273>
> >
> >     It is not very clear what we are trying to achieve here. Seems like the 
> > order of endpoints matter as per this logic?
> 
> Hao Hao wrote:
>     Here is after reach the retry max, checking for all exception received is 
> it caused by standby or active is unreachable. The order of endpoints does 
> not matter.

I think we should log.error these exceptions, even if we throw only the last 
exception. As connection to each could have failed due to a different reason. 
And can we make the exception messages a bit more descriptive to make it more 
actionable. Basically, if we have unreachable exceptions, then there is some 
thing weird going on like either a kerberos issue or service name is wrong etc. 
On the other hand if there is no active, even if there are standbys, then it 
can mean passive is taking too long to become active. We should also come back 
to this once we support {ACTIVE, STANDBY, STARTING} states.


- Sravya


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


On July 29, 2016, 12:47 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50578/
> -----------------------------------------------------------
> 
> (Updated July 29, 2016, 12:47 a.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The sentry client should retry configurable times RPCs if it gets a 
> SentryStandbyException
> 
> 
> Diffs
> -----
> 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryStandbyException.java
>  acb906fc8b792db70dc09821c0a6ad7cf7361807 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  1039e6ebfde32ec8db82287458b8b4b6a23136a3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/PoolClientInvocationHandler.java
>  a35bf1d4781c7424283610c7fa67ede990e35fef 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientFactory.java
>  48ee66a4132113d49dc16c419bd496dd1572b59d 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceClientPoolFactory.java
>  3a38b243e79e007c0e15ee947828301136608525 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java
>  51bba31bddc4404506bbe42f9f8d444b36d5056c 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceWithInvalidMsgSize.java
>  15eab15b79f9057d173e0ea4ea64e152ade698d6 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestPoolClientInvocationHandler.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50578/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Reply via email to