Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign 9e6d970ce -> 46c806912
SENTRY-1791: Sentry Clients failover not working with kerberos enabled (Kalyan Kalvagadda, reviewed by Alex Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/46c80691 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/46c80691 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/46c80691 Branch: refs/heads/sentry-ha-redesign Commit: 46c806912bfe37c4901555f5ae09c7c3defb63d3 Parents: 9e6d970 Author: Alexander Kolbasov <[email protected]> Authored: Wed Jun 14 00:51:17 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Wed Jun 14 00:51:17 2017 -0700 ---------------------------------------------------------------------- .../transport/RetryClientInvocationHandler.java | 27 ++++++++++---------- .../transport/SentryTransportFactory.java | 23 +++++++---------- .../core/common/transport/TransportFactory.java | 6 ++--- .../thrift/TestSentryServiceFailureCase.java | 3 +-- 4 files changed, 25 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/46c80691/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java index 62d0d2c..d400f89 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/RetryClientInvocationHandler.java @@ -131,29 +131,28 @@ public final class RetryClientInvocationHandler extends SentryClientInvocationHa * @throws Exception */ private void connect() throws Exception { - Exception lastExc = null; + Throwable lastExc = null; for (int retryCount = 0; retryCount < maxRetryCount; retryCount++) { try { + // If there is a TTransportException while connecting to sentry server, retry is + // attempted. For the rest, retry is not attempted. client.connect(); return; - } catch (TTransportException e) { + } catch (TTransportException failure) { // Retry when the exception is caused by connection problem. - LOGGER.error("failed to connect", e); - lastExc = e; - } catch (Exception e) { - Throwable causeException = e.getCause(); - if (causeException instanceof TTransportException) { - // Retry when the exception is caused by connection problem. - LOGGER.error("failed to connect", e); - lastExc = e; - continue; + LOGGER.error("Failed to connect", failure); + lastExc = failure; + } catch (Exception failure) { + Throwable causeException = failure.getCause(); + if (!(causeException instanceof TTransportException)) { + LOGGER.error("Failed to connect to Sentry Server", failure); + throw new SentryUserException(failure.getMessage(), causeException); } - // Some other failure, re-throw it - throw e; + lastExc = causeException; } } assert lastExc != null; - throw lastExc; + throw new SentryUserException (lastExc.getMessage(), lastExc); } @Override http://git-wip-us.apache.org/repos/asf/sentry/blob/46c80691/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java index d299113..391c97c 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryTransportFactory.java @@ -48,7 +48,6 @@ import java.security.PrivilegedExceptionAction; public final class SentryTransportFactory implements TransportFactory { private static final Logger LOGGER = LoggerFactory.getLogger(SentryTransportFactory.class); - private final Configuration conf; private final boolean useUgi; private final String serverPrincipal; private final int connectionTimeout; @@ -65,8 +64,7 @@ public final class SentryTransportFactory implements TransportFactory { public SentryTransportFactory(Configuration conf, SentryClientTransportConfigInterface transportConfig) { - this.conf = conf; - Preconditions.checkNotNull(this.conf, "Configuration object cannot be null"); + Preconditions.checkNotNull(conf, "Configuration object cannot be null"); connectionTimeout = transportConfig.getServerRpcConnTimeoutInMs(conf); isKerberosEnabled = transportConfig.isKerberosEnabled(conf); if (isKerberosEnabled) { @@ -84,10 +82,10 @@ public final class SentryTransportFactory implements TransportFactory { * @throws IOException */ @Override - public TTransportWrapper getTransport(HostAndPort endpoint) throws IOException { + public TTransportWrapper getTransport(HostAndPort endpoint) throws Exception { return new TTransportWrapper(connectToServer(new InetSocketAddress(endpoint.getHostText(), - endpoint.getPort())), - endpoint); + endpoint.getPort())), + endpoint); } /** @@ -96,14 +94,11 @@ public final class SentryTransportFactory implements TransportFactory { * @param serverAddress Address client needs to connect * @throws Exception if there is failure in establishing the connection. */ - private TTransport connectToServer(InetSocketAddress serverAddress) throws IOException { - try { - TTransport thriftTransport = createTransport(serverAddress); - thriftTransport.open(); - return thriftTransport; - } catch (TTransportException e) { - throw new IOException("Failed to open transport: " + e.getMessage(), e); - } + private TTransport connectToServer(InetSocketAddress serverAddress) throws Exception { + TTransport thriftTransport = createTransport(serverAddress); + thriftTransport.open(); + LOGGER.debug("Successfully opened transport {} to {}", thriftTransport, serverAddress); + return thriftTransport; } /** http://git-wip-us.apache.org/repos/asf/sentry/blob/46c80691/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java index e115cbb..bc0ccae 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/TransportFactory.java @@ -20,8 +20,6 @@ package org.apache.sentry.core.common.transport; import com.google.common.net.HostAndPort; -import java.io.IOException; - /** * Generic transport factory interface. * <p> @@ -33,7 +31,7 @@ public interface TransportFactory { /** * Connect to the endpoint and return a connected Thrift transport. * @return Connection to the endpoint - * @throws IOException + * @throws Exception if can't establish connection */ - TTransportWrapper getTransport(HostAndPort endpoint) throws IOException; + TTransportWrapper getTransport(HostAndPort endpoint) throws Exception; } http://git-wip-us.apache.org/repos/asf/sentry/blob/46c80691/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java index 7c7ebab..2f4e8f6 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java @@ -67,8 +67,7 @@ public class TestSentryServiceFailureCase extends SentryServiceIntegrationBase { if (cause == null) { throw e; } - String msg = "Exception message: " + cause.getMessage() + " to contain " + - PEER_CALLBACK_FAILURE; + String msg = "Exception message: " + cause.getMessage(); Assert.assertTrue(msg, Strings.nullToEmpty(cause.getMessage()) .contains(PEER_CALLBACK_FAILURE)); }
