> On Aug. 30, 2017, 5:36 p.m., Alexander Kolbasov wrote: > > My concern with this change is that we will delay normal client failover by > > 3 seconds. Can we initially retry without delay and then delay if it failed?
This will not happen. The actual logic of client trying all the servers is under SentryTransportPool. Every retry under RetryClientConnectionHandler attempts connecting to all servers exhaustively (without any delay). So we don't have to worry about adding delays for quick failover. > On Aug. 30, 2017, 5:36 p.m., Alexander Kolbasov wrote: > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java > > Lines 157 (patched) > > <https://reviews.apache.org/r/61983/diff/1/?file=1807498#file1807498line157> > > > > How are you handling InterruptedException here? Good point. I will set the interrupt again. - Vamsee ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61983/#review184173 ----------------------------------------------------------- On Aug. 30, 2017, 1:29 a.m., Vamsee Yarlagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61983/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2017, 1:29 a.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar > kalvagadda, and Na Li. > > > Repository: sentry > > > Description > ------- > > Sentry client logic supports retry of connections to server. But it should be > complemented with sufficient retry interval otherwise they fail fast without > trying for a decent time. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java > d400f8999d74cbc66c19a739a05d10a201635c5b > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java > 9fd401316f5184a80d737c463e03291bdebb2287 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java > 358d2824b7ce55a89945d321bf1a27fdbdd71e62 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java > 1724e7f9610e1e65cc24bd690d73530f6fb14049 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java > 45522df0d5803f525c29bfa9b06fde9e3c7e5939 > > > Diff: https://reviews.apache.org/r/61983/diff/1/ > > > Testing > ------- > > Sentry unit test run in progress. > > > Thanks, > > Vamsee Yarlagadda > >
