> 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 > >