SENTRY-1709: Avoid randomizing the servers at client side based on configuration. (Kalyan kalvagadda), reviewed by: Alex Kolbasov)
Change-Id: I1d5828197b7cc973890ad257f10b918f6342854e Reviewed-on: http://gerrit.sjc.cloudera.com:8080/22342 Reviewed-by: Kalyan Kumar Kalvagadda <[email protected]> Tested-by: Kalyan Kumar Kalvagadda <[email protected]> Tested-by: Jenkins User Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/c9a703c6 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/c9a703c6 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/c9a703c6 Branch: refs/for/cdh5-1.5.1_ha Commit: c9a703c617f678e094522d5dc7632dc70941367d Parents: ddfc9c8 Author: Alexander Kolbasov <[email protected]> Authored: Thu May 4 21:38:01 2017 -0700 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Fri May 5 11:24:06 2017 -0700 ---------------------------------------------------------------------- .../SentryClientTransportConfigInterface.java | 9 +++++++++ .../transport/SentryClientTransportConstants.java | 8 ++++++++ .../SentryHDFSClientTransportConfig.java | 6 ++++++ .../SentryPolicyClientTransportConfig.java | 6 ++++++ .../common/transport/SentryTransportFactory.java | 18 +++++++++--------- 5 files changed, 38 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/c9a703c6/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java index 24192fd..9ea7185 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConfigInterface.java @@ -100,4 +100,13 @@ interface SentryClientTransportConfigInterface { * configuration. */ int getServerRpcConnTimeoutInMs(Configuration conf) throws MissingConfigurationException; + + /** + * + * @param conf configuration + * @return True if the client should load balance connections between multiple servers + * @throws MissingConfigurationException if property is mandatory and is missing in + * configuration. + */ + boolean isLoadBalancingEnabled(Configuration conf)throws MissingConfigurationException; } http://git-wip-us.apache.org/repos/asf/sentry/blob/c9a703c6/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java index 3520787..4b353cf 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryClientTransportConstants.java @@ -102,6 +102,10 @@ class SentryClientTransportConstants { static final String SENTRY_POOL_MIN_IDLE = "sentry.service.client.connection.pool.min-idle"; static final int SENTRY_POOL_MIN_IDLE_DEFAULT = 0; + // configuration to load balance the connections to the configured sentry servers + static final String SENTRY_CLIENT_LOAD_BALANCING =" sentry.service.client.connection.loadbalance"; + static final boolean SENTRY_CLIENT_LOAD_BALANCING_DEFAULT = true; + // retry num for getting the connection from connection pool static final String SENTRY_POOL_RETRY_TOTAL = SentryClientTransportConstants.SENTRY_FULL_RETRY_TOTAL; @@ -162,5 +166,9 @@ class SentryClientTransportConstants { SentryClientTransportConstants.SENTRY_RPC_RETRY_TOTAL; static final int SENTRY_RPC_RETRY_TOTAL_DEFAULT = 3; + + // configuration to load balance the connections to the configured sentry servers + static final String SENTRY_CLIENT_LOAD_BALANCING =" sentry.hdfs.service.client.connection.loadbalance"; + static final boolean SENTRY_CLIENT_LOAD_BALANCING_DEFAULT = true; } } http://git-wip-us.apache.org/repos/asf/sentry/blob/c9a703c6/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java index 74f790b..4608238 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryHDFSClientTransportConfig.java @@ -85,4 +85,10 @@ public final class SentryHDFSClientTransportConfig throws MissingConfigurationException { return conf.getInt(SERVER_RPC_CONN_TIMEOUT, SERVER_RPC_CONN_TIMEOUT_DEFAULT); } + + @Override + public boolean isLoadBalancingEnabled(Configuration conf) + throws MissingConfigurationException { + return conf.getBoolean(SENTRY_CLIENT_LOAD_BALANCING, SENTRY_CLIENT_LOAD_BALANCING_DEFAULT); + } } http://git-wip-us.apache.org/repos/asf/sentry/blob/c9a703c6/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java index 37fd0b3..f07fb16 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/transport/SentryPolicyClientTransportConfig.java @@ -85,4 +85,10 @@ public final class SentryPolicyClientTransportConfig throws MissingConfigurationException { return conf.getInt(SERVER_RPC_CONN_TIMEOUT, SERVER_RPC_CONN_TIMEOUT_DEFAULT); } + + @Override + public boolean isLoadBalancingEnabled(Configuration conf) + throws MissingConfigurationException { + return conf.getBoolean(SENTRY_CLIENT_LOAD_BALANCING, SENTRY_CLIENT_LOAD_BALANCING_DEFAULT); + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/sentry/blob/c9a703c6/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 9ddb400..9b9f9e8 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 @@ -129,27 +129,28 @@ public class SentryTransportFactory { this.transportConfig = transportConfig; try { - String hostsAndPortsStr; this.connectionTimeout = transportConfig.getServerRpcConnTimeoutInMs(conf); this.connectionFullRetryTotal = transportConfig.getSentryFullRetryTotal(conf); - hostsAndPortsStr = transportConfig.getSentryServerRpcAddress(conf); + String hostsAndPortsStr = transportConfig.getSentryServerRpcAddress(conf); int serverPort = transportConfig.getServerRpcPort(conf); String[] hostsAndPortsStrArr = hostsAndPortsStr.split(","); HostAndPort[] hostsAndPorts = ThriftUtil.parseHostPortStrings(hostsAndPortsStrArr, serverPort); - this.endpoints = new ArrayList(hostsAndPortsStrArr.length); + this.endpoints = new ArrayList<>(hostsAndPortsStrArr.length); for (HostAndPort endpoint : hostsAndPorts) { this.endpoints.add( new InetSocketAddress(endpoint.getHostText(), endpoint.getPort())); LOGGER.debug("Added server endpoint: " + endpoint.toString()); } - // Reorder endpoints randomly to prevent all clients connecting to the same endpoint - // at the same time after a node failure. - Collections.shuffle(endpoints); + if((endpoints.size() > 1) && (transportConfig.isLoadBalancingEnabled(conf))) { + // Reorder endpoints randomly to prevent all clients connecting to the same endpoint + // and load balance the connections towards sentry servers + Collections.shuffle(endpoints); + } } catch (MissingConfigurationException e) { throw new RuntimeException("Sentry Thrift Client Creation Failed: " + e.getMessage(), e); } @@ -172,7 +173,7 @@ public class SentryTransportFactory { this.transportConfig = transportConfig; try { - this.endpoints = new ArrayList(1); + this.endpoints = new ArrayList<>(1); this.endpoints.add(NetUtils.createSocketAddr(addr, port)); this.connectionTimeout = transportConfig.getServerRpcConnTimeoutInMs(conf); this.connectionFullRetryTotal = transportConfig.getSentryFullRetryTotal(conf); @@ -199,8 +200,7 @@ public class SentryTransportFactory { } catch (IOException e) { currentException = e; LOGGER.error( - String.format("Failed to connect to all the configured sentry servers, " + - "Retrying again")); + "Failed to connect to all the configured sentry servers, Retrying again"); } } // Throws exception on reaching the connectionFullRetryTotal.
