From 6d2404998a09b9ea0c80a32c3c2329f8bd825c7f Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 14 Aug 2018 12:53:56 -0600 Subject: [PATCH 1/3] Work on removing conneciton profiles from transport --- .../transport/netty4/Netty4Transport.java | 9 +-- .../common/settings/ClusterSettings.java | 12 ++-- .../transport/ConnectionProfile.java | 26 ++++++- .../transport/RemoteClusterConnection.java | 4 +- .../elasticsearch/transport/TcpTransport.java | 72 +++---------------- .../transport/TransportService.java | 41 ++++++++++- .../discovery/AbstractDisruptionTestCase.java | 3 +- .../transport/TcpTransportTests.java | 14 ++-- .../test/InternalTestCluster.java | 6 +- .../test/transport/MockTransportService.java | 2 +- .../transport/MockTcpTransport.java | 9 ++- .../transport/nio/MockNioTransport.java | 13 ++-- 12 files changed, 102 insertions(+), 109 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index 90138bc59906e..ad6eb74358c2c 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -147,7 +147,6 @@ private Bootstrap createBootstrap() { bootstrap.handler(getClientChannelInitializer()); - bootstrap.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, Math.toIntExact(defaultConnectionProfile.getConnectTimeout().millis())); bootstrap.option(ChannelOption.TCP_NODELAY, TCP_NO_DELAY.get(settings)); bootstrap.option(ChannelOption.SO_KEEPALIVE, TCP_KEEP_ALIVE.get(settings)); @@ -175,14 +174,8 @@ private void createServerBootstrap(ProfileSettings profileSettings) { String name = profileSettings.profileName; if (logger.isDebugEnabled()) { logger.debug("using profile[{}], worker_count[{}], port[{}], bind_host[{}], publish_host[{}], compress[{}], " - + "connect_timeout[{}], connections_per_node[{}/{}/{}/{}/{}], receive_predictor[{}->{}]", + + "receive_predictor[{}->{}]", name, workerCount, profileSettings.portOrRange, profileSettings.bindHosts, profileSettings.publishHosts, compress, - defaultConnectionProfile.getConnectTimeout(), - defaultConnectionProfile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY), - defaultConnectionProfile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK), - defaultConnectionProfile.getNumConnectionsPerType(TransportRequestOptions.Type.REG), - defaultConnectionProfile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE), - defaultConnectionProfile.getNumConnectionsPerType(TransportRequestOptions.Type.PING), receivePredictorMin, receivePredictorMax); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 03a0f3b42a10a..de8691b3b6871 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -297,13 +297,13 @@ public void apply(Settings value, Settings current, Settings previous) { TcpTransport.TCP_REUSE_ADDRESS_PROFILE, TcpTransport.TCP_SEND_BUFFER_SIZE_PROFILE, TcpTransport.TCP_RECEIVE_BUFFER_SIZE_PROFILE, - TcpTransport.CONNECTIONS_PER_NODE_RECOVERY, - TcpTransport.CONNECTIONS_PER_NODE_BULK, - TcpTransport.CONNECTIONS_PER_NODE_REG, - TcpTransport.CONNECTIONS_PER_NODE_STATE, - TcpTransport.CONNECTIONS_PER_NODE_PING, + TransportService.CONNECTIONS_PER_NODE_RECOVERY, + TransportService.CONNECTIONS_PER_NODE_BULK, + TransportService.CONNECTIONS_PER_NODE_REG, + TransportService.CONNECTIONS_PER_NODE_STATE, + TransportService.CONNECTIONS_PER_NODE_PING, + TransportService.TCP_CONNECT_TIMEOUT, TcpTransport.PING_SCHEDULE, - TcpTransport.TCP_CONNECT_TIMEOUT, NetworkService.NETWORK_SERVER, TcpTransport.TCP_NO_DELAY, TcpTransport.TCP_KEEP_ALIVE, diff --git a/server/src/main/java/org/elasticsearch/transport/ConnectionProfile.java b/server/src/main/java/org/elasticsearch/transport/ConnectionProfile.java index e14f684bf72ef..b9ed42ca00a56 100644 --- a/server/src/main/java/org/elasticsearch/transport/ConnectionProfile.java +++ b/server/src/main/java/org/elasticsearch/transport/ConnectionProfile.java @@ -26,6 +26,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -61,14 +62,35 @@ public static ConnectionProfile buildSingleChannelProfile(TransportRequestOption private final TimeValue connectTimeout; private final TimeValue handshakeTimeout; - private ConnectionProfile(List handles, int numConnections, TimeValue connectTimeout, TimeValue handshakeTimeout) - { + private ConnectionProfile(List handles, int numConnections, TimeValue connectTimeout, + TimeValue handshakeTimeout) { this.handles = handles; this.numConnections = numConnections; this.connectTimeout = connectTimeout; this.handshakeTimeout = handshakeTimeout; } + /** + * takes a {@link ConnectionProfile} resolves it to a fully specified (i.e., no nulls) profile + */ + public static ConnectionProfile resolveConnectionProfile(@Nullable ConnectionProfile profile, ConnectionProfile fallbackProfile) { + Objects.requireNonNull(fallbackProfile); + if (profile == null) { + return fallbackProfile; + } else if (profile.getConnectTimeout() != null && profile.getHandshakeTimeout() != null) { + return profile; + } else { + ConnectionProfile.Builder builder = new ConnectionProfile.Builder(profile); + if (profile.getConnectTimeout() == null) { + builder.setConnectTimeout(fallbackProfile.getConnectTimeout()); + } + if (profile.getHandshakeTimeout() == null) { + builder.setHandshakeTimeout(fallbackProfile.getHandshakeTimeout()); + } + return builder.build(); + } + } + /** * A builder to build a new {@link ConnectionProfile} */ diff --git a/server/src/main/java/org/elasticsearch/transport/RemoteClusterConnection.java b/server/src/main/java/org/elasticsearch/transport/RemoteClusterConnection.java index 355a9c655c998..67c0e1a5aa64a 100644 --- a/server/src/main/java/org/elasticsearch/transport/RemoteClusterConnection.java +++ b/server/src/main/java/org/elasticsearch/transport/RemoteClusterConnection.java @@ -108,8 +108,8 @@ final class RemoteClusterConnection extends AbstractComponent implements Transpo this.nodePredicate = nodePredicate; this.clusterAlias = clusterAlias; ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); - builder.setConnectTimeout(TcpTransport.TCP_CONNECT_TIMEOUT.get(settings)); - builder.setHandshakeTimeout(TcpTransport.TCP_CONNECT_TIMEOUT.get(settings)); + builder.setConnectTimeout(TransportService.TCP_CONNECT_TIMEOUT.get(settings)); + builder.setHandshakeTimeout(TransportService.TCP_CONNECT_TIMEOUT.get(settings)); builder.addConnections(6, TransportRequestOptions.Type.REG, TransportRequestOptions.Type.PING); // TODO make this configurable? builder.addConnections(0, // we don't want this to be used for anything else but search TransportRequestOptions.Type.BULK, diff --git a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java index 064c76cc03fce..0b82417cfaa04 100644 --- a/server/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/server/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -29,7 +29,6 @@ import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.Booleans; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.BytesArray; @@ -135,18 +134,6 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements // the scheduled internal ping interval setting, defaults to disabled (-1) public static final Setting PING_SCHEDULE = timeSetting("transport.ping_schedule", TimeValue.timeValueSeconds(-1), Setting.Property.NodeScope); - public static final Setting CONNECTIONS_PER_NODE_RECOVERY = - intSetting("transport.connections_per_node.recovery", 2, 1, Setting.Property.NodeScope); - public static final Setting CONNECTIONS_PER_NODE_BULK = - intSetting("transport.connections_per_node.bulk", 3, 1, Setting.Property.NodeScope); - public static final Setting CONNECTIONS_PER_NODE_REG = - intSetting("transport.connections_per_node.reg", 6, 1, Setting.Property.NodeScope); - public static final Setting CONNECTIONS_PER_NODE_STATE = - intSetting("transport.connections_per_node.state", 1, 1, Setting.Property.NodeScope); - public static final Setting CONNECTIONS_PER_NODE_PING = - intSetting("transport.connections_per_node.ping", 1, 1, Setting.Property.NodeScope); - public static final Setting TCP_CONNECT_TIMEOUT = - timeSetting("transport.tcp.connect_timeout", NetworkService.TCP_CONNECT_TIMEOUT, Setting.Property.NodeScope); public static final Setting TCP_NO_DELAY = boolSetting("transport.tcp_no_delay", NetworkService.TCP_NO_DELAY, Setting.Property.NodeScope); public static final Setting TCP_KEEP_ALIVE = @@ -154,11 +141,9 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements public static final Setting TCP_REUSE_ADDRESS = boolSetting("transport.tcp.reuse_address", NetworkService.TCP_REUSE_ADDRESS, Setting.Property.NodeScope); public static final Setting TCP_SEND_BUFFER_SIZE = - Setting.byteSizeSetting("transport.tcp.send_buffer_size", NetworkService.TCP_SEND_BUFFER_SIZE, - Setting.Property.NodeScope); + Setting.byteSizeSetting("transport.tcp.send_buffer_size", NetworkService.TCP_SEND_BUFFER_SIZE, Setting.Property.NodeScope); public static final Setting TCP_RECEIVE_BUFFER_SIZE = - Setting.byteSizeSetting("transport.tcp.receive_buffer_size", NetworkService.TCP_RECEIVE_BUFFER_SIZE, - Setting.Property.NodeScope); + Setting.byteSizeSetting("transport.tcp.receive_buffer_size", NetworkService.TCP_RECEIVE_BUFFER_SIZE, Setting.Property.NodeScope); public static final Setting.AffixSetting TCP_NO_DELAY_PROFILE = affixKeySetting("transport.profiles.", "tcp_no_delay", @@ -213,7 +198,6 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements protected final boolean compress; private volatile BoundTransportAddress boundAddress; private final String transportName; - protected final ConnectionProfile defaultConnectionProfile; private final ConcurrentMap pendingHandshakes = new ConcurrentHashMap<>(); private final CounterMetric numHandshakes = new CounterMetric(); @@ -237,7 +221,6 @@ public TcpTransport(String transportName, Settings settings, ThreadPool threadPo this.compress = Transport.TRANSPORT_TCP_COMPRESS.get(settings); this.networkService = networkService; this.transportName = transportName; - defaultConnectionProfile = buildDefaultConnectionProfile(settings); final Settings defaultFeatures = DEFAULT_FEATURES_SETTING.get(settings); if (defaultFeatures == null) { this.features = new String[0]; @@ -261,25 +244,6 @@ public TcpTransport(String transportName, Settings settings, ThreadPool threadPo } } - static ConnectionProfile buildDefaultConnectionProfile(Settings settings) { - int connectionsPerNodeRecovery = CONNECTIONS_PER_NODE_RECOVERY.get(settings); - int connectionsPerNodeBulk = CONNECTIONS_PER_NODE_BULK.get(settings); - int connectionsPerNodeReg = CONNECTIONS_PER_NODE_REG.get(settings); - int connectionsPerNodeState = CONNECTIONS_PER_NODE_STATE.get(settings); - int connectionsPerNodePing = CONNECTIONS_PER_NODE_PING.get(settings); - ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); - builder.setConnectTimeout(TCP_CONNECT_TIMEOUT.get(settings)); - builder.setHandshakeTimeout(TCP_CONNECT_TIMEOUT.get(settings)); - builder.addConnections(connectionsPerNodeBulk, TransportRequestOptions.Type.BULK); - builder.addConnections(connectionsPerNodePing, TransportRequestOptions.Type.PING); - // if we are not master eligible we don't need a dedicated channel to publish the state - builder.addConnections(DiscoveryNode.isMasterNode(settings) ? connectionsPerNodeState : 0, TransportRequestOptions.Type.STATE); - // if we are not a data-node we don't need any dedicated channels for recovery - builder.addConnections(DiscoveryNode.isDataNode(settings) ? connectionsPerNodeRecovery : 0, TransportRequestOptions.Type.RECOVERY); - builder.addConnections(connectionsPerNodeReg, TransportRequestOptions.Type.REG); - return builder.build(); - } - @Override protected void doStart() { } @@ -456,41 +420,21 @@ public void sendRequest(long requestId, String action, TransportRequest request, } } - /** - * takes a {@link ConnectionProfile} that have been passed as a parameter to the public methods - * and resolves it to a fully specified (i.e., no nulls) profile - */ - protected static ConnectionProfile resolveConnectionProfile(@Nullable ConnectionProfile connectionProfile, - ConnectionProfile defaultConnectionProfile) { - Objects.requireNonNull(defaultConnectionProfile); - if (connectionProfile == null) { - return defaultConnectionProfile; - } else if (connectionProfile.getConnectTimeout() != null && connectionProfile.getHandshakeTimeout() != null) { - return connectionProfile; - } else { - ConnectionProfile.Builder builder = new ConnectionProfile.Builder(connectionProfile); - if (connectionProfile.getConnectTimeout() == null) { - builder.setConnectTimeout(defaultConnectionProfile.getConnectTimeout()); - } - if (connectionProfile.getHandshakeTimeout() == null) { - builder.setHandshakeTimeout(defaultConnectionProfile.getHandshakeTimeout()); - } - return builder.build(); - } - } - - protected ConnectionProfile resolveConnectionProfile(ConnectionProfile connectionProfile) { - return resolveConnectionProfile(connectionProfile, defaultConnectionProfile); + // This allows transport implementations to potentially override specific connection profiles. This + // primarily exists for the test implementations. + protected ConnectionProfile maybeOverrideConnectionProfile(ConnectionProfile connectionProfile) { + return connectionProfile; } @Override public NodeChannels openConnection(DiscoveryNode node, ConnectionProfile connectionProfile) { + Objects.requireNonNull(connectionProfile, "connection profile cannot be null"); if (node == null) { throw new ConnectTransportException(null, "can't open connection to a null node"); } boolean success = false; NodeChannels nodeChannels = null; - connectionProfile = resolveConnectionProfile(connectionProfile); + connectionProfile = maybeOverrideConnectionProfile(connectionProfile); closeLock.readLock().lock(); // ensure we don't open connections while we are closing try { ensureOpen(); diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index d7ece36d7fdda..de3d4a3e711e4 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -36,6 +36,7 @@ import org.elasticsearch.common.io.stream.Streamable; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.network.NetworkService; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; @@ -43,6 +44,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.BoundTransportAddress; import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.concurrent.AbstractRunnable; import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.common.util.concurrent.FutureUtils; @@ -71,10 +73,24 @@ import java.util.function.Supplier; import static java.util.Collections.emptyList; +import static org.elasticsearch.common.settings.Setting.intSetting; import static org.elasticsearch.common.settings.Setting.listSetting; +import static org.elasticsearch.common.settings.Setting.timeSetting; public class TransportService extends AbstractLifecycleComponent implements TransportConnectionListener { + public static final Setting CONNECTIONS_PER_NODE_RECOVERY = + intSetting("transport.connections_per_node.recovery", 2, 1, Setting.Property.NodeScope); + public static final Setting CONNECTIONS_PER_NODE_BULK = + intSetting("transport.connections_per_node.bulk", 3, 1, Setting.Property.NodeScope); + public static final Setting CONNECTIONS_PER_NODE_REG = + intSetting("transport.connections_per_node.reg", 6, 1, Setting.Property.NodeScope); + public static final Setting CONNECTIONS_PER_NODE_STATE = + intSetting("transport.connections_per_node.state", 1, 1, Setting.Property.NodeScope); + public static final Setting CONNECTIONS_PER_NODE_PING = + intSetting("transport.connections_per_node.ping", 1, 1, Setting.Property.NodeScope); + public static final Setting TCP_CONNECT_TIMEOUT = + timeSetting("transport.tcp.connect_timeout", NetworkService.TCP_CONNECT_TIMEOUT, Setting.Property.NodeScope); public static final String DIRECT_RESPONSE_PROFILE = ".direct"; public static final String HANDSHAKE_ACTION_NAME = "internal:transport/handshake"; @@ -182,7 +198,7 @@ public TransportService(Settings settings, Transport transport, ThreadPool threa this.connectToRemoteCluster = RemoteClusterService.ENABLE_REMOTE_CLUSTERS.get(settings); remoteClusterService = new RemoteClusterService(settings, this); responseHandlers = transport.getResponseHandlers(); - defaultConnectionProfile = TcpTransport.buildDefaultConnectionProfile(settings); + defaultConnectionProfile = buildDefaultConnectionProfile(settings); if (clusterSettings != null) { clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_INCLUDE_SETTING, this::setTracerLogInclude); clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracerLogExclude); @@ -350,7 +366,7 @@ public void connectToNode(final DiscoveryNode node, ConnectionProfile connection return; } - ConnectionProfile resolvedProfile = TcpTransport.resolveConnectionProfile(connectionProfile, defaultConnectionProfile); + ConnectionProfile resolvedProfile = ConnectionProfile.resolveConnectionProfile(connectionProfile, defaultConnectionProfile); connectionManager.connectToNode(node, resolvedProfile, (newConnection, actualProfile) -> { // We don't validate cluster names to allow for CCS connections. final DiscoveryNode remote = handshake(newConnection, actualProfile.getHandshakeTimeout().millis(), cn -> true).discoveryNode; @@ -370,7 +386,7 @@ public Transport.Connection openConnection(final DiscoveryNode node, ConnectionP if (isLocalNode(node)) { return localNodeConnection; } else { - return transport.openConnection(node, profile); + return transport.openConnection(node, ConnectionProfile.resolveConnectionProfile(profile, defaultConnectionProfile)); } } @@ -437,6 +453,25 @@ public ConnectionManager getConnectionManager() { return connectionManager; } + static ConnectionProfile buildDefaultConnectionProfile(Settings settings) { + int connectionsPerNodeRecovery = CONNECTIONS_PER_NODE_RECOVERY.get(settings); + int connectionsPerNodeBulk = CONNECTIONS_PER_NODE_BULK.get(settings); + int connectionsPerNodeReg = CONNECTIONS_PER_NODE_REG.get(settings); + int connectionsPerNodeState = CONNECTIONS_PER_NODE_STATE.get(settings); + int connectionsPerNodePing = CONNECTIONS_PER_NODE_PING.get(settings); + ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); + builder.setConnectTimeout(TCP_CONNECT_TIMEOUT.get(settings)); + builder.setHandshakeTimeout(TCP_CONNECT_TIMEOUT.get(settings)); + builder.addConnections(connectionsPerNodeBulk, TransportRequestOptions.Type.BULK); + builder.addConnections(connectionsPerNodePing, TransportRequestOptions.Type.PING); + // if we are not master eligible we don't need a dedicated channel to publish the state + builder.addConnections(DiscoveryNode.isMasterNode(settings) ? connectionsPerNodeState : 0, TransportRequestOptions.Type.STATE); + // if we are not a data-node we don't need any dedicated channels for recovery + builder.addConnections(DiscoveryNode.isDataNode(settings) ? connectionsPerNodeRecovery : 0, TransportRequestOptions.Type.RECOVERY); + builder.addConnections(connectionsPerNodeReg, TransportRequestOptions.Type.REG); + return builder.build(); + } + static class HandshakeRequest extends TransportRequest { public static final HandshakeRequest INSTANCE = new HandshakeRequest(); diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index 50dfd92d82e15..c3d325ae77033 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -45,6 +45,7 @@ import org.elasticsearch.test.disruption.SlowClusterStateProcessing; import org.elasticsearch.test.transport.MockTransportService; import org.elasticsearch.transport.TcpTransport; +import org.elasticsearch.transport.TransportService; import org.junit.Before; import java.util.Arrays; @@ -139,7 +140,7 @@ List startCluster(int numberOfNodes, int minimumMasterNode, @Nullable in .put(FaultDetection.PING_RETRIES_SETTING.getKey(), "1") // for hitting simulated network failures quickly .put("discovery.zen.join_timeout", "10s") // still long to induce failures but to long so test won't time out .put(DiscoverySettings.PUBLISH_TIMEOUT_SETTING.getKey(), "1s") // <-- for hitting simulated network failures quickly - .put(TcpTransport.TCP_CONNECT_TIMEOUT.getKey(), "10s") // Network delay disruption waits for the min between this + .put(TransportService.TCP_CONNECT_TIMEOUT.getKey(), "10s") // Network delay disruption waits for the min between this // value and the time of disruption and does not recover immediately // when disruption is stop. We should make sure we recover faster // then the default of 30s, causing ensureGreen and friends to time out diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index 8474b9947129e..215d3edc16fcc 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -306,8 +306,8 @@ public void writeTo(StreamOutput out) throws IOException { } public void testConnectionProfileResolve() { - final ConnectionProfile defaultProfile = TcpTransport.buildDefaultConnectionProfile(Settings.EMPTY); - assertEquals(defaultProfile, TcpTransport.resolveConnectionProfile(null, defaultProfile)); + final ConnectionProfile defaultProfile = TransportService.buildDefaultConnectionProfile(Settings.EMPTY); + assertEquals(defaultProfile, ConnectionProfile.resolveConnectionProfile(null, defaultProfile)); final ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.BULK); @@ -326,7 +326,7 @@ public void testConnectionProfileResolve() { } final ConnectionProfile profile = builder.build(); - final ConnectionProfile resolved = TcpTransport.resolveConnectionProfile(profile, defaultProfile); + final ConnectionProfile resolved = ConnectionProfile.resolveConnectionProfile(profile, defaultProfile); assertNotEquals(resolved, defaultProfile); assertThat(resolved.getNumConnections(), equalTo(profile.getNumConnections())); assertThat(resolved.getHandles(), equalTo(profile.getHandles())); @@ -338,7 +338,7 @@ public void testConnectionProfileResolve() { } public void testDefaultConnectionProfile() { - ConnectionProfile profile = TcpTransport.buildDefaultConnectionProfile(Settings.EMPTY); + ConnectionProfile profile = TransportService.buildDefaultConnectionProfile(Settings.EMPTY); assertEquals(13, profile.getNumConnections()); assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); @@ -346,7 +346,7 @@ public void testDefaultConnectionProfile() { assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - profile = TcpTransport.buildDefaultConnectionProfile(Settings.builder().put("node.master", false).build()); + profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.master", false).build()); assertEquals(12, profile.getNumConnections()); assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); @@ -354,7 +354,7 @@ public void testDefaultConnectionProfile() { assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - profile = TcpTransport.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).build()); + profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).build()); assertEquals(11, profile.getNumConnections()); assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); @@ -362,7 +362,7 @@ public void testDefaultConnectionProfile() { assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - profile = TcpTransport.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).put("node.master", false).build()); + profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).put("node.master", false).build()); assertEquals(10, profile.getNumConnections()); assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 5b4f528913f18..306f79e5e16ec 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -385,9 +385,9 @@ private Settings getRandomNodeSettings(long seed) { // randomize tcp settings if (random.nextBoolean()) { - builder.put(TcpTransport.CONNECTIONS_PER_NODE_RECOVERY.getKey(), random.nextInt(2) + 1); - builder.put(TcpTransport.CONNECTIONS_PER_NODE_BULK.getKey(), random.nextInt(3) + 1); - builder.put(TcpTransport.CONNECTIONS_PER_NODE_REG.getKey(), random.nextInt(6) + 1); + builder.put(TransportService.CONNECTIONS_PER_NODE_RECOVERY.getKey(), random.nextInt(2) + 1); + builder.put(TransportService.CONNECTIONS_PER_NODE_BULK.getKey(), random.nextInt(3) + 1); + builder.put(TransportService.CONNECTIONS_PER_NODE_REG.getKey(), random.nextInt(6) + 1); } if (random.nextBoolean()) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java index 40a6ad6476d4a..15ab06d651e92 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/MockTransportService.java @@ -309,7 +309,7 @@ public void addUnresponsiveRule(TransportAddress transportAddress, final TimeVal } // TODO: Replace with proper setting - TimeValue connectingTimeout = TcpTransport.TCP_CONNECT_TIMEOUT.getDefault(Settings.EMPTY); + TimeValue connectingTimeout = TransportService.TCP_CONNECT_TIMEOUT.getDefault(Settings.EMPTY); try { if (delay.millis() < connectingTimeout.millis()) { Thread.sleep(delay.millis()); diff --git a/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java b/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java index bbff340c86011..25579469fe2f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/MockTcpTransport.java @@ -189,12 +189,11 @@ protected MockChannel initiateChannel(InetSocketAddress address, ActionListener< } @Override - protected ConnectionProfile resolveConnectionProfile(ConnectionProfile connectionProfile) { - ConnectionProfile connectionProfile1 = resolveConnectionProfile(connectionProfile, defaultConnectionProfile); + protected ConnectionProfile maybeOverrideConnectionProfile(ConnectionProfile connectionProfile) { ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); Set allTypesWithConnection = new HashSet<>(); Set allTypesWithoutConnection = new HashSet<>(); - for (ConnectionProfile.ConnectionTypeHandle handle : connectionProfile1.getHandles()) { + for (ConnectionProfile.ConnectionTypeHandle handle : connectionProfile.getHandles()) { Set types = handle.getTypes(); if (handle.length > 0) { allTypesWithConnection.addAll(types); @@ -207,8 +206,8 @@ protected ConnectionProfile resolveConnectionProfile(ConnectionProfile connectio if (allTypesWithoutConnection.isEmpty() == false) { builder.addConnections(0, allTypesWithoutConnection.toArray(new TransportRequestOptions.Type[0])); } - builder.setHandshakeTimeout(connectionProfile1.getHandshakeTimeout()); - builder.setConnectTimeout(connectionProfile1.getConnectTimeout()); + builder.setHandshakeTimeout(connectionProfile.getHandshakeTimeout()); + builder.setConnectTimeout(connectionProfile.getConnectTimeout()); return builder.build(); } diff --git a/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java b/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java index dc5305d951bb0..fbe61db6ee721 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java @@ -133,16 +133,15 @@ protected void stopInternal() { } @Override - protected ConnectionProfile resolveConnectionProfile(ConnectionProfile connectionProfile) { - ConnectionProfile resolvedProfile = resolveConnectionProfile(connectionProfile, defaultConnectionProfile); - if (resolvedProfile.getNumConnections() <= 3) { - return resolvedProfile; + protected ConnectionProfile maybeOverrideConnectionProfile(ConnectionProfile connectionProfile) { + if (connectionProfile.getNumConnections() <= 3) { + return connectionProfile; } ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); Set allTypesWithConnection = new HashSet<>(); Set allTypesWithoutConnection = new HashSet<>(); for (TransportRequestOptions.Type type : TransportRequestOptions.Type.values()) { - int numConnections = resolvedProfile.getNumConnectionsPerType(type); + int numConnections = connectionProfile.getNumConnectionsPerType(type); if (numConnections > 0) { allTypesWithConnection.add(type); } else { @@ -155,8 +154,8 @@ protected ConnectionProfile resolveConnectionProfile(ConnectionProfile connectio if (allTypesWithoutConnection.isEmpty() == false) { builder.addConnections(0, allTypesWithoutConnection.toArray(new TransportRequestOptions.Type[0])); } - builder.setHandshakeTimeout(resolvedProfile.getHandshakeTimeout()); - builder.setConnectTimeout(resolvedProfile.getConnectTimeout()); + builder.setHandshakeTimeout(connectionProfile.getHandshakeTimeout()); + builder.setConnectTimeout(connectionProfile.getConnectTimeout()); return builder.build(); } From a387029ba6b17e15328c2763255194328270edba Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 14 Aug 2018 13:17:38 -0600 Subject: [PATCH 2/3] Move profiles into connection manager --- .../transport/netty4/Netty4Transport.java | 1 - .../transport/ConnectionManager.java | 34 +++++++++- .../transport/TransportService.java | 30 ++------ .../discovery/AbstractDisruptionTestCase.java | 1 - .../transport/ConnectionManagerTests.java | 68 +++++++++++++++++++ .../transport/TcpTransportTests.java | 67 ------------------ .../transport/StubbableConnectionManager.java | 5 ++ 7 files changed, 109 insertions(+), 97 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java index ad6eb74358c2c..e310f3012a9fb 100644 --- a/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Transport.java @@ -54,7 +54,6 @@ import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TcpTransport; -import org.elasticsearch.transport.TransportRequestOptions; import java.io.IOException; import java.net.InetSocketAddress; diff --git a/server/src/main/java/org/elasticsearch/transport/ConnectionManager.java b/server/src/main/java/org/elasticsearch/transport/ConnectionManager.java index f2b89ff59775c..1ff8b701a83e1 100644 --- a/server/src/main/java/org/elasticsearch/transport/ConnectionManager.java +++ b/server/src/main/java/org/elasticsearch/transport/ConnectionManager.java @@ -60,15 +60,21 @@ public class ConnectionManager implements Closeable { private final Transport transport; private final ThreadPool threadPool; private final TimeValue pingSchedule; + private final ConnectionProfile defaultProfile; private final Lifecycle lifecycle = new Lifecycle(); private final ReadWriteLock closeLock = new ReentrantReadWriteLock(); private final DelegatingNodeConnectionListener connectionListener = new DelegatingNodeConnectionListener(); public ConnectionManager(Settings settings, Transport transport, ThreadPool threadPool) { + this(settings, transport, threadPool, buildDefaultConnectionProfile(settings)); + } + + public ConnectionManager(Settings settings, Transport transport, ThreadPool threadPool, ConnectionProfile defaultProfile) { this.logger = Loggers.getLogger(getClass(), settings); this.transport = transport; this.threadPool = threadPool; this.pingSchedule = TcpTransport.PING_SCHEDULE.get(settings); + this.defaultProfile = defaultProfile; this.lifecycle.moveToStarted(); if (pingSchedule.millis() > 0) { @@ -84,6 +90,10 @@ public void removeListener(TransportConnectionListener listener) { this.connectionListener.listeners.remove(listener); } + public Transport.Connection openConnection(DiscoveryNode node, ConnectionProfile connectionProfile) { + return transport.openConnection(node, ConnectionProfile.resolveConnectionProfile(connectionProfile, defaultProfile)); + } + /** * Connects to a node with the given connection profile. If the node is already connected this method has no effect. * Once a successful is established, it can be validated before being exposed. @@ -91,6 +101,7 @@ public void removeListener(TransportConnectionListener listener) { public void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfile, CheckedBiConsumer connectionValidator) throws ConnectTransportException { + ConnectionProfile resolvedProfile = ConnectionProfile.resolveConnectionProfile(connectionProfile, defaultProfile); if (node == null) { throw new ConnectTransportException(null, "can't connect to a null node"); } @@ -104,8 +115,8 @@ public void connectToNode(DiscoveryNode node, ConnectionProfile connectionProfil } boolean success = false; try { - connection = transport.openConnection(node, connectionProfile); - connectionValidator.accept(connection, connectionProfile); + connection = transport.openConnection(node, resolvedProfile); + connectionValidator.accept(connection, resolvedProfile); // we acquire a connection lock, so no way there is an existing connection connectedNodes.put(node, connection); if (logger.isDebugEnabled()) { @@ -279,4 +290,23 @@ public void onNodeConnected(DiscoveryNode node) { } } } + + static ConnectionProfile buildDefaultConnectionProfile(Settings settings) { + int connectionsPerNodeRecovery = TransportService.CONNECTIONS_PER_NODE_RECOVERY.get(settings); + int connectionsPerNodeBulk = TransportService.CONNECTIONS_PER_NODE_BULK.get(settings); + int connectionsPerNodeReg = TransportService.CONNECTIONS_PER_NODE_REG.get(settings); + int connectionsPerNodeState = TransportService.CONNECTIONS_PER_NODE_STATE.get(settings); + int connectionsPerNodePing = TransportService.CONNECTIONS_PER_NODE_PING.get(settings); + ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); + builder.setConnectTimeout(TransportService.TCP_CONNECT_TIMEOUT.get(settings)); + builder.setHandshakeTimeout(TransportService.TCP_CONNECT_TIMEOUT.get(settings)); + builder.addConnections(connectionsPerNodeBulk, TransportRequestOptions.Type.BULK); + builder.addConnections(connectionsPerNodePing, TransportRequestOptions.Type.PING); + // if we are not master eligible we don't need a dedicated channel to publish the state + builder.addConnections(DiscoveryNode.isMasterNode(settings) ? connectionsPerNodeState : 0, TransportRequestOptions.Type.STATE); + // if we are not a data-node we don't need any dedicated channels for recovery + builder.addConnections(DiscoveryNode.isDataNode(settings) ? connectionsPerNodeRecovery : 0, TransportRequestOptions.Type.RECOVERY); + builder.addConnections(connectionsPerNodeReg, TransportRequestOptions.Type.REG); + return builder.build(); + } } diff --git a/server/src/main/java/org/elasticsearch/transport/TransportService.java b/server/src/main/java/org/elasticsearch/transport/TransportService.java index de3d4a3e711e4..8eca6504b70be 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportService.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportService.java @@ -127,7 +127,6 @@ protected boolean removeEldestEntry(Map.Entry eldest) { Function.identity(), Property.Dynamic, Property.NodeScope); private final Logger tracerLog; - private final ConnectionProfile defaultConnectionProfile; volatile String[] tracerLogInclude; volatile String[] tracerLogExclude; @@ -198,7 +197,6 @@ public TransportService(Settings settings, Transport transport, ThreadPool threa this.connectToRemoteCluster = RemoteClusterService.ENABLE_REMOTE_CLUSTERS.get(settings); remoteClusterService = new RemoteClusterService(settings, this); responseHandlers = transport.getResponseHandlers(); - defaultConnectionProfile = buildDefaultConnectionProfile(settings); if (clusterSettings != null) { clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_INCLUDE_SETTING, this::setTracerLogInclude); clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracerLogExclude); @@ -366,8 +364,7 @@ public void connectToNode(final DiscoveryNode node, ConnectionProfile connection return; } - ConnectionProfile resolvedProfile = ConnectionProfile.resolveConnectionProfile(connectionProfile, defaultConnectionProfile); - connectionManager.connectToNode(node, resolvedProfile, (newConnection, actualProfile) -> { + connectionManager.connectToNode(node, connectionProfile, (newConnection, actualProfile) -> { // We don't validate cluster names to allow for CCS connections. final DiscoveryNode remote = handshake(newConnection, actualProfile.getHandshakeTimeout().millis(), cn -> true).discoveryNode; if (validateConnections && node.equals(remote) == false) { @@ -380,13 +377,13 @@ public void connectToNode(final DiscoveryNode node, ConnectionProfile connection * Establishes and returns a new connection to the given node. The connection is NOT maintained by this service, it's the callers * responsibility to close the connection once it goes out of scope. * @param node the node to connect to - * @param profile the connection profile to use + * @param connectionProfile the connection profile to use */ - public Transport.Connection openConnection(final DiscoveryNode node, ConnectionProfile profile) throws IOException { + public Transport.Connection openConnection(final DiscoveryNode node, ConnectionProfile connectionProfile) throws IOException { if (isLocalNode(node)) { return localNodeConnection; } else { - return transport.openConnection(node, ConnectionProfile.resolveConnectionProfile(profile, defaultConnectionProfile)); + return connectionManager.openConnection(node, connectionProfile); } } @@ -453,25 +450,6 @@ public ConnectionManager getConnectionManager() { return connectionManager; } - static ConnectionProfile buildDefaultConnectionProfile(Settings settings) { - int connectionsPerNodeRecovery = CONNECTIONS_PER_NODE_RECOVERY.get(settings); - int connectionsPerNodeBulk = CONNECTIONS_PER_NODE_BULK.get(settings); - int connectionsPerNodeReg = CONNECTIONS_PER_NODE_REG.get(settings); - int connectionsPerNodeState = CONNECTIONS_PER_NODE_STATE.get(settings); - int connectionsPerNodePing = CONNECTIONS_PER_NODE_PING.get(settings); - ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); - builder.setConnectTimeout(TCP_CONNECT_TIMEOUT.get(settings)); - builder.setHandshakeTimeout(TCP_CONNECT_TIMEOUT.get(settings)); - builder.addConnections(connectionsPerNodeBulk, TransportRequestOptions.Type.BULK); - builder.addConnections(connectionsPerNodePing, TransportRequestOptions.Type.PING); - // if we are not master eligible we don't need a dedicated channel to publish the state - builder.addConnections(DiscoveryNode.isMasterNode(settings) ? connectionsPerNodeState : 0, TransportRequestOptions.Type.STATE); - // if we are not a data-node we don't need any dedicated channels for recovery - builder.addConnections(DiscoveryNode.isDataNode(settings) ? connectionsPerNodeRecovery : 0, TransportRequestOptions.Type.RECOVERY); - builder.addConnections(connectionsPerNodeReg, TransportRequestOptions.Type.REG); - return builder.build(); - } - static class HandshakeRequest extends TransportRequest { public static final HandshakeRequest INSTANCE = new HandshakeRequest(); diff --git a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java index c3d325ae77033..0f3288b1973e5 100644 --- a/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java +++ b/server/src/test/java/org/elasticsearch/discovery/AbstractDisruptionTestCase.java @@ -44,7 +44,6 @@ import org.elasticsearch.test.disruption.ServiceDisruptionScheme; import org.elasticsearch.test.disruption.SlowClusterStateProcessing; import org.elasticsearch.test.transport.MockTransportService; -import org.elasticsearch.transport.TcpTransport; import org.elasticsearch.transport.TransportService; import org.junit.Before; diff --git a/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java b/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java index 64e8a42600458..3c099c32bde2d 100644 --- a/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java +++ b/server/src/test/java/org/elasticsearch/transport/ConnectionManagerTests.java @@ -35,6 +35,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -62,6 +63,73 @@ public void stopThreadPool() { threadPool.shutdown(); } + public void testConnectionProfileResolve() { + final ConnectionProfile defaultProfile = ConnectionManager.buildDefaultConnectionProfile(Settings.EMPTY); + assertEquals(defaultProfile, ConnectionProfile.resolveConnectionProfile(null, defaultProfile)); + + final ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); + builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.BULK); + builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.RECOVERY); + builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.REG); + builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.STATE); + builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.PING); + + final boolean connectionTimeoutSet = randomBoolean(); + if (connectionTimeoutSet) { + builder.setConnectTimeout(TimeValue.timeValueMillis(randomNonNegativeLong())); + } + final boolean connectionHandshakeSet = randomBoolean(); + if (connectionHandshakeSet) { + builder.setHandshakeTimeout(TimeValue.timeValueMillis(randomNonNegativeLong())); + } + + final ConnectionProfile profile = builder.build(); + final ConnectionProfile resolved = ConnectionProfile.resolveConnectionProfile(profile, defaultProfile); + assertNotEquals(resolved, defaultProfile); + assertThat(resolved.getNumConnections(), equalTo(profile.getNumConnections())); + assertThat(resolved.getHandles(), equalTo(profile.getHandles())); + + assertThat(resolved.getConnectTimeout(), + equalTo(connectionTimeoutSet ? profile.getConnectTimeout() : defaultProfile.getConnectTimeout())); + assertThat(resolved.getHandshakeTimeout(), + equalTo(connectionHandshakeSet ? profile.getHandshakeTimeout() : defaultProfile.getHandshakeTimeout())); + } + + public void testDefaultConnectionProfile() { + ConnectionProfile profile = ConnectionManager.buildDefaultConnectionProfile(Settings.EMPTY); + assertEquals(13, profile.getNumConnections()); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); + assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); + assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); + assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); + + profile = ConnectionManager.buildDefaultConnectionProfile(Settings.builder().put("node.master", false).build()); + assertEquals(12, profile.getNumConnections()); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); + assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); + assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); + assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); + assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); + + profile = ConnectionManager.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).build()); + assertEquals(11, profile.getNumConnections()); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); + assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); + assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); + assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); + + profile = ConnectionManager.buildDefaultConnectionProfile(Settings.builder().put("node.data", false) + .put("node.master", false).build()); + assertEquals(10, profile.getNumConnections()); + assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); + assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); + assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); + assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); + assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); + } + public void testConnectAndDisconnect() { AtomicInteger nodeConnectedCount = new AtomicInteger(); AtomicInteger nodeDisconnectedCount = new AtomicInteger(); diff --git a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java index 215d3edc16fcc..a3d2e1bbc574e 100644 --- a/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java +++ b/server/src/test/java/org/elasticsearch/transport/TcpTransportTests.java @@ -29,7 +29,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; -import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.PageCacheRecycler; import org.elasticsearch.test.ESTestCase; @@ -305,72 +304,6 @@ public void writeTo(StreamOutput out) throws IOException { } } - public void testConnectionProfileResolve() { - final ConnectionProfile defaultProfile = TransportService.buildDefaultConnectionProfile(Settings.EMPTY); - assertEquals(defaultProfile, ConnectionProfile.resolveConnectionProfile(null, defaultProfile)); - - final ConnectionProfile.Builder builder = new ConnectionProfile.Builder(); - builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.BULK); - builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.RECOVERY); - builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.REG); - builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.STATE); - builder.addConnections(randomIntBetween(0, 5), TransportRequestOptions.Type.PING); - - final boolean connectionTimeoutSet = randomBoolean(); - if (connectionTimeoutSet) { - builder.setConnectTimeout(TimeValue.timeValueMillis(randomNonNegativeLong())); - } - final boolean connectionHandshakeSet = randomBoolean(); - if (connectionHandshakeSet) { - builder.setHandshakeTimeout(TimeValue.timeValueMillis(randomNonNegativeLong())); - } - - final ConnectionProfile profile = builder.build(); - final ConnectionProfile resolved = ConnectionProfile.resolveConnectionProfile(profile, defaultProfile); - assertNotEquals(resolved, defaultProfile); - assertThat(resolved.getNumConnections(), equalTo(profile.getNumConnections())); - assertThat(resolved.getHandles(), equalTo(profile.getHandles())); - - assertThat(resolved.getConnectTimeout(), - equalTo(connectionTimeoutSet ? profile.getConnectTimeout() : defaultProfile.getConnectTimeout())); - assertThat(resolved.getHandshakeTimeout(), - equalTo(connectionHandshakeSet ? profile.getHandshakeTimeout() : defaultProfile.getHandshakeTimeout())); - } - - public void testDefaultConnectionProfile() { - ConnectionProfile profile = TransportService.buildDefaultConnectionProfile(Settings.EMPTY); - assertEquals(13, profile.getNumConnections()); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); - assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); - assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); - assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - - profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.master", false).build()); - assertEquals(12, profile.getNumConnections()); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); - assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); - assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); - assertEquals(2, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); - assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - - profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).build()); - assertEquals(11, profile.getNumConnections()); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); - assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); - assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); - assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - - profile = TransportService.buildDefaultConnectionProfile(Settings.builder().put("node.data", false).put("node.master", false).build()); - assertEquals(10, profile.getNumConnections()); - assertEquals(1, profile.getNumConnectionsPerType(TransportRequestOptions.Type.PING)); - assertEquals(6, profile.getNumConnectionsPerType(TransportRequestOptions.Type.REG)); - assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.STATE)); - assertEquals(0, profile.getNumConnectionsPerType(TransportRequestOptions.Type.RECOVERY)); - assertEquals(3, profile.getNumConnectionsPerType(TransportRequestOptions.Type.BULK)); - } - public void testDecodeWithIncompleteHeader() throws IOException { BytesStreamOutput streamOutput = new BytesStreamOutput(1 << 14); streamOutput.write('E'); diff --git a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableConnectionManager.java b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableConnectionManager.java index a74cb2752c284..486ccc805d055 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableConnectionManager.java +++ b/test/framework/src/main/java/org/elasticsearch/test/transport/StubbableConnectionManager.java @@ -78,6 +78,11 @@ public void clearBehavior(TransportAddress transportAddress) { nodeConnectedBehaviors.remove(transportAddress); } + @Override + public Transport.Connection openConnection(DiscoveryNode node, ConnectionProfile connectionProfile) { + return delegate.openConnection(node, connectionProfile); + } + @Override public Transport.Connection getConnection(DiscoveryNode node) { TransportAddress address = node.getAddress(); From 357f2900b42c912cac28ad9d84a41acd9d5ed336 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 14 Aug 2018 14:11:13 -0600 Subject: [PATCH 3/3] Fix tests --- .../transport/AbstractSimpleTransportTestCase.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 50b7b8ce57597..5fff95116aee0 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -1995,11 +1995,11 @@ protected String handleRequest(TcpChannel mockChannel, String profileName, Strea assertEquals("handshake failed", exception.getCause().getMessage()); } + ConnectionProfile connectionProfile = ConnectionManager.buildDefaultConnectionProfile(Settings.EMPTY); try (TransportService service = buildService("TS_TPC", Version.CURRENT, null); TcpTransport.NodeChannels connection = originalTransport.openConnection( new DiscoveryNode("TS_TPC", "TS_TPC", service.boundAddress().publishAddress(), emptyMap(), emptySet(), version0), - null - )) { + connectionProfile)) { Version version = originalTransport.executeHandshake(connection.getNode(), connection.channel(TransportRequestOptions.Type.PING), TimeValue.timeValueSeconds(10)); assertEquals(version, Version.CURRENT);